Skip to content

Commit

Permalink
Merge pull request #68 from VedantMahabaleshwarkar/crd_fix
Browse files Browse the repository at this point in the history
fix the need to have istio crds for modelmesh reconciliation
  • Loading branch information
VedantMahabaleshwarkar authored Aug 21, 2023
2 parents 0786ddc + 3d5089b commit 67a2df8
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 9 deletions.
4 changes: 0 additions & 4 deletions controllers/inferenceservice_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ import (
kservev1beta1 "github.com/kserve/modelmesh-serving/apis/serving/v1beta1"
routev1 "github.com/openshift/api/route/v1"
monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
istiosecv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
telemetryv1alpha1 "istio.io/client-go/pkg/apis/telemetry/v1alpha1"
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
authv1 "k8s.io/api/rbac/v1"
Expand Down Expand Up @@ -116,11 +114,9 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager)
Owns(&corev1.Service{}).
Owns(&corev1.Secret{}).
Owns(&authv1.ClusterRoleBinding{}).
Owns(&istiosecv1beta1.PeerAuthentication{}).
Owns(&networkingv1.NetworkPolicy{}).
Owns(&monitoringv1.ServiceMonitor{}).
Owns(&monitoringv1.PodMonitor{}).
Owns(&telemetryv1alpha1.Telemetry{}).
Watches(&source.Kind{Type: &predictorv1.ServingRuntime{}},
handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request {
r.Log.Info("Reconcile event triggered by serving runtime: " + o.GetName())
Expand Down
45 changes: 43 additions & 2 deletions controllers/kserve_inferenceservice_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
k8srbacv1 "k8s.io/api/rbac/v1"
apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/errors"
apierrs "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -50,8 +51,37 @@ const (
clusterPrometheusAccessRoleBinding = "kserve-prometheus-k8s"
clusterPrometheusAccessRole = "kserve-prometheus-k8s"
InferenceSercviceLabelName = "serving.kserve.io/inferenceservice"
TelemetryCRD = "telemetries.telemetry.istio.io"
PeerAuthCRD = "peerauthentications.security.istio.io"
)

func (r *OpenshiftInferenceServiceReconciler) foundKserveDependenyCRDs(ctx context.Context) bool {

allCRDsFound := true
// Check if CRD for Telemetry is installed
crd := &apiextv1.CustomResourceDefinition{}
err := r.Client.Get(ctx, client.ObjectKey{Name: TelemetryCRD}, crd)
if err != nil {
if apierrs.IsNotFound(err) {
allCRDsFound = false
} else {
allCRDsFound = false
}
}

// Check if CRD for PeerAuthentication is installed
err = r.Client.Get(ctx, client.ObjectKey{Name: PeerAuthCRD}, crd)
if err != nil {
if apierrs.IsNotFound(err) {
allCRDsFound = false
} else {
allCRDsFound = false
}
}

return allCRDsFound
}

func (r *OpenshiftInferenceServiceReconciler) ensurePrometheusRoleBinding(ctx context.Context, req ctrl.Request, ns string) error {
// Initialize logger format
log := r.Log.WithValues("namespace", ns)
Expand Down Expand Up @@ -470,6 +500,12 @@ func (r *OpenshiftInferenceServiceReconciler) DeleteKserveMetricsResourcesIfNoKs
// Initialize logger format
log := r.Log.WithValues("namespace", ns)

//Check if the dependent CRDs for Kserve are installed
if !r.foundKserveDependenyCRDs(ctx) {
log.Info("Missing Istio CRDs")
return nil
}

inferenceServiceList := &kservev1beta1.InferenceServiceList{}
err := r.List(ctx, inferenceServiceList, client.InNamespace(req.Namespace))
if err != nil {
Expand Down Expand Up @@ -588,8 +624,13 @@ func (r *OpenshiftInferenceServiceReconciler) ReconcileKserveInference(ctx conte
// Initialize logger format
log := r.Log.WithValues("InferenceService", inferenceService.Name, "namespace", inferenceService.Namespace)

//Also create the metrics service and servicemonitor with OwnerReferences, as these are not common namespace-scope resources
//Create the metrics service and servicemonitor first, then ensure all namespace-scoped resources exist.
//Check if the dependent CRDs for Kserve are installed
if !r.foundKserveDependenyCRDs(ctx) {
log.Info("Missing Istio CRDs")
return nil
}

//Create the metrics service and servicemonitor with OwnerReferences, as these are not common namespace-scope resources
log.Info("Reconciling Metrics Service for InferenceSercvice")
err := r.createOrUpdateMetricsService(ctx, req, inferenceService)
if err != nil {
Expand Down
4 changes: 1 addition & 3 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@ import (
inferenceservicev1 "github.com/kserve/modelmesh-serving/apis/serving/v1beta1"
mf "github.com/manifestival/manifestival"
monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
corev1 "k8s.io/api/core/v1"
k8srbacv1 "k8s.io/api/rbac/v1"

"go.uber.org/zap/zapcore"
k8srbacv1 "k8s.io/api/rbac/v1"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
ctrl "sigs.k8s.io/controller-runtime"

Expand Down

0 comments on commit 67a2df8

Please sign in to comment.