Merge pull request #84 from vaibhavjainwiz/generic_route

Expose generic OpenShift routes for KServe InferenceServices

Makefile

@@ -2,7 +2,7 @@
 # Image URL to use all building/pushing image targets
 IMG ?= quay.io/${USER}/odh-model-controller:latest
 # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.23
+ENVTEST_K8S_VERSION = 1.26
 
 # Setting SHELL to bash allows bash commands to be executed by recipes.
 # This is a requirement for 'setup-envtest.sh' in the test target.
@@ -66,7 +66,7 @@ vet: ## Run go vet against code.
 
 .PHONY: test
 test: manifests generate fmt vet envtest ## Run tests.
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./... -coverprofile cover.out
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" POD_NAMESPACE=default go test ./... -coverprofile cover.out
 
 ##@ Build
 

config/crd/external/maistra.io_servicemeshmemberrolls.yaml

@@ -0,0 +1,208 @@
+
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: v0.7.0
+  creationTimestamp: null
+  name: servicemeshmemberrolls.maistra.io
+spec:
+  group: maistra.io
+  names:
+    categories:
+    - maistra-io
+    kind: ServiceMeshMemberRoll
+    listKind: ServiceMeshMemberRollList
+    plural: servicemeshmemberrolls
+    shortNames:
+    - smmr
+    singular: servicemeshmemberroll
+  scope: Namespaced
+  versions:
+  - additionalPrinterColumns:
+    - description: How many of the total number of member namespaces are configured
+      jsonPath: .status.annotations.configuredMemberCount
+      name: Ready
+      type: string
+    - description: Whether all member namespaces have been configured or why that's
+        not the case
+      jsonPath: .status.conditions[?(@.type=="Ready")].reason
+      name: Status
+      type: string
+    - description: The age of the object
+      jsonPath: .metadata.creationTimestamp
+      name: Age
+      type: date
+    - description: Namespaces that are members of this Control Plane
+      jsonPath: .status.members
+      name: Members
+      priority: 1
+      type: string
+    name: v1
+    schema:
+      openAPIV3Schema:
+        description: The ServiceMeshMemberRoll object configures which namespaces
+          belong to a service mesh. Only namespaces listed in the ServiceMeshMemberRoll
+          will be affected by the control plane. Any number of namespaces can be added,
+          but a namespace may not exist in more than one service mesh. The ServiceMeshMemberRoll
+          object must be created in the same namespace as the ServiceMeshControlPlane
+          object and must be named "default".
+        properties:
+          apiVersion:
+            description: 'APIVersion defines the versioned schema of this representation
+              of an object. Servers should convert recognized schemas to the latest
+              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+            type: string
+          kind:
+            description: 'Kind is a string value representing the REST resource this
+              object represents. Servers may infer this from the endpoint the client
+              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+            type: string
+          metadata:
+            type: object
+          spec:
+            description: Specification of the desired list of members of the service
+              mesh.
+            properties:
+              members:
+                description: ' List of namespaces that should be members of the service
+                  mesh.'
+                items:
+                  type: string
+                nullable: true
+                type: array
+            type: object
+          status:
+            description: The current status of this ServiceMeshMemberRoll. This data
+              may be out of date by some window of time.
+            properties:
+              annotations:
+                additionalProperties:
+                  type: string
+                description: Annotations is an unstructured key value map used to
+                  store additional, usually redundant status information, such as
+                  the number of components deployed by the ServiceMeshControlPlane
+                  (number is redundant because you could just as easily count the
+                  elements in the ComponentStatus array). The reason to add this redundant
+                  information is to make it available to kubectl, which does not yet
+                  allow counting objects in JSONPath expressions.
+                type: object
+              conditions:
+                description: Represents the latest available observations of this
+                  ServiceMeshMemberRoll's current state.
+                items:
+                  description: Condition represents a specific condition on a resource
+                  properties:
+                    lastTransitionTime:
+                      format: date-time
+                      type: string
+                    message:
+                      type: string
+                    reason:
+                      type: string
+                    status:
+                      type: string
+                    type:
+                      description: 'ServiceMeshMemberRollConditionType represents
+                        the type of the condition.  Condition types are: Reconciled,
+                        NamespaceConfigured'
+                      type: string
+                  type: object
+                nullable: true
+                type: array
+              configuredMembers:
+                description: List of namespaces that are configured as members of
+                  the service mesh.
+                items:
+                  type: string
+                nullable: true
+                type: array
+              memberStatuses:
+                description: Represents the latest available observations of each
+                  member's current state.
+                items:
+                  description: ServiceMeshMemberStatusSummary represents a summary
+                    status of a ServiceMeshMember.
+                  properties:
+                    conditions:
+                      items:
+                        description: Condition represents a specific condition on
+                          a resource
+                        properties:
+                          lastTransitionTime:
+                            format: date-time
+                            type: string
+                          message:
+                            type: string
+                          reason:
+                            type: string
+                          status:
+                            type: string
+                          type:
+                            description: 'ServiceMeshMemberConditionType represents
+                              the type of the condition.  Condition types are: Reconciled,
+                              NamespaceConfigured'
+                            type: string
+                        type: object
+                      type: array
+                    namespace:
+                      type: string
+                  required:
+                  - conditions
+                  - namespace
+                  type: object
+                nullable: true
+                type: array
+              members:
+                description: "Complete list of namespaces that are configured as members
+                  of the service mesh\t- this includes namespaces specified in spec.members
+                  and those that contain a ServiceMeshMember object"
+                items:
+                  type: string
+                nullable: true
+                type: array
+              meshGeneration:
+                description: The generation of the ServiceMeshControlPlane object
+                  observed by the controller during the most recent reconciliation
+                  of this ServiceMeshMemberRoll.
+                format: int64
+                type: integer
+              meshReconciledVersion:
+                description: The reconciled version of the ServiceMeshControlPlane
+                  object observed by the controller during the most recent reconciliation
+                  of this ServiceMeshMemberRoll.
+                type: string
+              observedGeneration:
+                description: The generation observed by the controller during the
+                  most recent reconciliation. The information in the status pertains
+                  to this particular generation of the object.
+                format: int64
+                type: integer
+              pendingMembers:
+                description: List of namespaces that haven't been configured as members
+                  of the service mesh yet.
+                items:
+                  type: string
+                nullable: true
+                type: array
+              terminatingMembers:
+                description: List of namespaces that are being removed as members
+                  of the service mesh.
+                items:
+                  type: string
+                nullable: true
+                type: array
+            type: object
+        required:
+        - spec
+        type: object
+    served: true
+    storage: true
+    subresources:
+      status: {}
+status:
+  acceptedNames:
+    kind: ""
+    plural: ""
+  conditions: []
+  storedVersions: []

