Skip to content

Commit

Permalink
Merge pull request #229 from Jooho/internal_hostname_gateway_follow_up
Browse files Browse the repository at this point in the history 
Internal hostname gateway follow up
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Jun 27, 2024
2 parents 2ea55ef + e818f62 commit 76845fe
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 49 deletions.
17 changes: 17 additions & 0 deletions controllers/inferenceservice_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,23 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager)
})
}
return reconcileRequests
})).
Watches(&corev1.Secret{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request {
r.log.Info("Reconcile event triggered by Secret: " + o.GetName())
isvc := &kservev1beta1.InferenceService{}
err := r.client.Get(ctx, types.NamespacedName{Name: o.GetName(), Namespace: o.GetNamespace()}, isvc)
if err != nil {
if apierrs.IsNotFound(err) {
return []reconcile.Request{}
}
r.log.Error(err, "Error getting the inferenceService", "name", o.GetName())
return []reconcile.Request{}
}

return []reconcile.Request{
{NamespacedName: types.NamespacedName{Name: o.GetName(), Namespace: o.GetNamespace()}},
}
}))

kserveWithMeshEnabled, kserveWithMeshEnabledErr := utils.VerifyIfComponentIsEnabled(context.Background(), mgr.GetClient(), utils.KServeWithServiceMeshComponent)
Expand Down
47 changes: 9 additions & 38 deletions controllers/kserve_inferenceservice_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,21 +195,14 @@ var _ = Describe("The Openshift Kserve model controller", func() {
// Verify that the certificate secret is created in the istio-system namespace.
Eventually(func() error {
secret := &corev1.Secret{}
err := cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", inferenceService.Name, inferenceService.Namespace)}, secret)
if err != nil {
return err
}
return nil
return cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", inferenceService.Name, inferenceService.Namespace)}, secret)
}, timeout, interval).Should(Succeed())

// Verify that the gateway is updated in the istio-system namespace.
var gateway *istioclientv1beta1.Gateway
Eventually(func() error {
gateway, err = waitForUpdatedGatewayCompletion(cli, "add", constants.IstioNamespace, constants.KServeGatewayName, inferenceService.Name)
if err != nil {
return err
}
return nil
return err
}, timeout, interval).Should(Succeed())

// Ensure that the server is successfully added to the KServe local gateway within the istio-system namespace.
Expand Down Expand Up @@ -311,29 +304,18 @@ var _ = Describe("The Openshift Kserve model controller", func() {
// Verify that the certificate secret is created in the istio-system namespace.
Eventually(func() error {
secret := &corev1.Secret{}
err := cli.Get(ctx, types.NamespacedName{Name: inferenceService.Name, Namespace: inferenceService.Namespace}, secret)
if err != nil {
return err
}
return nil
return cli.Get(ctx, types.NamespacedName{Name: inferenceService.Name, Namespace: inferenceService.Namespace}, secret)
}, timeout, interval).Should(Succeed())

Eventually(func() error {
err = cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", inferenceService.Name, inferenceService.Namespace)}, secret)
if err != nil {
return err
}
return nil
return cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", inferenceService.Name, inferenceService.Namespace)}, secret)
}, timeout, interval).Should(Succeed())

// Verify that the gateway is updated in the istio-system namespace.
var gateway *istioclientv1beta1.Gateway
Eventually(func() error {
gateway, err = waitForUpdatedGatewayCompletion(cli, "add", constants.IstioNamespace, constants.KServeGatewayName, inferenceService.Name)
if err != nil {
return err
}
return nil
return err
}, timeout, interval).Should(Succeed())

// Ensure that the server is successfully added to the KServe local gateway within the istio-system namespace.
Expand Down Expand Up @@ -365,11 +347,7 @@ var _ = Describe("The Openshift Kserve model controller", func() {
// Verify that the certificate secret in the istio-system namespace is updated.
destSecret := &corev1.Secret{}
Eventually(func() error {
err := cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", deployedInferenceService.Name, deployedInferenceService.Namespace)}, destSecret)
if err != nil {
return err
}

Expect(cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", deployedInferenceService.Name, deployedInferenceService.Namespace)}, destSecret)).Should(Succeed())
if string(destSecret.Data["tls.crt"]) != updatedDataString {
return fmt.Errorf("destSecret is not updated yet")
}
Expand All @@ -392,10 +370,7 @@ var _ = Describe("The Openshift Kserve model controller", func() {
var gateway *istioclientv1beta1.Gateway
Eventually(func() error {
gateway, err = waitForUpdatedGatewayCompletion(cli, "delete", constants.IstioNamespace, constants.KServeGatewayName, isvcName)
if err != nil {
return err
}
return nil
return err
}, timeout, interval).Should(Succeed())

// Ensure that the server is successfully removed from the KServe local gateway within the istio-system namespace.
Expand All @@ -405,12 +380,8 @@ var _ = Describe("The Openshift Kserve model controller", func() {
// Ensure that the synced Secret is successfully deleted within the istio-system namespace.
secret := &corev1.Secret{}
Eventually(func() error {
err := cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", isvcName, constants.IstioNamespace)}, secret)
if err != nil && errors.IsNotFound(err) {
return nil
}
return err
}, timeout, interval).Should(Succeed())
return cli.Get(ctx, client.ObjectKey{Namespace: constants.IstioNamespace, Name: fmt.Sprintf("%s-%s", isvcName, constants.IstioNamespace)}, secret)
}, timeout, interval).ShouldNot(Succeed())
})
})
})
Expand Down
41 changes: 32 additions & 9 deletions controllers/reconcilers/kserve_isvc_gateway_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type KserveGatewayReconciler struct {
gatewayHandler resources.GatewayHandler
deltaProcessor processors.DeltaProcessor
}

// The clientReader uses the API server to retrieve Secrets that are not cached. By default, only Secrets with the specific label "opendatahub.io/managed: true" are cached.
func NewKserveGatewayReconciler(client client.Client, clientReader client.Reader) *KserveGatewayReconciler {

Expand Down Expand Up @@ -94,14 +95,14 @@ func (r *KserveGatewayReconciler) Reconcile(ctx context.Context, log logr.Logger
}
}
return err
}

// Recreate copied secrt when src secret is updated
if !reflect.DeepEqual(srcCertSecret.Data, copiedCertSecret.Data) {
log.V(1).Info(fmt.Sprintf("Recreating for serving certificate Secret(%s) in %s namespace", copiedCertSecret.Name, meshNamespace))
if err := r.copyServingCertSecretFromIsvcNamespace(ctx, srcCertSecret, copiedCertSecret); err != nil {
log.V(1).Error(err, fmt.Sprintf("Failed to copy the Secret(%s) for InferenceService in %s namespace", copiedCertSecret.Name, meshNamespace))
return err
} else {
// Recreate copied secrt when src secret is updated
if !reflect.DeepEqual(srcCertSecret.Data, copiedCertSecret.Data) {
log.V(2).Info(fmt.Sprintf("Recreating for serving certificate Secret(%s) in %s namespace", copiedCertSecret.Name, meshNamespace))
if err := r.copyServingCertSecretFromIsvcNamespace(ctx, srcCertSecret, copiedCertSecret); err != nil {
log.V(1).Error(err, fmt.Sprintf("Failed to copy the Secret(%s) for InferenceService in %s namespace", copiedCertSecret.Name, meshNamespace))
return err
}
}
}

Expand Down Expand Up @@ -237,7 +238,11 @@ func (r *KserveGatewayReconciler) copyServingCertSecretFromIsvcNamespace(ctx con
Name: fmt.Sprintf("%s-%s", sourceSecret.Name, sourceSecret.Namespace),
Namespace: meshNamespace,
Labels: map[string]string{
"opendatahub.io/managed": "true",
"opendatahub.io/managed": "true",
"app.kubernetes.io/name": "odh-model-controller",
"app.kubernetes.io/component": "kserve",
"app.kubernetes.io/part-of": "odh-model-serving",
"app.kubernetes.io/managed-by": "odh-model-controller",
},
},
Data: sourceSecret.Data,
Expand All @@ -254,9 +259,27 @@ func (r *KserveGatewayReconciler) copyServingCertSecretFromIsvcNamespace(ctx con
if err := r.client.Create(ctx, destinationSecret); err != nil {
return err
}

// add label 'opendatahub.io/managed=true' to original Secret for caching
if err := r.addServingCertSecretLabel(ctx, sourceSecret); err != nil {
return err
}
return nil
}

func (r *KserveGatewayReconciler) addServingCertSecretLabel(ctx context.Context, sourceSecret *corev1.Secret) error {
service := sourceSecret.DeepCopy()
if service.Labels == nil {
service.Labels = make(map[string]string)
}

service.Labels["opendatahub.io/managed"] = "true"

err := r.client.Update(ctx, service)

return err
}

func (r *KserveGatewayReconciler) deleteServingCertSecretInIstioNamespace(ctx context.Context, targetSecretName string) error {
secret, err := r.secretHandler.Get(ctx, types.NamespacedName{Name: targetSecretName, Namespace: meshNamespace})
if err != nil && errors.IsNotFound(err) {
Expand Down
7 changes: 5 additions & 2 deletions controllers/reconcilers/kserve_isvc_service_cert_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (r *KserveIsvcServiceReconciler) Cleanup(_ context.Context, _ logr.Logger,
return nil
}

//To support KServe local gateway using HTTPS, each InferenceService (ISVC) needs a certificate. This reconciliation process helps add a serving certificate annotation to the ISVC service.
// To support KServe local gateway using HTTPS, each InferenceService (ISVC) needs a certificate. This reconciliation process helps add a serving certificate annotation to the ISVC service.
func (r *KserveIsvcServiceReconciler) Reconcile(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error {
log.V(1).Info("Reconciling InferenceService Service serving cert")

Expand Down Expand Up @@ -104,7 +104,10 @@ func (r *KserveIsvcServiceReconciler) processDelta(ctx context.Context, log logr
if service.Annotations == nil {
service.Annotations = make(map[string]string)
}
service.Annotations = desiredService.Annotations

for key, value := range desiredService.Annotations {
service.Annotations[key] = value
}

if err = r.client.Update(ctx, service); err != nil {
return err
Expand Down

0 comments on commit 76845fe

Please sign in to comment.