Fix autoscaling bug and update HPA sample

api/v1alpha1/spinapp_types.go

@@ -46,14 +46,6 @@ type SpinAppSpec struct {
 	// Number of replicas to run.
 	Replicas int32 `json:"replicas,omitempty"`
 
-	// EnableAutoscaling indicates whether the app is allowed to autoscale. If
-	// true then the operator leaves the replica count of the underlying
-	// deployment to be managed by an external autoscaler (HPA/KEDA). Replicas
-	// cannot be defined if this is enabled. By default EnableAutoscaling is false.
-	//
-	// +kubebuilder:default:=false
-	EnableAutoscaling bool `json:"enableAutoscaling,omitempty"`
-
 	// RuntimeConfig defines configuration to be applied at runtime for this app.
 	RuntimeConfig RuntimeConfig `json:"runtimeConfig,omitempty"`
 
@@ -99,6 +91,9 @@ type SpinAppStatus struct {
 
 	// Represents the current number of active replicas on the application deployment.
 	ReadyReplicas int32 `json:"readyReplicas"`
+
+	// Selector for the underlying pods
+	Selector string `json:"selector"`
 }
 
 // SpinApp is the Schema for the spinapps API
@@ -108,6 +103,7 @@ type SpinAppStatus struct {
 // +kubebuilder:printcolumn:JSONPath=".status.readyReplicas",name=Ready,type=integer
 // +kubebuilder:printcolumn:JSONPath=".spec.replicas",name=Desired,type=integer
 // +kubebuilder:printcolumn:JSONPath=".spec.executor",name=Executor,type=string
+// +kubebuilder:subresource:scale:specpath=.spec.replicas,statuspath=.status.replicas,selectorpath=.status.selector
 type SpinApp struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

config/crd/bases/core.spinoperator.dev_spinapps.yaml

@@ -194,14 +194,6 @@ spec:
                 description: DeploymentAnnotations defines annotations to be applied
                   to the underlying deployment.
                 type: object
-              enableAutoscaling:
-                default: false
-                description: |-
-                  EnableAutoscaling indicates whether the app is allowed to autoscale. If
-                  true then the operator leaves the replica count of the underlying
-                  deployment to be managed by an external autoscaler (HPA/KEDA). Replicas
-                  cannot be defined if this is enabled. By default EnableAutoscaling is false.
-                type: boolean
               executor:
                 description: |-
                   Executor controls how this app is executed in the cluster.
@@ -2577,11 +2569,19 @@ spec:
                   application deployment.
                 format: int32
                 type: integer
+              selector:
+                description: Selector for the underlying pods
+                type: string
             required:
             - readyReplicas
+            - selector
             type: object
         type: object
     served: true
     storage: true
     subresources:
+      scale:
+        labelSelectorPath: .status.selector
+        specReplicasPath: .spec.replicas
+        statusReplicasPath: .status.replicas
       status: {}

config/samples/hpa.yaml

@@ -5,7 +5,7 @@ metadata:
 spec:
   image: ghcr.io/spinkube/spin-operator/cpu-load-gen:20240311-163328-g1121986
   executor: containerd-shim-spin
-  enableAutoscaling: true
+  replicas: 1
   resources:
     limits:
       cpu: 500m
@@ -20,8 +20,8 @@ metadata:
   name: spinapp-autoscaler
 spec:
   scaleTargetRef:
-    apiVersion: apps/v1
-    kind: Deployment
+    apiVersion: core.spinoperator.dev/v1alpha1
+    kind: SpinApp
     name: hpa-spinapp
   minReplicas: 1
   maxReplicas: 10

internal/controller/spinapp_controller.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"hash/adler32"
 	"maps"
+	"strings"
 
 	"github.com/pelletier/go-toml/v2"
 	appsv1 "k8s.io/api/apps/v1"
@@ -187,6 +188,8 @@ func (r *SpinAppReconciler) updateStatus(ctx context.Context, app *spinv1alpha1.
 				Reason:  "DeploymentNotFound",
 				Message: "Deployment not found",
 			})
+
+		app.Status.Selector = ""
 		app.Status.ReadyReplicas = 0
 	} else {
 		deploymentConditions := deployment.Status.Conditions
@@ -212,6 +215,13 @@ func (r *SpinAppReconciler) updateStatus(ctx context.Context, app *spinv1alpha1.
 					})
 			}
 		}
