Skip to content

Commit

Permalink
remove webhook and add the isvc_service_cert_reconciler again
Browse files Browse the repository at this point in the history
Signed-off-by: jooho lee <[email protected]>
  • Loading branch information
Jooho committed Jun 22, 2024
1 parent de1451c commit 1de88b0
Show file tree
Hide file tree
Showing 11 changed files with 203 additions and 200 deletions.
12 changes: 0 additions & 12 deletions config/webhook/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,9 @@ patches:
kind: ValidatingWebhookConfiguration
name: validating-webhook-configuration
version: v1
- path: mutator_webhook_patch.yaml
target:
group: admissionregistration.k8s.io
kind: MutatingWebhookConfiguration
name: mutating-webhook-configuration
version: v1
- path: field_patch.yaml
target:
group: admissionregistration.k8s.io
kind: ValidatingWebhookConfiguration
name: validating-webhook-configuration
version: v1
- path: mutator_field_patch.yaml
target:
group: admissionregistration.k8s.io
kind: MutatingWebhookConfiguration
name: mutating-webhook-configuration
version: v1
25 changes: 0 additions & 25 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
@@ -1,30 +1,5 @@
---
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
name: mutating-webhook-configuration
webhooks:
- admissionReviewVersions:
- v1
clientConfig:
service:
name: webhook-service
namespace: system
path: /mutate-serving-kserve-service
failurePolicy: Fail
name: mutating.kserve-service.odh-model-controller.opendatahub.io
rules:
- apiGroups:
- ""
apiVersions:
- v1
operations:
- CREATE
resources:
- services
sideEffects: None
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
name: validating-webhook-configuration
Expand Down
3 changes: 0 additions & 3 deletions config/webhook/mutator_field_patch.yaml

This file was deleted.

11 changes: 0 additions & 11 deletions config/webhook/mutator_webhook_patch.yaml

This file was deleted.

24 changes: 21 additions & 3 deletions controllers/inferenceservice_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
apierrs "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
// "sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
Expand Down Expand Up @@ -126,9 +126,10 @@ var certSecretPredicate = predicate.Funcs{

// check if the src secret has ownerReferences for InferenceService
func hasInferenceServiceOwner(obj client.Object) bool {

ownerReferences := obj.GetOwnerReferences()
for _, ownerReference := range ownerReferences {
if ownerReference.Kind == "InferenceService" {
if ownerReference.Kind == "Service" {
return true
}
}
Expand Down Expand Up @@ -195,8 +196,25 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager)
return []reconcile.Request{
{NamespacedName: types.NamespacedName{Name: o.GetName(), Namespace: o.GetNamespace()}},
}
}), builder.WithPredicates(certSecretPredicate))
}))

// 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()}},
// }
// }), builder.WithPredicates(certSecretPredicate))
kserveWithMeshEnabled, kserveWithMeshEnabledErr := utils.VerifyIfComponentIsEnabled(context.Background(), mgr.GetClient(), utils.KServeWithServiceMeshComponent)
if kserveWithMeshEnabledErr != nil {
r.log.V(1).Error(kserveWithMeshEnabledErr, "could not determine if kserve have service mesh enabled")
Expand Down
49 changes: 49 additions & 0 deletions controllers/kserve_inferenceservice_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,55 @@ var _ = Describe("The Openshift Kserve model controller", func() {
return err
}, timeout, interval).Should(Succeed())
})
It("With a new Kserve InferenceService, serving cert annotation should be added to the runtime Service object.", func() {
// We need to stub the cluster state and indicate where is istio namespace (reusing authConfig test data)
if dsciErr := createDSCI(DSCIWithoutAuthorization); dsciErr != nil && !errors.IsAlreadyExists(dsciErr) {
Fail(dsciErr.Error())
}
// Create a new InferenceService
inferenceService := &kservev1beta1.InferenceService{}
err := convertToStructuredResource(KserveInferenceServicePath1, inferenceService)
Expect(err).NotTo(HaveOccurred())
inferenceService.SetNamespace(testNs)
Expect(cli.Create(ctx, inferenceService)).Should(Succeed())
// Update the URL of the InferenceService to indicate it is ready.
deployedInferenceService := &kservev1beta1.InferenceService{}
err = cli.Get(ctx, types.NamespacedName{Name: inferenceService.Name, Namespace: inferenceService.Namespace}, deployedInferenceService)
Expect(err).NotTo(HaveOccurred())
// url, err := apis.ParseURL("https://example-onnx-mnist-default.test.com")
Expect(err).NotTo(HaveOccurred())
newAddress := &duckv1.Addressable{
URL: apis.HTTPS("example-onnx-mnist-default.test.com"),
}
deployedInferenceService.Status.Address = newAddress
err = cli.Status().Update(ctx, deployedInferenceService)
Expect(err).NotTo(HaveOccurred())
// Stub: Create a Kserve Service, which must be created by the KServe operator.
svc := &corev1.Service{}
err = convertToStructuredResource(testIsvcSvcPath, svc)
Expect(err).NotTo(HaveOccurred())
svc.SetNamespace(inferenceService.Namespace)
Expect(cli.Create(ctx, svc)).Should(Succeed())
err = cli.Status().Update(ctx, deployedInferenceService)
Expect(err).NotTo(HaveOccurred())
// isvcService, err := waitForService(cli, testNs, inferenceService.Name, 5, 2*time.Second)
// Expect(err).NotTo(HaveOccurred())

isvcService := &corev1.Service{}
Eventually(func() error {
err := cli.Get(ctx, client.ObjectKey{Namespace: inferenceService.Namespace, Name: inferenceService.Name}, isvcService)
if err != nil {
return err
}
if isvcService.Annotations == nil || isvcService.Annotations[constants.ServingCertAnnotationKey] == "" {

return fmt.Errorf("Annotation[constants.ServingCertAnnotationKey] is not added yet")
}
return nil
}, timeout, interval).Should(Succeed())

Expect(isvcService.Annotations[constants.ServingCertAnnotationKey]).Should(Equal(inferenceService.Name))
})

