Sync kserve/main (v0.11.1) to odh/main

.github/workflows/fvt.yml

@@ -1,6 +1,7 @@
 name: FVT
 
 on:
+  workflow_dispatch:
   pull_request:
     branches:
       - main

.golangci.yaml

@@ -243,11 +243,6 @@ issues:
         - staticcheck
       text: "SA9003:"
 
-    # Exclude some deprecation errors
-    - linters:
-        - staticcheck
-      text: "SA1019:"
-
     # Exclude lll issues for long lines with go:generate
     - linters:
         - lll

config/crd/bases/serving.kserve.io_clusterservingruntimes.yaml

@@ -1,4 +1,4 @@
-# Copied from https://github.com/kserve/kserve/blob/v0.11.0/config/crd/serving.kserve.io_clusterservingruntimes.yaml
+# Copied from https://github.com/kserve/kserve/blob/v0.11.1/config/crd/serving.kserve.io_clusterservingruntimes.yaml
 ---
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
@@ -1133,6 +1133,10 @@ spec:
                         type: boolean
                       name:
                         type: string
+                      priority:
+                        format: int32
+                        minimum: 1
+                        type: integer
                       version:
                         type: string
                     required:

config/crd/bases/serving.kserve.io_inferenceservices.yaml

@@ -1,4 +1,4 @@
-# Copied from https://github.com/kserve/kserve/blob/v0.11.0/config/crd/serving.kserve.io_inferenceservices.yaml
+# Copied from https://github.com/kserve/kserve/blob/v0.11.1/config/crd/serving.kserve.io_inferenceservices.yaml
 ---
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition

config/crd/bases/serving.kserve.io_servingruntimes.yaml

@@ -1,4 +1,4 @@
-# Copied from https://github.com/kserve/kserve/blob/v0.11.0/config/crd/serving.kserve.io_servingruntimes.yaml
+# Copied from https://github.com/kserve/kserve/blob/v0.11.1/config/crd/serving.kserve.io_servingruntimes.yaml
 ---
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
@@ -1133,6 +1133,10 @@ spec:
                         type: boolean
                       name:
                         type: string
+                      priority:
+                        format: int32
+                        minimum: 1
+                        type: integer
                       version:
                         type: string
                     required:

controllers/modelmesh/constraints.go

@@ -18,6 +18,8 @@ import (
 	"errors"
 	"strings"
 
+	"k8s.io/apimachinery/pkg/util/sets"
+
 	kserveapi "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -68,5 +70,5 @@ func (m *Deployment) addModelTypeConstraints(deployment *appsv1.Deployment) erro
 }
 
 func generateLabelsEnvVar(rts *kserveapi.ServingRuntimeSpec, restProxyEnabled bool, rtName string) string {
-	return strings.Join(GetServingRuntimeLabelSet(rts, restProxyEnabled, rtName).List(), ",")
+	return strings.Join(sets.List(GetServingRuntimeLabelSet(rts, restProxyEnabled, rtName)), ",")
 }

controllers/modelmesh/model_type_labels.go