+
+		var selectorStringArray []string
+		for key, value := range deployment.Spec.Selector.MatchLabels {
+			selectorStringArray = append(selectorStringArray, key+"="+value)
+		}
+
+		app.Status.Selector = strings.Join(selectorStringArray, ",")
 		app.Status.ReadyReplicas = deployment.Status.ReadyReplicas
 	}
 
@@ -381,14 +391,7 @@ func (r *SpinAppReconciler) deleteDeployment(ctx context.Context, app *spinv1alp
 // constructDeployment builds an appsv1.Deployment based on the configuration of a SpinApp.
 func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config *spinv1alpha1.ExecutorDeploymentConfig,
 	generatedRuntimeConfigSecretName, caSecretName string, scheme *runtime.Scheme) (*appsv1.Deployment, error) {
-	// TODO: Once we land admission webhooks write some validation to make
-	// replicas and enableAutoscaling mutually exclusive.
-	var replicas *int32
-	if app.Spec.EnableAutoscaling {
-		replicas = nil
-	} else {
-		replicas = ptr(app.Spec.Replicas)
-	}
+	replicas := app.Spec.Replicas
 
 	volumes, volumeMounts, err := ConstructVolumeMountsForApp(ctx, app, generatedRuntimeConfigSecretName, caSecretName)
 	if err != nil {
@@ -444,7 +447,7 @@ func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config
 			Annotations: annotations,
 		},
 		Spec: appsv1.DeploymentSpec{
-			Replicas: replicas,
+			Replicas: &replicas,
 			Selector: &metav1.LabelSelector{
 				MatchLabels: readyLabels,
 			},

internal/webhook/spinapp_validating.go

@@ -58,9 +58,6 @@ func (v *SpinAppValidator) validateSpinApp(ctx context.Context, spinApp *spinv1a
 	if err != nil {
 		allErrs = append(allErrs, err)
 	}
-	if err := validateReplicas(spinApp.Spec); err != nil {
-		allErrs = append(allErrs, err)
-	}
 	if err := validateAnnotations(spinApp.Spec, executor); err != nil {
 		allErrs = append(allErrs, err)
 	}
@@ -103,17 +100,6 @@ func validateExecutor(spec spinv1alpha1.SpinAppSpec, fetchExecutor func(name str
 	return executor, nil
 }
 
-func validateReplicas(spec spinv1alpha1.SpinAppSpec) *field.Error {
-	if spec.EnableAutoscaling && spec.Replicas != 0 {
-		return field.Invalid(field.NewPath("spec").Child("replicas"), spec.Replicas, "replicas cannot be set when autoscaling is enabled")
-	}
-	if !spec.EnableAutoscaling && spec.Replicas < 1 {
-		return field.Invalid(field.NewPath("spec").Child("replicas"), spec.Replicas, "replicas must be > 0")
-	}
-
-	return nil
-}
-
 func validateAnnotations(spec spinv1alpha1.SpinAppSpec, executor *spinv1alpha1.SpinAppExecutor) *field.Error {
 	// We can't do any validation if the executor isn't available, but validation
 	// will fail because of earlier errors.

internal/webhook/spinapp_validating_test.go

@@ -24,16 +24,6 @@ func TestValidateExecutor(t *testing.T) {
 	require.Nil(t, fldErr)
 }
 
-func TestValidateReplicas(t *testing.T) {
-	t.Parallel()
-
-	fldErr := validateReplicas(spinv1alpha1.SpinAppSpec{})
-	require.EqualError(t, fldErr, "spec.replicas: Invalid value: 0: replicas must be > 0")
-
-	fldErr = validateReplicas(spinv1alpha1.SpinAppSpec{Replicas: 1})
-	require.Nil(t, fldErr)
-}
-
 func TestValidateAnnotations(t *testing.T) {
 	t.Parallel()