fix: CappConfig types mismatch

dana-team · Dec 3, 2024 · 33ba9a3 · 33ba9a3
1 parent 5e9b1fb
commit 33ba9a3
Show file tree

Hide file tree

Showing 13 changed files with 131 additions and 105 deletions.
diff --git a/README.md b/README.md
@@ -148,16 +148,16 @@ metadata:
   namespace: capp-operator-system
 spec:
   autoscaleConfig:
-    rps: "200"
-    cpu: "80"
-    memory: "70"
-    concurrency: "10"
-    activationScale: "3"
+    rps: 200
+    cpu: 80
+    memory: 70
+    concurrency: 10
+    activationScale: 3
   dnsConfig:
-    zone: capp-zone.com.
-    cname: ingress.capp-zone.com.
-    provider: dns-default
-    issuer: cert-issuer
+    zone: "capp-zone.com."
+    cname: "ingress.capp-zone.com."
+    provider: "dns-default"
+    issuer: "cert-issuer"
 
 ```
 

diff --git a/api/v1alpha1/cappconfig_types.go b/api/v1alpha1/cappconfig_types.go
@@ -4,23 +4,34 @@ import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 // CappConfigSpec defines the desired state of CappConfig
 type CappConfigSpec struct {
-	DNSConfig       DNSConfig       `json:"dnsConfig"`
+	// +kubebuilder:validation:Required
+	DNSConfig DNSConfig `json:"dnsConfig"`
+	// +kubebuilder:validation:Required
 	AutoscaleConfig AutoscaleConfig `json:"autoscaleConfig"`
 }
 
 type DNSConfig struct {
-	Zone     string `json:"zone"`
-	CNAME    string `json:"cname"`
+	//Zone defines the DNS zone for Capp Hostnames
+	Zone string `json:"zone"`
+	//CNAME defines the CNAME record that will be used for Capp Hostnames
+	CNAME string `json:"cname"`
+	//Provider defines the DNS provider
 	Provider string `json:"provider"`
-	Issuer   string `json:"issuer"`
+	//Issuer defines the certificate issuer
+	Issuer string `json:"issuer"`
 }
 
 type AutoscaleConfig struct {
-	RPS             string `json:"rps"`
-	CPU             string `json:"cpu"`
-	Memory          string `json:"memory"`
-	Concurrency     string `json:"concurrency"`
-	ActivationScale string `json:"activationScale"`
+	// RPS is the desired requests per second to trigger upscaling.
+	RPS int `json:"rps"`
+	// CPU is the desired CPU utilization to trigger upscaling.
+	CPU int `json:"cpu"`
+	// Memory is the desired memory utilization to trigger upscaling.
+	Memory int `json:"memory"`
+	// Concurrency is the maximum concurrency of a Capp.
+	Concurrency int `json:"concurrency"`
+	// ActivationScale is the default scale.
+	ActivationScale int `json:"activationScale"`
 }
 
 // CappConfigStatus defines the observed state of CappConfig

diff --git a/charts/container-app-operator/crds/rcs.dana.io_cappconfigs.yaml b/charts/container-app-operator/crds/rcs.dana.io_cappconfigs.yaml
@@ -42,15 +42,22 @@ spec:
                 autoscaleConfig:
                   properties:
                     activationScale:
-                      type: string
+                      description: ActivationScale is the default scale.
+                      type: integer
                     concurrency:
-                      type: string
+                      description: Concurrency is the maximum concurrency of a Capp.
+                      type: integer
                     cpu:
-                      type: string
+                      description: CPU is the desired CPU utilization to trigger upscaling.
+                      type: integer
                     memory:
-                      type: string
+                      description: Memory is the desired memory utilization to trigger
+                        upscaling.
+                      type: integer
                     rps:
-                      type: string
+                      description: RPS is the desired requests per second to trigger
+                        upscaling.
+                      type: integer
                   required:
                     - activationScale
                     - concurrency
@@ -61,12 +68,17 @@ spec:
                 dnsConfig:
                   properties:
                     cname:
+                      description: CNAME defines the CNAME record that will be used
+                        for Capp Hostnames
                       type: string
                     issuer:
+                      description: Issuer defines the certificate issuer
                       type: string
                     provider:
+                      description: Provider defines the DNS provider
                       type: string
                     zone:
+                      description: Zone defines the DNS zone for Capp Hostnames
                       type: string
                   required:
                     - cname

diff --git a/charts/container-app-operator/templates/manager-rbac-cluster-role.yaml b/charts/container-app-operator/templates/manager-rbac-cluster-role.yaml
@@ -17,36 +17,39 @@ rules:
 - apiGroups:
   - ""
   resources:
-  - events
   - secrets
   verbs:
   - create
+  - delete
   - get
   - list
   - patch
   - update
   - watch
 - apiGroups:
-  - cert-manager.io
+  - ""
+  - events.k8s.io
   resources:
-  - certificates
+  - events
   verbs:
   - create
   - delete
   - get
   - list
+  - patch
   - update
   - watch
 - apiGroups:
-  - events.k8s.io
+  - cert-manager.io
   resources:
-  - events
+  - certificates
   verbs:
   - create
+  - delete
   - get
   - list
-  - patch
   - update
+  - patch
   - watch
 - apiGroups:
   - logging.banzaicloud.io
@@ -71,6 +74,14 @@ rules:
   - list
   - update
   - watch
+- apiGroups:
+  - rcs.dana.io
+  resources:
+  - cappconfigs
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - rcs.dana.io
   resources:
@@ -141,4 +152,4 @@ rules:
   - get
   - list
   - update
-  - watch
+  - watch
diff --git a/charts/container-app-operator/values.yaml b/charts/container-app-operator/values.yaml
@@ -151,12 +151,12 @@ config:
       issuer: cert-issuer
     autoscaleConfig:
       # -- The default Requests Per Second (RPS) threshold for autoscaling.
-      rps: "200"
+      rps: 200
       # -- The default CPU utilization percentage for autoscaling.
-      cpu: "80"
+      cpu: 80
       # -- The default memory utilization percentage for autoscaling.
-      memory: "70"
+      memory: 70
       # -- The default concurrency limit for autoscaling.
-      concurrency: "10"
+      concurrency: 10
       # -- The default activationScale for autoscaling.
-      activationScale: "3"
+      activationScale: 3
diff --git a/config/crd/bases/rcs.dana.io_cappconfigs.yaml b/config/crd/bases/rcs.dana.io_cappconfigs.yaml
@@ -42,15 +42,22 @@ spec:
               autoscaleConfig:
                 properties:
                   activationScale:
-                    type: string
+                    description: ActivationScale is the default scale.
+                    type: integer
                   concurrency:
-                    type: string
+                    description: Concurrency is the maximum concurrency of a Capp.
+                    type: integer
                   cpu:
-                    type: string
+                    description: CPU is the desired CPU utilization to trigger upscaling.
+                    type: integer
                   memory:
-                    type: string
+                    description: Memory is the desired memory utilization to trigger
+                      upscaling.
+                    type: integer
                   rps:
-                    type: string
+                    description: RPS is the desired requests per second to trigger
+                      upscaling.
+                    type: integer
                 required:
                 - activationScale
                 - concurrency
@@ -61,12 +68,17 @@ spec:
               dnsConfig:
                 properties:
                   cname:
+                    description: CNAME defines the CNAME record that will be used
+                      for Capp Hostnames
                     type: string
                   issuer:
+                    description: Issuer defines the certificate issuer
                     type: string
                   provider:
+                    description: Provider defines the DNS provider
                     type: string
                   zone:
+                    description: Zone defines the DNS zone for Capp Hostnames
                     type: string
                 required:
                 - cname

diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
@@ -75,6 +75,13 @@ rules:
   - rcs.dana.io
   resources:
   - cappconfigs
+  verbs:
+  - get
+  - list
+  - watch
+- apiGroups:
+  - rcs.dana.io
+  resources:
   - capprevisions
   - capps
   verbs:
@@ -142,4 +149,4 @@ rules:
   - get
   - list
   - update
-  - watch
+  - watch
diff --git a/hack/manifests/cappconfig.yaml b/hack/manifests/cappconfig.yaml
@@ -5,13 +5,13 @@ metadata:
   namespace: capp-operator-system
 spec:
   autoscaleConfig:
-    rps: "200"
-    cpu: "80"
-    memory: "70"
-    concurrency: "10"
-    activationScale: "3"
+    rps: 200
+    cpu: 80
+    memory: 70
+    concurrency: 10
+    activationScale: 3
   dnsConfig:
-    zone: capp-zone.com.
-    cname: ingress.capp-zone.com.
-    provider: dns-default
-    issuer: cert-issuer
+    zone: "capp-zone.com."
+    cname: "ingress.capp-zone.com."
+    provider: "dns-default"
+    issuer: "cert-issuer"
diff --git a/internal/kinds/capp/autoscale/autoscale.go b/internal/kinds/capp/autoscale/autoscale.go
@@ -1,6 +1,8 @@
 package autoscale
 
 import (
+	"fmt"
+
 	cappv1alpha1 "github.com/dana-team/container-app-operator/api/v1alpha1"
 	"github.com/dana-team/container-app-operator/internal/kinds/capp/utils"
 	"k8s.io/utils/strings/slices"
@@ -20,11 +22,11 @@ const (
 )
 
 var TargetDefaultValues = cappv1alpha1.AutoscaleConfig{
-	RPS:             "200",
-	CPU:             "80",
-	Memory:          "70",
-	Concurrency:     "10",
-	ActivationScale: "3",
+	RPS:             200,
+	CPU:             80,
+	Memory:          70,
+	Concurrency:     10,
+	ActivationScale: 3,
 }
 
 var KPAMetrics = []string{"rps", "concurrency"}
@@ -48,7 +50,7 @@ func SetAutoScaler(capp cappv1alpha1.Capp, defaults cappv1alpha1.AutoscaleConfig
 	autoScaleAnnotations[KnativeAutoscaleClassKey] = getAutoScaleClassByMetric(scaleMetric)
 	autoScaleAnnotations[KnativeMetricKey] = scaleMetric
 	autoScaleAnnotations[KnativeAutoscaleTargetKey] = getTargetValue(scaleMetric, defaults)
-	autoScaleAnnotations[KnativeActivationScaleKey] = activationScale
+	autoScaleAnnotations[KnativeActivationScaleKey] = fmt.Sprintf("%d", activationScale)
 	autoScaleAnnotations = utils.MergeMaps(autoScaleAnnotations, givenAutoScaleAnnotation)
 
 	return autoScaleAnnotations
@@ -60,13 +62,13 @@ func getTargetValue(scaleMetric string, autoscale cappv1alpha1.AutoscaleConfig)
 	var targetValue string
 	switch scaleMetric {
 	case rpsScaleKey:
-		targetValue = autoscale.RPS
+		targetValue = fmt.Sprintf("%d", autoscale.RPS)
 	case cpuScaleKey:
-		targetValue = autoscale.CPU
+		targetValue = fmt.Sprintf("%d", autoscale.CPU)
 	case memoryScaleKey:
-		targetValue = autoscale.Memory
+		targetValue = fmt.Sprintf("%d", autoscale.Memory)
 	case concurrencyScaleKey:
-		targetValue = autoscale.Concurrency
+		targetValue = fmt.Sprintf("%d", autoscale.Concurrency)
 	default:
 		targetValue = "" // handle unknown scale metrics
 	}
@@ -75,7 +77,7 @@ func getTargetValue(scaleMetric string, autoscale cappv1alpha1.AutoscaleConfig)
 
 // isAutoScaleEmpty checks if all the values of the AutoscaleConfig are empty.
 func isAutoScaleEmpty(config cappv1alpha1.AutoscaleConfig) bool {
-	return config.RPS == "" && config.CPU == "" && config.Memory == "" && config.Concurrency == "" && config.ActivationScale == ""
+	return config.RPS == 0 && config.CPU == 0 && config.Memory == 0 && config.Concurrency == 0 && config.ActivationScale == 0
 }
 
 // Determines the autoscaling class based on the metric provided. Returns "kpa.autoscaling.knative.dev" if the metric is in KPAMetrics, "hpa.autoscaling.knative.dev" otherwise.