Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose generic OpenShift routes for KServe InferenceServices #84

Merged
merged 8 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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

Expand Down
208 changes: 208 additions & 0 deletions config/crd/external/maistra.io_servicemeshmemberrolls.yaml
Original file line number Diff line number Diff line change
@@ -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: []
7 changes: 7 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 10 additions & 0 deletions controllers/comparators/resourcecomparator.go
Original file line number Diff line number Diff line change
@@ -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
}
18 changes: 18 additions & 0 deletions controllers/comparators/route_comparator.go
Original file line number Diff line number Diff line change
@@ -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)
}
66 changes: 66 additions & 0 deletions controllers/components/route.go
Original file line number Diff line number Diff line change
@@ -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
}
8 changes: 8 additions & 0 deletions controllers/constants/constants.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package constants

const (
IstioNamespace = "istio-system"
IstioIngressService = "istio-ingressgateway"
IstioIngressServiceHTTPPortName = "http2"
IstioIngressServiceHTTPSPortName = "https"
)
25 changes: 18 additions & 7 deletions controllers/inferenceservice_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
Loading