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.

Sign up for GitHub

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

Add the grpc route creation option #35

Closed
wants to merge 7 commits into from
Closed

Add the grpc route creation option #35

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2c58092
add the grpc route creation option
heyselbi May 7, 2023
eecb5f7
Merge branch 'opendatahub-io:main' into rhods-6401-grpc
heyselbi May 8, 2023
aefe1d9
remove enable-http annotation and enable-grpc by default
heyselbi May 9, 2023
2f5216d
removed http/s from naming of routes
heyselbi May 9, 2023
ba857f9
set passthrough for grpc tls termination
heyselbi Jun 1, 2023
5cdcb41
Merge branch 'opendatahub-io:main' into rhods-6401-grpc
heyselbi Jun 7, 2023
403b9c7
removed path for grpc passthrough route
heyselbi Jun 13, 2023
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
171 changes: 105 additions & 66 deletions controllers/inferenceservice_route.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@ const (
modelmeshServiceName = "modelmesh-serving"
modelmeshAuthServicePort = 8443
modelmeshServicePort = 8008
modelmeshGrpcPort = 8033
)

// NewInferenceServiceRoute defines the desired route object
func NewInferenceServiceRoute(inferenceservice *inferenceservicev1.InferenceService, enableAuth bool) *routev1.Route {
func NewInferenceServiceRoute(inferenceservice *inferenceservicev1.InferenceService, enableAuth bool, enableGrpc bool) *routev1.Route {

// create a http route
finalRoute := &routev1.Route{
ObjectMeta: metav1.ObjectMeta{
Name: inferenceservice.Name,
Expand All @@ -62,12 +64,17 @@ func NewInferenceServiceRoute(inferenceservice *inferenceservicev1.InferenceServ
},
WildcardPolicy: routev1.WildcardPolicyNone,
Path: "/v2/models/" + inferenceservice.Name,
TLS: &routev1.TLSConfig{
Termination: routev1.TLSTerminationEdge,
InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect,
},
},
Status: routev1.RouteStatus{
Ingress: []routev1.RouteIngress{},
},
}

// if secure route is selected, create the https route
if enableAuth {
finalRoute.Spec.Port = &routev1.RoutePort{
TargetPort: intstr.FromInt(modelmeshAuthServicePort),
Expand All @@ -76,9 +83,17 @@ func NewInferenceServiceRoute(inferenceservice *inferenceservicev1.InferenceServ
Termination: routev1.TLSTerminationReencrypt,
InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect,
}
} else {
}

// if grpc is selected, create a grpc route
if enableGrpc {
finalRoute.ObjectMeta.Name = inferenceservice.Name + "-grpc"
finalRoute.Spec.Port = &routev1.RoutePort{
TargetPort: intstr.FromInt(modelmeshGrpcPort),
}
finalRoute.Spec.Path = ""
finalRoute.Spec.TLS = &routev1.TLSConfig{
Termination: routev1.TLSTerminationEdge,
Termination: routev1.TLSTerminationPassthrough,
InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect,
}
}
Expand Down Expand Up @@ -157,91 +172,115 @@ func (r *OpenshiftInferenceServiceReconciler) findSupportingRuntimeForISvc(ctx c
// Reconcile will manage the creation, update and deletion of the route returned
// by the newRoute function
func (r *OpenshiftInferenceServiceReconciler) reconcileRoute(inferenceservice *inferenceservicev1.InferenceService,
ctx context.Context, newRoute func(service *inferenceservicev1.InferenceService, enableAuth bool) *routev1.Route) error {
ctx context.Context, newRoute func(service *inferenceservicev1.InferenceService, enableAuth bool, enableGrpc bool) *routev1.Route) error {
// Initialize logger format
log := r.Log.WithValues("inferenceservice", inferenceservice.Name, "namespace", inferenceservice.Namespace)

desiredServingRuntime := r.findSupportingRuntimeForISvc(ctx, log, inferenceservice)

enableAuth := true
if desiredServingRuntime.Annotations["enable-auth"] != "true" {
enableAuth = false
}
createRoute := true
if desiredServingRuntime.Annotations["enable-route"] != "true" {
createRoute = false
// optionsList allows for creation of both grpc and http/s routes as desired
var optionsList [2]string

createRoute, enableAuth, enableGrpc := false, false, false

// If routes need to be exposed, then enable both grpc and http/s
if desiredServingRuntime.Annotations["enable-route"] == "true" {
createRoute = true
enableGrpc = true
optionsList[1] = "grpc"

if desiredServingRuntime.Annotations["enable-auth"] == "true" {
enableAuth = true
optionsList[0] = "https"
} else {
optionsList[0] = "http"
}
}

// Generate the desired route
desiredRoute := newRoute(inferenceservice, enableAuth)

// Create the route if it does not already exist
foundRoute := &routev1.Route{}
justCreated := false
err := r.Get(ctx, types.NamespacedName{
Name: desiredRoute.Name,
Namespace: inferenceservice.Namespace,
}, foundRoute)
if err != nil {
if !createRoute {
log.Info("Serving runtime does not have 'enable-route' annotation set to 'True'. Skipping route creation")
return nil
for _, v := range optionsList {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer extracting the code inside this for to a function, so that you can call inline in the if block that is up these lines avoiding the need for this for loop.


// Check which route to create
if v == "http" || v == "https" {
enableGrpc = false
} else if v == "grpc" {
enableAuth, enableGrpc = false, true
} else {
continue
}
if apierrs.IsNotFound(err) {
log.Info("Creating Route")
// Add .metatada.ownerReferences to the route to be deleted by the
// Kubernetes garbage collector if the predictor is deleted
err = ctrl.SetControllerReference(inferenceservice, desiredRoute, r.Scheme)
if err != nil {
log.Error(err, "Unable to add OwnerReference to the Route")
return err

// Generate the desired routes
desiredRoute := newRoute(inferenceservice, enableAuth, enableGrpc)

// Create the route if it does not already exist
foundRoute := &routev1.Route{}
justCreated := false
err := r.Get(ctx, types.NamespacedName{
Name: desiredRoute.Name,
Namespace: inferenceservice.Namespace,
}, foundRoute)

if err != nil {
if !createRoute {
log.Info("Serving runtime does not have 'enable-route' annotation set to 'True'. Skipping route creation")
return nil
}
// Create the route in the Openshift cluster
err = r.Create(ctx, desiredRoute)
if err != nil && !apierrs.IsAlreadyExists(err) {
log.Error(err, "Unable to create the Route")
if apierrs.IsNotFound(err) {
log.Info("Creating Route")
// Add .metatada.ownerReferences to the route to be deleted by the
// Kubernetes garbage collector if the predictor is deleted
err = ctrl.SetControllerReference(inferenceservice, desiredRoute, r.Scheme)
if err != nil {
log.Error(err, "Unable to add OwnerReference to the Route")
return err
}
// Create the route in the Openshift cluster
err = r.Create(ctx, desiredRoute)
if err != nil && !apierrs.IsAlreadyExists(err) {
log.Error(err, "Unable to create the Route")
return err
}
justCreated = true
} else {
log.Error(err, "Unable to fetch the Route")
return err
}
justCreated = true
} else {
log.Error(err, "Unable to fetch the Route")
return err
}
}

if !createRoute {
log.Info("Serving Runtime does not have 'enable-route' annotation set to 'True'. Deleting existing route")
return r.Delete(ctx, foundRoute)
}
// Reconcile the route spec if it has been manually modified
if !justCreated && !CompareInferenceServiceRoutes(*desiredRoute, *foundRoute) {
log.Info("Reconciling Route")
// Retry the update operation when the ingress controller eventually
// updates the resource version field
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
// Get the last route revision
if err := r.Get(ctx, types.NamespacedName{
Name: desiredRoute.Name,
Namespace: inferenceservice.Namespace,
}, foundRoute); err != nil {
if !createRoute {
log.Info("Serving Runtime does not have 'enable-route' annotation set to 'True'. Deleting existing route")
return r.Delete(ctx, foundRoute)
}

// Reconcile the route spec if it has been manually modified
if !justCreated && !CompareInferenceServiceRoutes(*desiredRoute, *foundRoute) {
log.Info("Reconciling Route")
// Retry the update operation when the ingress controller eventually
// updates the resource version field
err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
// Get the last route revision
if err := r.Get(ctx, types.NamespacedName{
Name: desiredRoute.Name,
Namespace: inferenceservice.Namespace,
}, foundRoute); err != nil {
return err
}
// Reconcile labels and spec field
foundRoute.Spec = desiredRoute.Spec
foundRoute.ObjectMeta.Labels = desiredRoute.ObjectMeta.Labels
return r.Update(ctx, foundRoute)
})
if err != nil {
log.Error(err, "Unable to reconcile the Route")
return err
}
// Reconcile labels and spec field
foundRoute.Spec = desiredRoute.Spec
foundRoute.ObjectMeta.Labels = desiredRoute.ObjectMeta.Labels
return r.Update(ctx, foundRoute)
})
if err != nil {
log.Error(err, "Unable to reconcile the Route")
return err
}
}

return nil
}

// ReconcileRoute will manage the creation, update and deletion of the
// TLS route when the predictor is reconciled
// TLS route wheoptionsList[v]n the predictor is reconciled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy&paste mistake?

func (r *OpenshiftInferenceServiceReconciler) ReconcileRoute(
inferenceservice *inferenceservicev1.InferenceService, ctx context.Context) error {
return r.reconcileRoute(inferenceservice, ctx, NewInferenceServiceRoute)
Expand Down