Fix HPA related issues

Signed-off-by: jooho <jlee@redhat.com>

.github/workflows/run-fvt.yml

@@ -66,6 +66,7 @@ jobs:
           ./scripts/install.sh --namespace modelmesh-serving --fvt --dev-mode-logging
       - name: Free up disk space
         run: |
+          eval $(minikube -p minikube docker-env)
           echo "Pruning images"
           docker image prune -a -f
           docker system df
@@ -82,6 +83,7 @@ jobs:
           docker pull kserve/modelmesh
       - name: Check installation
         run: |
+          eval $(minikube -p minikube docker-env)
           docker images
           kubectl get pods
           kubectl get clusterservingruntimes
@@ -91,4 +93,4 @@ jobs:
           export PATH=/root/go/bin/:$PATH
           export NAMESPACE=modelmesh-serving
           export NAMESPACESCOPEMODE=false
-          make fvt
+          make fvt

.gitignore

@@ -33,4 +33,4 @@ bin
 # Modelmesh development related artifacts
 devbuild
 .develop_image_name
-.dev/
+.dev/

apis/serving/v1alpha1/servingruntime_webhook.go

@@ -17,12 +17,14 @@ package v1alpha1
 import (
 	"context"
 	"fmt"
+	"math"
 	"net/http"
 	"strconv"
 
 	kservev1alpha "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
 	"github.com/kserve/kserve/pkg/constants"
 	"github.com/kserve/modelmesh-serving/controllers/autoscaler"
+	mmcontstant "github.com/kserve/modelmesh-serving/pkg/constants"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
@@ -35,7 +37,8 @@ type ServingRuntimeWebhook struct {
 
 func (s *ServingRuntimeWebhook) Handle(ctx context.Context, req admission.Request) admission.Response {
 	var srAnnotations map[string]string
-	srReplicas := uint16(65535)
+	srReplicas := uint16(math.MaxUint16)
+	multiModel := false
 
 	if req.Kind.Kind == "ServingRuntime" {
 		servingRuntime := &kservev1alpha.ServingRuntime{}
@@ -44,19 +47,34 @@ func (s *ServingRuntimeWebhook) Handle(ctx context.Context, req admission.Reques
 			return admission.Errored(http.StatusBadRequest, err)
 		}
 		srAnnotations = servingRuntime.ObjectMeta.Annotations
+
 		if (*servingRuntime).Spec.Replicas != nil {
 			srReplicas = uint16(*servingRuntime.Spec.Replicas)
 		}
+
+		if (*servingRuntime).Spec.MultiModel != nil {
+			multiModel = *servingRuntime.Spec.MultiModel
+		}
+
 	} else {
 		clusterServingRuntime := &kservev1alpha.ClusterServingRuntime{}
 		err := s.decoder.Decode(req, clusterServingRuntime)
 		if err != nil {
 			return admission.Errored(http.StatusBadRequest, err)
 		}
 		srAnnotations = clusterServingRuntime.ObjectMeta.Annotations
+
 		if (*clusterServingRuntime).Spec.Replicas != nil {
 			srReplicas = uint16(*clusterServingRuntime.Spec.Replicas)
 		}
+
+		if (*clusterServingRuntime).Spec.MultiModel != nil {
+			multiModel = *clusterServingRuntime.Spec.MultiModel
+		}
+	}
+
+	if !multiModel {
+		return admission.Allowed("Not validating ServingRuntime because it is not ModelMesh compatible")
 	}
 
 	if err := validateServingRuntimeAutoscaler(srAnnotations); err != nil {
@@ -130,7 +148,7 @@ func validateAutoScalingReplicas(annotations map[string]string, srReplicas uint1
 
 	switch autoscalerClassType {
 	case string(constants.AutoscalerClassHPA):
-		if srReplicas != 65535 {
+		if srReplicas != math.MaxUint16 {
 			return fmt.Errorf("Autoscaler is enabled and also replicas variable set. You can not set both.")
 		}
 		return validateScalingHPA(annotations)
@@ -146,7 +164,7 @@ func validateScalingHPA(annotations map[string]string) error {
 	}
 
 	minReplicas := 1
-	if value, ok := annotations[constants.MinScaleAnnotationKey]; ok {
+	if value, ok := annotations[mmcontstant.MinScaleAnnotationKey]; ok {
 		if valueInt, err := strconv.Atoi(value); err != nil {
 			return fmt.Errorf("The min replicas should be a integer.")
 		} else if valueInt < 1 {
@@ -157,7 +175,7 @@ func validateScalingHPA(annotations map[string]string) error {
 	}
 
 	maxReplicas := 1
-	if value, ok := annotations[constants.MaxScaleAnnotationKey]; ok {
+	if value, ok := annotations[mmcontstant.MaxScaleAnnotationKey]; ok {
 		if valueInt, err := strconv.Atoi(value); err != nil {
 			return fmt.Errorf("The max replicas should be a integer.")
 		} else {

apis/serving/v1alpha1/servingruntime_webhook_test.go

@@ -16,15 +16,15 @@ limitations under the License.
 package v1alpha1
 
 import (
-	// "fmt"
-
+	"math"
 	"testing"
 
 	"github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	kservev1alpha "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
 	"github.com/kserve/kserve/pkg/constants"
+	mmcontstant "github.com/kserve/modelmesh-serving/pkg/constants"
 )
 
 func makeTestRawServingRuntime() kservev1alpha.ServingRuntime {
@@ -36,8 +36,8 @@ func makeTestRawServingRuntime() kservev1alpha.ServingRuntime {
 				"serving.kserve.io/autoscalerClass":             "hpa",
 				"serving.kserve.io/metrics":                     "cpu",
 				"serving.kserve.io/targetUtilizationPercentage": "75",
-				"autoscaling.knative.dev/min-scale":             "2",
-				"autoscaling.knative.dev/max-scale":             "3",
+				"serving.kserve.io/min-scale":                   "2",
+				"serving.kserve.io/max-scale":                   "3",
 			},
 		},
 	}
@@ -74,16 +74,16 @@ func TestInvalidAutoscalerTargetUtilizationPercentageHighValue(t *testing.T) {
 func TestInvalidAutoscalerLowMinReplicas(t *testing.T) {
 	g := gomega.NewGomegaWithT(t)
 	sr := makeTestRawServingRuntime()
-	sr.ObjectMeta.Annotations[constants.MinScaleAnnotationKey] = "0"
+	sr.ObjectMeta.Annotations[mmcontstant.MinScaleAnnotationKey] = "0"
 	g.Expect(validateScalingHPA(sr.Annotations)).ShouldNot(gomega.Succeed())
 }
 
 func TestInvalidAutoscalerMaxReplicasMustBiggerThanMixReplicas(t *testing.T) {
 	g := gomega.NewGomegaWithT(t)
 	sr := makeTestRawServingRuntime()
-	sr.ObjectMeta.Annotations[constants.MinScaleAnnotationKey] = "4"
-	sr.ObjectMeta.Annotations[constants.MaxScaleAnnotationKey] = "3"
-	g.Expect(validateAutoScalingReplicas(sr.Annotations, 65535)).ShouldNot(gomega.Succeed())
+	sr.ObjectMeta.Annotations[mmcontstant.MinScaleAnnotationKey] = "4"
+	sr.ObjectMeta.Annotations[mmcontstant.MaxScaleAnnotationKey] = "3"
+	g.Expect(validateAutoScalingReplicas(sr.Annotations, math.MaxUint16)).ShouldNot(gomega.Succeed())
 }
 func TestDuplicatedReplicas(t *testing.T) {
 	g := gomega.NewGomegaWithT(t)

config/certmanager/certificate.yaml

@@ -26,13 +26,13 @@ spec:
 apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:
-  name: serving-cert # this name should match the one appeared in kustomizeconfig.yaml
+  name: modelmesh-webhook-server-cert # this name should match the one appeared in kustomizeconfig.yaml
   namespace: system
 spec:
   # SERVICE_NAME_PLACEHOLDER and SERVICE_NAMESPACE_PLACEHOLDER will be substituted by kustomize
   dnsNames:
-    - SERVICE_NAME_PLACEHOLDER.SERVICE_NAMESPACE_PLACEHOLDER.svc
-    - SERVICE_NAME_PLACEHOLDER.SERVICE_NAMESPACE_PLACEHOLDER.svc.cluster.local
+    - $(SERVICE_NAME_PLACEHOLDER).$(SERVICE_NAMESPACE_PLACEHOLDER).svc
+    - $(SERVICE_NAME_PLACEHOLDER).$(SERVICE_NAMESPACE_PLACEHOLDER).svc.cluster.local
   issuerRef:
     kind: Issuer
     name: selfsigned-issuer

config/default/kustomization.yaml

@@ -11,111 +11,36 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-# Adds namespace to all resources.
-#namespace: model-serving
 
-# Value of this field is prepended to the
-# names of all resources, e.g. a deployment named
-# "wordpress" becomes "alices-wordpress".
-# Note that it should also match with the prefix (text before '-') of the namespace
-# field above.
-#namePrefix: model-serving-
-
-# Labels to add to all resources and selectors.
-#commonLabels:
-#  someName: someValue
-
-# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
-# crd/kustomization.yaml
-#- ../webhook
-# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required.
-#- ../certmanager
-# [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'.
-#- ../prometheus
-
-#patchesStrategicMerge:
-# Protect the /metrics endpoint by putting it behind auth.
-# If you want your controller-manager to expose the /metrics
-# endpoint w/o any authn/z, please comment the following line.
-#- manager_auth_proxy_patch.yaml
-
-# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
-# crd/kustomization.yaml
-#- manager_webhook_patch.yaml
-
-# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'.
-# Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks.
-# 'CERTMANAGER' needs to be enabled to use ca injection
-#- webhookcainjection_patch.yaml
-
-# the following config is for teaching kustomize how to do var substitution
-replacements:
-  - source:
+vars:
+  - fieldref:
       fieldPath: metadata.namespace
+    name: CERTIFICATE_NAMESPACE_PLACEHOLDER
+    objref:
+      group: cert-manager.io
       kind: Certificate
       name: serving-cert
-  - source:
+      version: v1
+  - fieldref: {}
+    name: CERTIFICATE_NAME_PLACEHOLDER
+    objref:
+      group: cert-manager.io
       kind: Certificate
       name: serving-cert
-  - source:
+      version: v1
+  - fieldref:
       fieldPath: metadata.namespace
+    name: SERVICE_NAMESPACE_PLACEHOLDER
+    objref:
       kind: Service
       name: modelmesh-webhook-server-service
-    targets:
-      - fieldPaths:
-          - |-
-            spec.# $(SERVICE_NAME) and $(SERVICE_NAMESPACE) will be substituted by kustomize
-            dnsNames.0
-        options:
-          delimiter: .
-          index: 1
-        select:
-          group: cert-manager.io
-          kind: Certificate
-          name: serving-cert
-          namespace: system
-          version: v1
-      - fieldPaths:
-          - |-
-            spec.# $(SERVICE_NAME) and $(SERVICE_NAMESPACE) will be substituted by kustomize
-            dnsNames.1
-        options:
-          delimiter: .
-          index: 1
-        select:
-          group: cert-manager.io
-          kind: Certificate
-          name: serving-cert
-          namespace: system
-          version: v1
-  - source:
+      version: v1
+  - fieldref: {}
+    name: SERVICE_NAME_PLACEHOLDER
+    objref:
       kind: Service
       name: modelmesh-webhook-server-service
-    targets:
-      - fieldPaths:
-          - |-
-            spec.# $(SERVICE_NAME) and SERVICE_NAMESPACE_PLACEHOLDER will be substituted by kustomize
-            dnsNames.0
-        options:
-          delimiter: .
-        select:
-          group: cert-manager.io
-          kind: Certificate
-          name: serving-cert
-          namespace: system
-          version: v1
-      - fieldPaths:
-          - |-
-            spec.# $(SERVICE_NAME) and SERVICE_NAMESPACE_PLACEHOLDER will be substituted by kustomize
-            dnsNames.1
-        options:
-          delimiter: .
-        select:
-          group: cert-manager.io
-          kind: Certificate
-          name: serving-cert
-          namespace: system
-          version: v1
+      version: v1
 
 configMapGenerator:
   - files:
@@ -135,7 +60,6 @@ resources:
   - ../webhook
   - ../certmanager
 
-patchesStrategicMerge:
-  - manager_webhook_patch.yaml
-  - webhookcainjection_patch.yaml
-namespace: modelmesh-serving
+patches:
+  - path: manager_webhook_patch.yaml
+  - path: webhookcainjection_patch.yaml

config/default/webhookcainjection_patch.yaml

@@ -12,10 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 # This patch add annotation to admission webhook config and
-# the variables $(CERTIFICATE_NAMESPACE) and $(CERTIFICATE_NAME) will be substituted by kustomize.
+# the string CERTIFICATE_NAMESPACE_PLACEHOLDER and CERTIFICATE_NAME_PLACEHOLDER will be replaced by kustomize.
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
   name: servingruntime.serving.kserve.io
   annotations:
-    cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME)
+    cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE_PLACEHOLDER)/$(CERTIFICATE_NAME_PLACEHOLDER)

config/rbac/cluster-scope/kustomization.yaml

@@ -17,4 +17,3 @@ resources:
   - role_binding.yaml
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
-namespace: modelmesh-serving

config/rbac/namespace-scope/kustomization.yaml

@@ -17,4 +17,3 @@ resources:
   - role_binding.yaml
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
-namespace: modelmesh-serving

config/webhook/kustomization.yaml

@@ -1,10 +1,20 @@
+# Copyright 2021 IBM Corporation
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
 ---
 resources:
   - manifests.yaml
   - service.yaml
 
-# Adds namespace to all resources.
-namespace: modelmesh-serving
-
 configurations:
   - kustomizeconfig.yaml