@@ -24,10 +24,10 @@ import (
 )
 
 func GetServingRuntimeLabelSets(rt *kserveapi.ServingRuntimeSpec, restProxyEnabled bool, rtName string) (
-	mtLabels sets.String, pvLabels sets.String, rtLabel string) {
+	mtLabels sets.Set[string], pvLabels sets.Set[string], rtLabel string) {
 
 	// model type labels
-	mtSet := make(sets.String, 2*len(rt.SupportedModelFormats))
+	mtSet := make(sets.Set[string], 2*len(rt.SupportedModelFormats))
 	for _, t := range rt.SupportedModelFormats {
 		// only include model type labels when autoSelect is true
 		if t.AutoSelect != nil && *t.AutoSelect {
@@ -38,7 +38,7 @@ func GetServingRuntimeLabelSets(rt *kserveapi.ServingRuntimeSpec, restProxyEnabl
 		}
 	}
 	// protocol versions
-	pvSet := make(sets.String, len(rt.ProtocolVersions))
+	pvSet := make(sets.Set[string], len(rt.ProtocolVersions))
 	for _, pv := range rt.ProtocolVersions {
 		pvSet.Insert(fmt.Sprintf("pv:%s", pv))
 		if restProxyEnabled && pv == constants.ProtocolGRPCV2 {
@@ -49,7 +49,7 @@ func GetServingRuntimeLabelSets(rt *kserveapi.ServingRuntimeSpec, restProxyEnabl
 	return mtSet, pvSet, fmt.Sprintf("rt:%s", rtName)
 }
 
-func GetServingRuntimeLabelSet(rt *kserveapi.ServingRuntimeSpec, restProxyEnabled bool, rtName string) sets.String {
+func GetServingRuntimeLabelSet(rt *kserveapi.ServingRuntimeSpec, restProxyEnabled bool, rtName string) sets.Set[string] {
 	s1, s2, l := GetServingRuntimeLabelSets(rt, restProxyEnabled, rtName)
 	s1 = s1.Union(s2)
 	s1.Insert(l)

controllers/modelmesh/model_type_labels_test.go

@@ -19,6 +19,8 @@ import (
 	"sort"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/util/sets"
+
 	kserveapi "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
 	"github.com/kserve/kserve/pkg/constants"
 	api "github.com/kserve/modelmesh-serving/apis/serving/v1alpha1"
@@ -82,11 +84,11 @@ func TestGetServingRuntimeLabelSets(t *testing.T) {
 	if expectedRtLabel != rtLabel {
 		t.Errorf("Missing expected entry [%s] in set: %v", expectedRtLabel, rtLabel)
 	}
-	if !reflect.DeepEqual(mtLabelSet.List(), expectedMtLabels) {
-		t.Errorf("Labels [%s] don't match expected: %v", mtLabelSet.List(), expectedMtLabels)
+	if !reflect.DeepEqual(sets.List(mtLabelSet), expectedMtLabels) {
+		t.Errorf("Labels [%s] don't match expected: %v", sets.List(mtLabelSet), expectedMtLabels)
 	}
-	if !reflect.DeepEqual(pvLabelSet.List(), expectedPvLabels) {
-		t.Errorf("Labels [%s] don't match expected: %v", pvLabelSet.List(), expectedPvLabels)
+	if !reflect.DeepEqual(sets.List(pvLabelSet), expectedPvLabels) {
+		t.Errorf("Labels [%s] don't match expected: %v", sets.List(pvLabelSet), expectedPvLabels)
 	}
 }
 

controllers/service_controller.go

@@ -119,24 +119,40 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		namespace = req.Name
 		n := &corev1.Namespace{}
 		if err := r.Client.Get(ctx, req.NamespacedName, n); err != nil {
+			// Previously, the controller kept checking namespaces even though the namespaces do not exist anymore.
+			// As a result, a lot of misleading error messages showed up in the log
+			if k8serr.IsNotFound(err) {
+				err = nil
+			}
 			return ctrl.Result{}, err
 		}
 		if !modelMeshEnabled(n, r.ControllerDeployment.Namespace) {
 			sl := &corev1.ServiceList{}
-			err := r.List(ctx, sl, client.HasLabels{"modelmesh-service"}, client.InNamespace(namespace))
-			if err == nil {
+			//The logic is
+			// - If the namespace is not for modelmesh anymore, it will delete modelmesh Service when it exists.
+			// - If the namespace is being terminated, it does not need to delete the modelmesh Service because it will be gone with the namespace
+			if err := r.List(ctx, sl, client.HasLabels{"modelmesh-service"}, client.InNamespace(namespace)); err != nil {
+				return ctrl.Result{}, err
+			} else {
 				for i := range sl.Items {
 					s := &sl.Items[i]
-					if err2 := r.Delete(ctx, s); err2 != nil && err == nil {
-						err = err2
+					if err := r.Delete(ctx, s); err != nil {
+						return ctrl.Result{}, err
 					}
 				}
 			}
+
 			if mms := r.MMServices.Get(namespace); mms != nil {
 				mms.Disconnect()
 				r.MMServices.Delete(namespace)
+				//requeue is never expected here
+				//If the namespace is not for modelmesh anymore, it should trigger reconcileService for MMService list that manages the goroutines.
+				if _, err, _ := r.reconcileService(ctx, mms, namespace, owner); err != nil {
+					return ctrl.Result{}, err
+				}
 			}
-			return ctrl.Result{}, err
+
+			return ctrl.Result{}, nil
 		}
 		owner = n
 	} else {
@@ -222,10 +238,27 @@ func (r *ServiceReconciler) reconcileService(ctx context.Context, mms *mmesh.MMS
 		return nil, errors.New("unexpected state - MMService uninitialized"), false
 	}
 
+	if r.ClusterScope {
+		namespaceObj := &corev1.Namespace{}
+		// Get the namespace object to check label and state of the namespace
+		if err := r.Client.Get(ctx, types.NamespacedName{Name: namespace}, namespaceObj); err != nil {
+			return nil, err, false
+		}
+		// This will remove the goroutine when modelmesh is not enabled for a namespace.
+		//  - when the namespace does not have the annotation modelmesh-enabled
+		//  - when the namespace is under a Terminating state.
+		if !modelMeshEnabled(namespaceObj, r.ControllerDeployment.Namespace) {
+			r.ModelEventStream.RemoveWatchedService(serviceName, namespace)
+			r.Log.V(1).Info("Deleted Watched Service", "name", serviceName, "namespace", namespace)
+			return nil, nil, false
+		}
+	}
+
 	sl := &corev1.ServiceList{}
 	if err := r.List(ctx, sl, client.HasLabels{"modelmesh-service"}, client.InNamespace(namespace)); err != nil {
 		return nil, err, false
 	}
+
 	var s *corev1.Service
 	for i := range sl.Items {
 		ss := &sl.Items[i]

controllers/servingruntime_validator.go

@@ -140,7 +140,7 @@ func validateVolumes(rts *kserveapi.ServingRuntimeSpec, _ *config.Config) error
 	return nil
 }
 
-func checkName(name string, internalNames sets.String, logStr string) error {
+func checkName(name string, internalNames sets.Set[string], logStr string) error {
 	if internalNames.Has(name) {
 		return fmt.Errorf("%s %s is reserved for internal use", logStr, name)
 	}
@@ -151,29 +151,29 @@ func checkName(name string, internalNames sets.String, logStr string) error {
 	return nil
 }
 
-var internalContainerNames = sets.NewString(
+var internalContainerNames = sets.New[string](
 	modelmesh.ModelMeshContainerName,
 	modelmesh.RESTProxyContainerName,
 	modelmesh.PullerContainerName,
 )
 
-var internalOnlyVolumeMounts = sets.NewString(
+var internalOnlyVolumeMounts = sets.New[string](
 	modelmesh.ConfigStorageMount,
 	modelmesh.EtcdVolume,
 	modelmesh.InternalConfigMapName,
 	modelmesh.SocketVolume,
 )
 
-var internalNamedPorts = sets.NewString("grpc", "http", "prometheus")
+var internalNamedPorts = sets.New[string]("grpc", "http", "prometheus")
 
-var internalPorts = sets.NewInt32(
+var internalPorts = sets.New[int32](
 	8080, // is used for LiteLinks communication in Model Mesh
 	8085, // is the port the built-in adapter listens on
 	8089, // is used for Model Mesh probes
 	8090, // is used for default preStop hooks
 )
 
-var internalVolumes = sets.NewString(
+var internalVolumes = sets.New[string](
 	modelmesh.ConfigStorageMount,
 	modelmesh.EtcdVolume,
 	modelmesh.InternalConfigMapName,

controllers/util.go

@@ -18,12 +18,17 @@ import (
 	"context"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func modelMeshEnabled(n *corev1.Namespace, controllerNamespace string) bool {
 	if v, ok := n.Labels["modelmesh-enabled"]; ok {
+		// Returns false if the namespace state is terminating even though the namespace have the 'modelmesh-enabled=true' label.
+		if n.Status.Phase == corev1.NamespaceTerminating {
+			return false
+		}
 		return v == "true"
 	}
 	return n.Name == controllerNamespace
@@ -38,7 +43,10 @@ func modelMeshEnabled2(ctx context.Context, namespace, controllerNamespace strin
 	}
 	n := &corev1.Namespace{}
 	if err := client.Get(ctx, types.NamespacedName{Name: namespace}, n); err != nil {
-		return false, err
+		if errors.IsNotFound(err) {
+			// If the namespace has already been deleted, it can not be modelmesh namespace
+			return false, nil
+		}
 	}
 	return modelMeshEnabled(n, controllerNamespace), nil
 }

fvt/fvtclient.go

@@ -59,8 +59,8 @@ import (
 	torchserveapi "github.com/kserve/modelmesh-serving/fvt/generated/torchserve/apis"
 )
 
-const PredictorTimeout = time.Second * 120       // absolute time to wait for predictor to become ready
-const TimeForStatusToStabilize = time.Second * 5 // time to wait between watcher events before assuming a stable state
+const PredictorTimeout = time.Second * 120        // absolute time to wait for predictor to become ready
+const TimeForStatusToStabilize = time.Second * 10 // time to wait between watcher events before assuming a stable state
 
 type ModelServingConnectionType int
 

fvt/predictor/predictor_suite_test.go

@@ -56,7 +56,7 @@ var _ = SynchronizedBeforeSuite(func() []byte {
 	FVTClientInstance.CreateTLSSecrets()
 
 	// ensure a stable deploy state
-	WaitForStableActiveDeployState(time.Second * 30)
+	WaitForStableActiveDeployState(time.Second * 45)
 
 	return nil
 }, func(_ []byte) {

fvt/storage/storage_test.go

@@ -130,11 +130,21 @@ var _ = Describe("ISVCs", func() {
 			//   from the old to the new pod
 
 			// make a shallow copy of default configmap (don't modify the DefaultConfig reference)
-			// keeping 1 pod per runtime and don't scale to 0
 			config := make(map[string]interface{})
 			for k, v := range DefaultConfig {
 				config[k] = v
 			}
+
+			// scale to 0 for resource-constrained environments (only 2 CPUs on GH actions)
+			// to stop and remove runtimes which are not used for this test
+			//   Warning   FailedScheduling   pod/modelmesh-serving-mlserver-1.x-...
+			//     0/1 nodes are available: 1 Insufficient cpu. preemption: 0/1 nodes are available:
+			//       1 No preemption victims found for incoming pod.
+			config["scaleToZero"] = map[string]interface{}{
+				"enabled":            true,
+				"gracePeriodSeconds": 5,
+			}
+
 			// update the model-serving-config to allow any PVC
 			config["allowAnyPVC"] = true
 
@@ -194,11 +204,21 @@ var _ = Describe("ISVCs", func() {
 
 		It("should fail with non-existent PVC", func() {
 			// make a shallow copy of default configmap (don't modify the DefaultConfig reference)
-			// keeping 1 pod per runtime and don't scale to 0
 			config := make(map[string]interface{})
 			for k, v := range DefaultConfig {
 				config[k] = v
 			}
+
+			// scale to 0 for resource-constrained environments (only 2 CPUs on GH actions)
+			// to stop and remove runtimes which are not used for this test
+			//   Warning   FailedScheduling   pod/modelmesh-serving-mlserver-1.x-...
+			//     0/1 nodes are available: 1 Insufficient cpu. preemption: 0/1 nodes are available:
+			//       1 No preemption victims found for incoming pod.
+			config["scaleToZero"] = map[string]interface{}{
+				"enabled":            true,
+				"gracePeriodSeconds": 5,
+			}
+
 			// update the model-serving-config to allow any PVC
 			config["allowAnyPVC"] = true
 			FVTClientInstance.ApplyUserConfigMap(config)

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/go-logr/logr v1.2.4
 	github.com/golang/protobuf v1.5.3
 	github.com/google/go-cmp v0.5.9
-	github.com/kserve/kserve v0.11.0
+	github.com/kserve/kserve v0.11.1
 	github.com/manifestival/controller-runtime-client v0.4.0
 	github.com/manifestival/manifestival v0.7.1
 	github.com/moverest/mnist v0.0.0-20160628192128-ec5d9d203b59
@@ -98,12 +98,12 @@ require (
 	go.opencensus.io v0.24.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.24.0 // indirect
-	golang.org/x/crypto v0.9.0 // indirect
-	golang.org/x/net v0.10.0 // indirect
+	golang.org/x/crypto v0.12.0 // indirect
+	golang.org/x/net v0.14.0 // indirect
 	golang.org/x/oauth2 v0.8.0 // indirect
-	golang.org/x/sys v0.8.0 // indirect
-	golang.org/x/term v0.8.0 // indirect
-	golang.org/x/text v0.9.0 // indirect
+	golang.org/x/sys v0.11.0 // indirect
+	golang.org/x/term v0.11.0 // indirect
+	golang.org/x/text v0.12.0 // indirect
 	golang.org/x/time v0.3.0 // indirect
 	golang.org/x/tools v0.9.1 // indirect
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect