Skip to content

Commit

Permalink
Enhance controller setup based on available CRDs
Browse files Browse the repository at this point in the history
This enhances the setup of the InferenceService controller and the InferenceGraph controller. Instead of relying on the `defaultDeploymentMode` configuration to determine what CRDs to watch, the setup now checks whether KNative Services and Istio VirtualServices are available in the cluster and setup the watches (invoke `Owns`) accordingly.

This enhancement has the following advantages:
* A crashloop is prevented if the CRDs are missing in the cluster. The user would still be able to create InferenceServices by taking care of annotating the ISVC for RawDeployment mode.
* If RawDeployment mode is configured as the default mode, the controllers would still watch for KNative and Istio resources if these components are available. This will let the controller watch for changes for the dependent resources if the user uses Serverless mode for some of the InferenceServices.
* In the InferenceService controller, the watch for the VirtualServices is still conditioned to the value of the `disableVirtualHost` configuration.

Signed-off-by: Edgar Hernández <23639005+israel-hdez@users.noreply.github.com>
  • Loading branch information
israel-hdez committed Feb 22, 2024
1 parent 99ac7b2 commit b6bf6aa
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 26 deletions.
24 changes: 15 additions & 9 deletions pkg/controller/v1alpha1/inferencegraph/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
v1beta1api "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
"github.com/kserve/kserve/pkg/constants"
isvcutils "github.com/kserve/kserve/pkg/controller/v1beta1/inferenceservice/utils"
"github.com/kserve/kserve/pkg/utils"
"github.com/pkg/errors"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -266,15 +267,20 @@ func inferenceGraphReadiness(status v1alpha1api.InferenceGraphStatus) bool {
}

func (r *InferenceGraphReconciler) SetupWithManager(mgr ctrl.Manager, deployConfig *v1beta1api.DeployConfig) error {
if deployConfig.DefaultDeploymentMode == string(constants.RawDeployment) {
return ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1api.InferenceGraph{}).
Owns(&appsv1.Deployment{}).
Complete(r)
ksvcFound, err := utils.IsCrdAvailable(mgr.GetConfig(), "serving.knative.dev/v1", "Service")
if err != nil {
return err
}

ctrlBuilder := ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1api.InferenceGraph{}).
Owns(&appsv1.Deployment{})

if ksvcFound {
ctrlBuilder = ctrlBuilder.Owns(&knservingv1.Service{})
} else {
return ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1api.InferenceGraph{}).
Owns(&knservingv1.Service{}).
Complete(r)
r.Log.Info("The InferenceGraph controller won't watch serving.knative.dev/v1/Service resources because the CRD is not available.")
}

return ctrlBuilder.Complete(r)
}
41 changes: 24 additions & 17 deletions pkg/controller/v1beta1/inferenceservice/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,26 +296,33 @@ func inferenceServiceStatusEqual(s1, s2 v1beta1api.InferenceServiceStatus, deplo
}

func (r *InferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager, deployConfig *v1beta1api.DeployConfig, ingressConfig *v1beta1api.IngressConfig) error {
if deployConfig.DefaultDeploymentMode == string(constants.RawDeployment) {
return ctrl.NewControllerManagedBy(mgr).
For(&v1beta1api.InferenceService{}).
Owns(&appsv1.Deployment{}).
Complete(r)
} else if ingressConfig.DisableIstioVirtualHost == false {
return ctrl.NewControllerManagedBy(mgr).
For(&v1beta1api.InferenceService{}).
Owns(&knservingv1.Service{}).
Owns(&istioclientv1beta1.VirtualService{}).
Owns(&appsv1.Deployment{}).
Complete(r)
ksvcFound, err := utils.IsCrdAvailable(mgr.GetConfig(), "serving.knative.dev/v1", "Service")
if err != nil {
return err
}

vsFound, err := utils.IsCrdAvailable(mgr.GetConfig(), "networking.istio.io/v1beta1", "VirtualService")
if err != nil {
return err
}

ctrlBuilder := ctrl.NewControllerManagedBy(mgr).
For(&v1beta1api.InferenceService{}).
Owns(&appsv1.Deployment{})

if ksvcFound {
ctrlBuilder = ctrlBuilder.Owns(&knservingv1.Service{})
} else {
r.Log.Info("The InferenceService controller won't watch serving.knative.dev/v1/Service resources because the CRD is not available.")
}

if vsFound && ingressConfig.DisableIstioVirtualHost == false {
ctrlBuilder = ctrlBuilder.Owns(&istioclientv1beta1.VirtualService{})
} else {
return ctrl.NewControllerManagedBy(mgr).
For(&v1beta1api.InferenceService{}).
Owns(&knservingv1.Service{}).
Owns(&appsv1.Deployment{}).
Complete(r)
r.Log.Info("The InferenceService controller won't watch networking.istio.io/v1beta1/VirtualService resources because the CRD is not available.")
}

return ctrlBuilder.Complete(r)
}

func (r *InferenceServiceReconciler) deleteExternalResources(isvc *v1beta1api.InferenceService) error {
Expand Down
44 changes: 44 additions & 0 deletions pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ import (

"github.com/kserve/kserve/pkg/constants"
v1 "k8s.io/api/core/v1"
apierr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/discovery"
"k8s.io/client-go/rest"
)

/* NOTE TO AUTHORS:
Expand All @@ -29,6 +33,8 @@ import (
* Please add functional style container operations sparingly and intentionally.
*/

var gvResourcesCache map[string]*metav1.APIResourceList

func Filter(origin map[string]string, predicate func(string) bool) map[string]string {
result := make(map[string]string)
for k, v := range origin {
Expand Down Expand Up @@ -169,3 +175,41 @@ func AppendPortIfNotExists(slice []v1.ContainerPort, elems ...v1.ContainerPort)
}
return slice
}

// IsCrdAvailable checks if a given CRD is present in the cluster by verifying the
// existence of its API.
func IsCrdAvailable(config *rest.Config, groupVersion, kind string) (bool, error) {
var gvResources *metav1.APIResourceList
var ok bool

if gvResourcesCache == nil {
gvResourcesCache = make(map[string]*metav1.APIResourceList)
}

if gvResources, ok = gvResourcesCache[groupVersion]; !ok {
discoveryClient, newClientErr := discovery.NewDiscoveryClientForConfig(config)
if newClientErr != nil {
return false, newClientErr
}

var getGvResourcesErr error
gvResources, getGvResourcesErr = discoveryClient.ServerResourcesForGroupVersion(groupVersion)
if getGvResourcesErr != nil && !apierr.IsNotFound(getGvResourcesErr) {
return false, getGvResourcesErr
}

gvResourcesCache[groupVersion] = gvResources
}

found := false
if gvResources != nil {
for _, crd := range gvResources.APIResources {
if crd.Kind == kind {
found = true
break
}
}
}

return found, nil
}

0 comments on commit b6bf6aa

Please sign in to comment.