Merge pull request #993 from lyarwood/release-v0.20-deprecate-common-…

…instancetypes

[release-v0.20] common instancetypes: Deprecate operand

api/v1beta2/ssp_types.go

@@ -82,6 +82,8 @@ type SSPSpec struct {
 	TLSSecurityProfile *ocpv1.TLSSecurityProfile `json:"tlsSecurityProfile,omitempty"`
 
 	// CommonInstancetypes is the configuration of the common-instancetypes operand
+	//
+	// Deprecated: This functionality will be removed in a future release.
 	CommonInstancetypes *CommonInstancetypes `json:"commonInstancetypes,omitempty"`
 
 	// TektonPipelines is the configuration of the tekton-pipelines operand

config/crd/bases/ssp.kubevirt.io_ssps.yaml

@@ -3146,8 +3146,9 @@ spec:
             description: SSPSpec defines the desired state of SSP
             properties:
               commonInstancetypes:
-                description: CommonInstancetypes is the configuration of the common-instancetypes
-                  operand
+                description: "CommonInstancetypes is the configuration of the common-instancetypes
+                  operand \n Deprecated: This functionality will be removed in a future
+                  release."
                 properties:
                   url:
                     description: "URL of a remote Kustomize target from which to generate

data/crd/ssp.kubevirt.io_ssps.yaml

@@ -3147,8 +3147,9 @@ spec:
             description: SSPSpec defines the desired state of SSP
             properties:
               commonInstancetypes:
-                description: CommonInstancetypes is the configuration of the common-instancetypes
-                  operand
+                description: "CommonInstancetypes is the configuration of the common-instancetypes
+                  operand \n Deprecated: This functionality will be removed in a future
+                  release."
                 properties:
                   url:
                     description: "URL of a remote Kustomize target from which to generate

internal/operands/common-instancetypes/reconcile.go

@@ -240,18 +240,15 @@ func (c *CommonInstancetypes) reconcileRemovedResources(request *common.Request)
 }
 
 func (c *CommonInstancetypes) reconcileFromURL(request *common.Request) ([]common.ReconcileResult, error) {
-	// Handle the featureGate being disabled by ensuring any resources previously reconciled are cleaned up
-	if !isFeatureGateEnabled(request) {
-		return c.cleanupReconcile(request)
-	}
-
 	// TODO - In the future we should handle cases where the URL remains the same but the provided resources change.
+	//nolint:staticcheck
 	if c.resourceURL != "" && c.resourceURL == *request.Instance.Spec.CommonInstancetypes.URL {
 		request.Logger.Info(fmt.Sprintf("Skipping reconcile of common-instancetypes from URL %s, force with a restart of the service.", *request.Instance.Spec.CommonInstancetypes.URL))
 		return nil, nil
 	}
 
 	// Cache the URL so we can check if it changes with future reconcile attempts above
+	//nolint:staticcheck
 	c.resourceURL = *request.Instance.Spec.CommonInstancetypes.URL
 	request.Logger.Info(fmt.Sprintf("Reconciling common-instancetypes from URL %s", c.resourceURL))
 	var err error
@@ -274,11 +271,6 @@ func (c *CommonInstancetypes) reconcileFromURL(request *common.Request) ([]commo
 }
 
 func (c *CommonInstancetypes) reconcileFromBundle(request *common.Request) ([]common.ReconcileResult, error) {
-	// Handle the featureGate being disabled by ensuring any resources previously reconciled are cleaned up
-	if !isFeatureGateEnabled(request) {
-		return c.cleanupReconcile(request)
-	}
-
 	request.Logger.Info("Reconciling common-instancetypes from internal bundle")
 	var err error
 	c.virtualMachineClusterInstancetypes, c.virtualMachineClusterPreferences, err = c.fetchResourcesFromBundle()
@@ -306,6 +298,15 @@ func isFeatureGateEnabled(request *common.Request) bool {
 }
 
 func (c *CommonInstancetypes) Reconcile(request *common.Request) ([]common.ReconcileResult, error) {
+	// Handle the featureGate being disabled by ensuring any resources previously reconciled are cleaned up
+	if !isFeatureGateEnabled(request) {
+		return c.cleanupReconcile(request)
+	}
+
+	// Log that this functionality is now deprecated ahead of removal in a future release
+	request.Logger.Info("deployment of common-instancetypes by this operator is now deprecated and will be removed in a future release")
+
+	//nolint:staticcheck
 	if request.Instance.Spec.CommonInstancetypes != nil && request.Instance.Spec.CommonInstancetypes.URL != nil {
 		return c.reconcileFromURL(request)
 	}

internal/operands/common-instancetypes/reconcile_test.go

@@ -312,6 +312,7 @@ var _ = Describe("Common-Instancetypes operand", func() {
 		}
 
 		// Update the SSP CR to use a URL so that it calls our mock KustomizeRunFunc
+		//nolint:staticcheck
 		request.Instance.Spec.CommonInstancetypes = &ssp.CommonInstancetypes{
 			URL: ptr.To("https://foo.com/bar?ref=1"),
 		}
@@ -341,6 +342,7 @@ var _ = Describe("Common-Instancetypes operand", func() {
 			return mockResMap, nil
 		}
 
+		//nolint:staticcheck
 		request.Instance.Spec.CommonInstancetypes = &ssp.CommonInstancetypes{
 			URL: ptr.To("https://foo.com/bar?ref=1"),
 		}
@@ -359,6 +361,7 @@ var _ = Describe("Common-Instancetypes operand", func() {
 		operand.KustomizeRunFunc = func(_ filesys.FileSystem, _ string) (resmap.ResMap, error) {
 			return mockResMap, nil
 		}
+		//nolint:staticcheck
 		request.Instance.Spec.CommonInstancetypes = &ssp.CommonInstancetypes{
 			URL: ptr.To("https://foo.com/bar?ref=2"),
 		}
@@ -390,6 +393,7 @@ var _ = Describe("Common-Instancetypes operand", func() {
 		}
 
 		// Update the SSP CR to use a URL so that it calls KustomizeRunFunc
+		//nolint:staticcheck
 		request.Instance.Spec.CommonInstancetypes = &ssp.CommonInstancetypes{
 			URL: ptr.To("https://foo.com/bar?ref=1"),
 		}
@@ -463,6 +467,7 @@ var _ = Describe("Common-Instancetypes operand", func() {
 		}
 
 		// Update the SSP CR to use a URL so that it calls our mock KustomizeRunFunc
+		//nolint:staticcheck
 		request.Instance.Spec.CommonInstancetypes = &ssp.CommonInstancetypes{
 			URL: ptr.To("https://foo.com/bar?ref=1"),
 		}
@@ -485,6 +490,7 @@ var _ = Describe("Common-Instancetypes operand", func() {
 		}
 
 		// Update the SSP CR to use a URL so that it calls our mock KustomizeRunFunc
+		//nolint:staticcheck
 		request.Instance.Spec.CommonInstancetypes = &ssp.CommonInstancetypes{
 			URL: ptr.To("https://foo.com/bar?ref=1"),
 		}

tests/common_instancetypes_test.go

@@ -43,6 +43,7 @@ var _ = Describe("Common Instance Types", func() {
 		It("should reconcile from URL when provided", func() {
 			URL := "https://github.com/kubevirt/common-instancetypes//VirtualMachineClusterPreferences?ref=v0.3.3"
 			sspObj := getSsp()
+			//nolint:staticcheck
 			sspObj.Spec.CommonInstancetypes = &ssp.CommonInstancetypes{
 				URL: ptr.To(URL),
 			}
@@ -130,6 +131,7 @@ var _ = Describe("Common Instance Types", func() {
 	Context("webhook", func() {
 		DescribeTable("should reject URL", func(URL string) {
 			sspObj := getSsp()
+			//nolint:staticcheck
 			sspObj.Spec.CommonInstancetypes = &ssp.CommonInstancetypes{
 				URL: ptr.To(URL),
 			}

webhooks/ssp_webhook.go

@@ -213,10 +213,12 @@ func validateDataImportCronTemplates(ssp *sspv1beta2.SSP) error {
 }
 
 func validateCommonInstancetypes(ssp *sspv1beta2.SSP) error {
+	//nolint:staticcheck
 	if ssp.Spec.CommonInstancetypes == nil || ssp.Spec.CommonInstancetypes.URL == nil {
 		return nil
 	}
 
+	//nolint:staticcheck
 	url := *ssp.Spec.CommonInstancetypes.URL
 	if !strings.HasPrefix(url, "https://") && !strings.HasPrefix(url, "ssh://") {
 		return fmt.Errorf("%s is invalid, only https:// or ssh:// are supported as a remote kustomize target for commonInstancetypes", url)

webhooks/ssp_webhook_test.go

@@ -247,18 +247,21 @@ var _ = Describe("SSP Validation", func() {
 		})
 
 		It("should reject URL without https:// or ssh://", func() {
+			//nolint:staticcheck
 			sspObj.Spec.CommonInstancetypes.URL = ptr.To("file://foo/bar")
 			_, err := validator.ValidateCreate(ctx, toUnstructured(sspObj))
 			Expect(err).To(HaveOccurred())
 		})
 
 		It("should reject URL without ?ref= or ?version=", func() {
+			//nolint:staticcheck
 			sspObj.Spec.CommonInstancetypes.URL = ptr.To("https://foo.com/bar")
 			_, err := validator.ValidateCreate(ctx, toUnstructured(sspObj))
 			Expect(err).To(HaveOccurred())
 		})
 
 		DescribeTable("should accept a valid remote kustomize target URL", func(url string) {
+			//nolint:staticcheck
 			sspObj.Spec.CommonInstancetypes.URL = ptr.To(url)
 			_, err := validator.ValidateCreate(ctx, toUnstructured(sspObj))
 			Expect(err).ToNot(HaveOccurred())