Remove deprecations and fix CRD columns (#888)

* Remove deprecations and fix CRD columns Signed-off-by: Jorge Turrado <[email protected]> * add missing files Signed-off-by: Jorge Turrado <[email protected]> * add missing files Signed-off-by: Jorge Turrado <[email protected]> * fix duplicated conditions Signed-off-by: Jorge Turrado <[email protected]> * fix style Signed-off-by: Jorge Turrado <[email protected]> * fix space Signed-off-by: Jorge Turrado <[email protected]> * Update changelog Signed-off-by: Jorge Turrado <[email protected]> --------- Signed-off-by: Jorge Turrado <[email protected]>
kedacore · Jan 22, 2024 · b8f3972 · b8f3972
1 parent 855547a
commit b8f3972
Show file tree

Hide file tree

Showing 10 changed files with 50 additions and 283 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,7 +17,7 @@ This changelog keeps track of work items that have been completed and are ready
 
 ### Breaking Changes
 
-- **General**: TODO ([#TODO](https://github.com/kedacore/http-add-on/issues/TODO))
+- **General**: `host` field has been removed in favor of `hosts` in `HTTPScaledObject` ([#552](https://github.com/kedacore/http-add-on/issues/552)|[#888](https://github.com/kedacore/http-add-on/pull/888))
 
 ### New
 
@@ -44,8 +44,6 @@ New deprecation(s):
 
 Previously announced deprecation(s):
 
-- **General**: `host` field deprecated in favor of `hosts` in `HTTPScaledObject` ([#552](https://github.com/kedacore/http-add-on/issues/552))
-
 ### Other
 
 - **General**: Bump golang version ([#853](https://github.com/kedacore/http-add-on/pull/853))

diff --git a/config/crd/bases/http.keda.sh_httpscaledobjects.yaml b/config/crd/bases/http.keda.sh_httpscaledobjects.yaml
@@ -17,15 +17,12 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
-    - jsonPath: .spec.scaleTargetRef.deploymentName
-      name: ScaleTargetDeploymentName
+    - jsonPath: .status.targetWorkload
+      name: TargetWorkload
       type: string
-    - jsonPath: .spec.scaleTargetRef
-      name: ScaleTargetServiceName
+    - jsonPath: .status.targetService
+      name: TargetService
       type: string
-    - jsonPath: .spec.scaleTargetRef
-      name: ScaleTargetPort
-      type: integer
     - jsonPath: .spec.replicas.min
       name: MinReplicas
       type: integer
@@ -58,19 +55,12 @@ spec:
           spec:
             description: HTTPScaledObjectSpec defines the desired state of HTTPScaledObject
             properties:
-              host:
-                description: (optional) (deprecated) The host to route. All requests
-                  which the "Host" header matches .spec.host and the Request Target
-                  matches any .spec.pathPrefixes will be routed to the Service and
-                  Port specified in the scaleTargetRef. The .spec.host field is mutually
-                  exclusive with the .spec.hosts field.
-                type: string
               hosts:
-                description: (optional) The hosts to route. All requests which the
-                  "Host" header matches any .spec.hosts and the Request Target matches
-                  any .spec.pathPrefixes will be routed to the Service and Port specified
-                  in the scaleTargetRef. The .spec.hosts field is mutually exclusive
-                  with the .spec.host field.
+                description: The hosts to route. All requests which the "Host" header
+                  matches any .spec.hosts and the Request Target matches any .spec.pathPrefixes
+                  will be routed to the Service and Port specified in the scaleTargetRef.
+                  The .spec.hosts field is mutually exclusive with the .spec.host
+                  field.
                 items:
                   type: string
                 type: array
@@ -178,6 +168,12 @@ spec:
                   - type
                   type: object
                 type: array
+              targetService:
+                description: TargetService reflects details about the scaled service.
+                type: string
+              targetWorkload:
+                description: TargetWorkload reflects details about the scaled workload.
+                type: string
             type: object
         type: object
     served: true

diff --git a/operator/apis/http/v1alpha1/httpscaledobject_types.go b/operator/apis/http/v1alpha1/httpscaledobject_types.go
@@ -47,14 +47,7 @@ type ReplicaStruct struct {
 
 // HTTPScaledObjectSpec defines the desired state of HTTPScaledObject
 type HTTPScaledObjectSpec struct {
-	// (optional) (deprecated) The host to route. All requests which the "Host"
-	// header matches .spec.host and the Request Target matches any
-	// .spec.pathPrefixes will be routed to the Service and Port specified in
-	// the scaleTargetRef. The .spec.host field is mutually exclusive with the
-	// .spec.hosts field.
-	// +optional
-	Host *string `json:"host,omitempty"`
-	// (optional) The hosts to route. All requests which the "Host" header
+	// The hosts to route. All requests which the "Host" header
 	// matches any .spec.hosts and the Request Target matches any
 	// .spec.pathPrefixes will be routed to the Service and Port specified in
 	// the scaleTargetRef. The .spec.hosts field is mutually exclusive with the
@@ -141,16 +134,21 @@ type HTTPScaledObjectCondition struct {
 
 // HTTPScaledObjectStatus defines the observed state of HTTPScaledObject
 type HTTPScaledObjectStatus struct {
+	// TargetWorkload reflects details about the scaled workload.
+	// +optional
+	TargetWorkload string `json:"targetWorkload,omitempty" description:"It reflects details about the scaled workload"`
+	// TargetService reflects details about the scaled service.
+	// +optional
+	TargetService string `json:"targetService,omitempty" description:"It reflects details about the scaled service"`
 	// Conditions of the operator
 	Conditions []HTTPScaledObjectCondition `json:"conditions,omitempty" description:"List of auditable conditions of the operator"`
 }
 
 // +genclient
 // +k8s:openapi-gen=true
 // +kubebuilder:object:root=true
-// +kubebuilder:printcolumn:name="ScaleTargetDeploymentName",type="string",JSONPath=".spec.scaleTargetRef.deploymentName"
-// +kubebuilder:printcolumn:name="ScaleTargetServiceName",type="string",JSONPath=".spec.scaleTargetRef"
-// +kubebuilder:printcolumn:name="ScaleTargetPort",type="integer",JSONPath=".spec.scaleTargetRef"
+// +kubebuilder:printcolumn:name="TargetWorkload",type="string",JSONPath=".status.targetWorkload"
+// +kubebuilder:printcolumn:name="TargetService",type="string",JSONPath=".status.targetService"
 // +kubebuilder:printcolumn:name="MinReplicas",type="integer",JSONPath=".spec.replicas.min"
 // +kubebuilder:printcolumn:name="MaxReplicas",type="integer",JSONPath=".spec.replicas.max"
 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"

diff --git a/operator/apis/http/v1alpha1/zz_generated.deepcopy.go b/operator/apis/http/v1alpha1/zz_generated.deepcopy.go
diff --git a/operator/controllers/http/app.go b/operator/controllers/http/app.go
@@ -30,7 +30,7 @@ func (r *HTTPScaledObjectReconciler) createOrUpdateApplicationResources(
 	)
 
 	// set initial statuses
-	AddCondition(
+	AddOrUpdateCondition(
 		httpso,
 		*SetMessage(
 			CreateCondition(

diff --git a/operator/controllers/http/condition_provider.go b/operator/controllers/http/condition_provider.go
@@ -29,9 +29,18 @@ func SaveStatus(
 	}
 }
 
-// AddCondition adds a new condition to the HTTPScaledObject
-func AddCondition(httpso *httpv1alpha1.HTTPScaledObject, condition httpv1alpha1.HTTPScaledObjectCondition) *httpv1alpha1.HTTPScaledObject {
-	httpso.Status.Conditions = append(httpso.Status.Conditions, condition)
+// AddOrUpdateCondition adds or update a condition to the HTTPScaledObject
+func AddOrUpdateCondition(httpso *httpv1alpha1.HTTPScaledObject, condition httpv1alpha1.HTTPScaledObjectCondition) *httpv1alpha1.HTTPScaledObject {
+	found := false
+	for i := range httpso.Status.Conditions {
+		if httpso.Status.Conditions[i].Reason == condition.Reason {
+			found = true
+			httpso.Status.Conditions[i] = condition
+		}
+	}
+	if !found {
+		httpso.Status.Conditions = append(httpso.Status.Conditions, condition)
+	}
 	return httpso
 }
 

diff --git a/operator/controllers/http/httpscaledobject_controller.go b/operator/controllers/http/httpscaledobject_controller.go
@@ -19,6 +19,7 @@ package http
 import (
 	"context"
 	"errors"
+	"fmt"
 	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -86,11 +87,6 @@ func (r *HTTPScaledObjectReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, err
 	}
 
-	// TODO(pedrotorres): delete this on v0.6.0
-	if httpso.Spec.Host != nil {
-		logger.Info(".spec.host is deprecated, performing automated migration to .spec.hosts")
-		return ctrl.Result{}, r.migrateHost(ctx, httpso)
-	}
 	// TODO(jorturfer): delete this for v0.9.0
 	if httpso.Spec.ScaleTargetRef.Name == "" ||
 		httpso.Spec.ScaleTargetRef.Kind == "" ||
@@ -99,6 +95,10 @@ func (r *HTTPScaledObjectReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, r.migrateTargetRef(ctx, httpso)
 	}
 
+	// update status
+	httpso.Status.TargetWorkload = fmt.Sprintf("%s/%s/%s", httpso.Spec.ScaleTargetRef.APIVersion, httpso.Spec.ScaleTargetRef.Kind, httpso.Spec.ScaleTargetRef.Name)
+	httpso.Status.TargetService = fmt.Sprintf("%s:%d", httpso.Spec.ScaleTargetRef.Service, httpso.Spec.ScaleTargetRef.Port)
+
 	// httpso is updated now
 	logger.Info(
 		"Reconciling HTTPScaledObject",
@@ -125,7 +125,7 @@ func (r *HTTPScaledObjectReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		ctx,
 		logger,
 		r.Client,
-		AddCondition(
+		AddOrUpdateCondition(
 			httpso,
 			*SetMessage(
 				CreateCondition(
@@ -150,18 +150,6 @@ func (r *HTTPScaledObjectReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-// TODO(pedrotorres): delete this on v0.6.0
-func (r *HTTPScaledObjectReconciler) migrateHost(ctx context.Context, httpso *httpv1alpha1.HTTPScaledObject) error {
-	if (httpso.Spec.Hosts != nil) == (httpso.Spec.Host != nil) {
-		return errors.New("exactly one of .spec.host and .spec.hosts must be set")
-	}
-	httpso.Spec.Hosts = []string{
-		*httpso.Spec.Host,
-	}
-	httpso.Spec.Host = nil
-	return r.Client.Update(ctx, httpso)
-}
-
 // TODO(jorturfer): delete this for v0.9.0
 func (r *HTTPScaledObjectReconciler) migrateTargetRef(ctx context.Context, httpso *httpv1alpha1.HTTPScaledObject) error {
 	if (httpso.Spec.ScaleTargetRef.Deployment != "") == (httpso.Spec.ScaleTargetRef.Name != "") {

diff --git a/operator/controllers/http/scaled_object.go b/operator/controllers/http/scaled_object.go
@@ -2,7 +2,6 @@ package http
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/go-logr/logr"
 	kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1"
@@ -75,7 +74,7 @@ func (r *HTTPScaledObjectReconciler) createOrUpdateScaledObject(
 				return err
 			}
 		} else {
-			AddCondition(
+			AddOrUpdateCondition(
 				httpso,
 				*SetMessage(
 					CreateCondition(
@@ -92,7 +91,7 @@ func (r *HTTPScaledObjectReconciler) createOrUpdateScaledObject(
 		}
 	}
 
-	AddCondition(
+	AddOrUpdateCondition(
 		httpso,
 		*SetMessage(
 			CreateCondition(
@@ -103,43 +102,5 @@ func (r *HTTPScaledObjectReconciler) createOrUpdateScaledObject(
 			"App ScaledObject created",
 		),
 	)
-
-	return r.purgeLegacySO(ctx, cl, logger, httpso)
-}
-
-// TODO(pedrotorres): delete this on v0.6.0
-func (r *HTTPScaledObjectReconciler) purgeLegacySO(
-	ctx context.Context,
-	cl client.Client,
-	logger logr.Logger,
-	httpso *httpv1alpha1.HTTPScaledObject,
-) error {
-	legacyName := fmt.Sprintf("%s-app", httpso.GetName())
-	legacyKey := client.ObjectKey{
-		Namespace: httpso.GetNamespace(),
-		Name:      legacyName,
-	}
-
-	var legacySO kedav1alpha1.ScaledObject
-	if err := cl.Get(ctx, legacyKey, &legacySO); err != nil {
-		if errors.IsNotFound(err) {
-			logger.Info("legacy ScaledObject not found")
-			return nil
-		}
-
-		logger.Error(err, "failed getting legacy ScaledObject")
-		return err
-	}
-
-	if err := cl.Delete(ctx, &legacySO); err != nil {
-		if errors.IsNotFound(err) {
-			logger.Info("legacy ScaledObject not found")
-			return nil
-		}
-
-		logger.Error(err, "failed deleting legacy ScaledObject")
-		return err
-	}
-
 	return nil
 }
diff --git a/tests/checks/path_prefix/path_prefix_test.go b/tests/checks/path_prefix/path_prefix_test.go
@@ -107,7 +107,8 @@ metadata:
   name: {{.HTTPScaledObjectName}}
   namespace: {{.TestNamespace}}
 spec:
-  host: {{.Host}}
+  hosts:
+  - {{.Host}}
   pathPrefixes:
   - {{.PathPrefix0}}
   - {{.PathPrefix1}}
@@ -180,7 +181,8 @@ metadata:
   name: {{.HTTPScaledObjectName}}-2
   namespace: {{.TestNamespace}}
 spec:
-  host: {{.Host}}
+  hosts:
+  - {{.Host}}
   pathPrefixes:
   - {{.PathPrefix2}}
   targetPendingRequests: 100