config/rbac/role.yaml

@@ -138,11 +138,18 @@ rules:
   - routes
   verbs:
   - create
+  - delete
   - get
   - list
   - patch
   - update
   - watch
+- apiGroups:
+  - route.openshift.io
+  resources:
+  - routes/custom-host
+  verbs:
+  - create
 - apiGroups:
   - security.istio.io
   resources:

controllers/comparators/resourcecomparator.go

@@ -0,0 +1,10 @@
+package comparators
+
+import (
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// ResourceComparator would compare deployed & requested resource. It would retrun `true` if both resource are same else it would return `false`
+type ResourceComparator interface {
+	Compare(deployed client.Object, requested client.Object) bool
+}

controllers/comparators/route_comparator.go

@@ -0,0 +1,18 @@
+package comparators
+
+import (
+	v1 "github.com/openshift/api/route/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+type RouteComparator struct {
+}
+
+func (c *RouteComparator) Compare(deployed client.Object, requested client.Object) bool {
+	deployedRoute := deployed.(*v1.Route)
+	requestedRoute := requested.(*v1.Route)
+	return equality.Semantic.DeepEqual(deployedRoute.Spec, requestedRoute.Spec) &&
+		equality.Semantic.DeepEqual(deployedRoute.Annotations, requestedRoute.Annotations) &&
+		equality.Semantic.DeepEqual(deployedRoute.Labels, requestedRoute.Labels)
+}

controllers/components/route.go

@@ -0,0 +1,66 @@
+package components
+
+import (
+	"context"
+	"fmt"
+	"github.com/go-logr/logr"
+	"github.com/opendatahub-io/odh-model-controller/controllers/comparators"
+	v1 "github.com/openshift/api/route/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// RouteHandler to provide route specific implementation.
+type RouteHandler interface {
+	FetchRoute(key types.NamespacedName) (*v1.Route, error)
+	GetComparator() comparators.ResourceComparator
+	DeleteRoute(key types.NamespacedName) error
+}
+
+type routeHandler struct {
+	client.Client
+	ctx context.Context
+	log logr.Logger
+}
+
+func NewRouteHandler(client client.Client, ctx context.Context, log logr.Logger) RouteHandler {
+	return &routeHandler{
+		Client: client,
+		ctx:    ctx,
+		log:    log,
+	}
+}
+
+func (r *routeHandler) FetchRoute(key types.NamespacedName) (*v1.Route, error) {
+	route := &v1.Route{}
+	err := r.Get(r.ctx, key, route)
+	if err != nil && errors.IsNotFound(err) {
+		r.log.Info("Openshift Route not found.")
+		return nil, nil
+	} else if err != nil {
+		return nil, err
+	}
+	r.log.Info("Successfully fetch deployed Openshift Route")
+	return route, nil
+}
+
+func (r *routeHandler) GetComparator() comparators.ResourceComparator {
+	return &comparators.RouteComparator{}
+}
+
+func (r *routeHandler) DeleteRoute(key types.NamespacedName) error {
+	route := &v1.Route{}
+	err := r.Get(r.ctx, key, route)
+	if err != nil {
+		if errors.IsNotFound(err) {
+			return nil
+		}
+		return err
+	}
+	if err = r.Delete(r.ctx, route); err != nil {
+		return fmt.Errorf("failed to delete route: %w", err)
+	}
+
+	return nil
+}

controllers/constants/constants.go

@@ -0,0 +1,8 @@
+package constants
+
+const (
+	IstioNamespace                   = "istio-system"
+	IstioIngressService              = "istio-ingressgateway"
+	IstioIngressServiceHTTPPortName  = "http2"
+	IstioIngressServiceHTTPSPortName = "https"
+)

controllers/inferenceservice_controller.go

@@ -82,15 +82,14 @@ func (r *OpenshiftInferenceServiceReconciler) Reconcile(ctx context.Context, req
 		return ctrl.Result{}, err
 	}
 
+	if inferenceservice.GetDeletionTimestamp() != nil {
+		return reconcile.Result{}, r.onDeletion(ctx, inferenceservice)
+	}
+
 	// Check what deployment mode is used by the InferenceService. We have differing reconciliation logic for Kserve and ModelMesh
 	if r.isDeploymentModeForIsvcModelMesh(inferenceservice) {
 		log.Info("Reconciling InferenceService for ModelMesh")
-		err = r.ReconcileRoute(inferenceservice, ctx)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-
-		err = r.ReconcileSA(inferenceservice, ctx)
+		err = r.ReconcileModelMeshInference(ctx, req, inferenceservice)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
@@ -99,7 +98,7 @@ func (r *OpenshiftInferenceServiceReconciler) Reconcile(ctx context.Context, req
 		err = r.ReconcileKserveInference(ctx, req, inferenceservice)
 	}
 
-	return ctrl.Result{}, nil
+	return ctrl.Result{}, err
 }
 
 // SetupWithManager sets up the controller with the Manager.
@@ -152,3 +151,15 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager)
 
 	return nil
 }
+
+// general clean-up, mostly resources in different namespaces from kservev1beta1.InferenceService
+func (r *OpenshiftInferenceServiceReconciler) onDeletion(ctx context.Context, inferenceService *kservev1beta1.InferenceService) error {
+	log := r.Log.WithValues("InferenceService", inferenceService.Name, "namespace", inferenceService.Namespace)
+	log.V(1).Info("Running cleanup logic")
+
+	if !r.isDeploymentModeForIsvcModelMesh(inferenceService) {
+		log.V(1).Info("Deleting kserve inference resource")
+		return r.OnDeletionOfKserveInferenceService(ctx, inferenceService)
+	}
+	return nil
+}