It("should create a secret for runtime and update kserve local gateway in the istio-system namespace", func() {
// We need to stub the cluster state and indicate where is istio namespace (reusing authConfig test data)
Expand Down
131 changes: 131 additions & 0 deletions controllers/reconcilers/kserve_isvc_service_cert_reconciler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package reconcilers

import (
"context"

"github.com/go-logr/logr"
kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
"github.com/opendatahub-io/odh-model-controller/controllers/constants"
"github.com/opendatahub-io/odh-model-controller/controllers/resources"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)

var _ SubResourceReconciler = (*KserveIsvcServiceReconciler)(nil)

type KserveIsvcServiceReconciler struct {
client client.Client
serviceHandler resources.ServiceHandler
}

func NewKserveIsvcServiceReconciler(client client.Client) *KserveIsvcServiceReconciler {
return &KserveIsvcServiceReconciler{
client: client,
serviceHandler: resources.NewServiceHandler(client),
}
}

func (r *KserveIsvcServiceReconciler) Delete(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error {
// NOOP - Resources are deleted along with the deletion of InferenceServices
return nil
}

func (r *KserveIsvcServiceReconciler) Cleanup(_ context.Context, _ logr.Logger, _ string) error {
// NOOP - Resources are deleted along with the deletion of InferenceServices
return nil
}

func (r *KserveIsvcServiceReconciler) Reconcile(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error {
log.V(1).Info("Reconciling InferenceService Service serving cert")

// return if Address.URL is not set
if isvc.Status.Address != nil && isvc.Status.Address.URL == nil {
log.V(1).Info("Waiting for the URL as the InferenceService is not ready yet")
return nil
}

// Create Desired resource
desiredResource, err := r.createDesiredResource(isvc)
if err != nil {
return err
}

// Get Existing resource
existingResource, err := r.getExistingResource(ctx, log, isvc)
if err != nil {
return err
}

// Process Delta
if err = r.processDelta(ctx, log, desiredResource, existingResource); err != nil {
return err
}
return nil
}

func (r *KserveIsvcServiceReconciler) createDesiredResource(isvc *kservev1beta1.InferenceService) (*v1.Service, error) {
service := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "isvc-service",
Annotations: map[string]string{
constants.ServingCertAnnotationKey: isvc.Name,
},
},
}
return service, nil
}

func (r *KserveIsvcServiceReconciler) getExistingResource(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) (*v1.Service, error) {
return r.serviceHandler.FetchService(ctx, log, types.NamespacedName{Name: isvc.Name, Namespace: isvc.Namespace})
}

func (r *KserveIsvcServiceReconciler) processDelta(ctx context.Context, log logr.Logger, desiredService *v1.Service, existingService *v1.Service) (err error) {
if isUpdated(desiredService, existingService, log) {
log.V(1).Info("Delta found", "update", existingService.GetName())
service := existingService.DeepCopy()
if service.Annotations == nil {
service.Annotations = make(map[string]string)
}
service.Annotations = desiredService.Annotations

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

func isUpdated(desiredService *v1.Service, existingService *v1.Service, log logr.Logger) bool {
if existingService == nil {
log.Info("The service for the InferenceService has not been created yet")
return false
}
deployedAnnotations := existingService.GetAnnotations()

if len(deployedAnnotations) != 0 {
if val, exists := existingService.Annotations[constants.ServingCertAnnotationKey]; exists {
if val == desiredService.Annotations[constants.ServingCertAnnotationKey] {
return false
}
}
}

return true
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func NewKServeServerlessInferenceServiceReconciler(client client.Client) *Kserve
NewKServeIstioPeerAuthenticationReconciler(client),
NewKServeNetworkPolicyReconciler(client),
NewKserveAuthConfigReconciler(client),
NewKserveIsvcServiceReconciler(client),
NewKserveGatewayReconciler(client),
NewKserveMetricsDashboardReconciler(client),
}
Expand Down
66 changes: 0 additions & 66 deletions controllers/webhook/kserve_service_mutator.go

This file was deleted.

Loading

0 comments on commit 1de88b0

Please sign in to comment.