Merge pull request #702 from lyarwood/common-instancetypes-featuregate

common-instancetypes: Add DeployCommonInstancetypes featureGate

api/v1beta2/ssp_types.go

@@ -90,7 +90,7 @@ type SSPSpec struct {
 	// TektonTasks is the configuration of the tekton-tasks operand
 	TektonTasks *TektonTasks `json:"tektonTasks,omitempty"`
 
-	// FeatureGates is the configuration of the tekton operands
+	// FeatureGates for SSP
 	FeatureGates *FeatureGates `json:"featureGates,omitempty"`
 }
 
@@ -104,11 +104,14 @@ type TektonTasks struct {
 	Namespace string `json:"namespace,omitempty"`
 }
 
-// FeatureGates defines feature gate for tto operator
+// FeatureGates for SSP
 type FeatureGates struct {
 	DeployTektonTaskResources bool `json:"deployTektonTaskResources,omitempty"`
 
 	DeployVmConsoleProxy bool `json:"deployVmConsoleProxy,omitempty"`
+
+	// Enables deployment of the common-instancetypes bundles, defaults to true.
+	DeployCommonInstancetypes *bool `json:"deployCommonInstancetypes,omitempty"`
 }
 
 // DataImportCronTemplate defines the template type for DataImportCrons.

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

@@ -3911,8 +3911,12 @@ spec:
                 - namespace
                 type: object
               featureGates:
-                description: FeatureGates is the configuration of the tekton operands
+                description: FeatureGates for SSP
                 properties:
+                  deployCommonInstancetypes:
+                    description: Enables deployment of the common-instancetypes bundles,
+                      defaults to true.
+                    type: boolean
                   deployTektonTaskResources:
                     type: boolean
                   deployVmConsoleProxy:

data/crd/ssp.kubevirt.io_ssps.yaml

@@ -3912,8 +3912,10 @@ spec:
                 - namespace
                 type: object
               featureGates:
-                description: FeatureGates is the configuration of the tekton operands
+                description: FeatureGates for SSP
                 properties:
+                  deployCommonInstancetypes:
+                    type: boolean
                   deployTektonTaskResources:
                     type: boolean
                   deployVmConsoleProxy:

internal/operands/common-instancetypes/reconcile.go

@@ -223,23 +223,28 @@ func reconcileRemovedPreferences(request *common.Request, existingResources []in
 	return nil
 }
 
-func (c *CommonInstancetypes) reconcileRemovedResources(request *common.Request, newInstancetypes []instancetypev1beta1.VirtualMachineClusterInstancetype, newPreferences []instancetypev1beta1.VirtualMachineClusterPreference) error {
+func (c *CommonInstancetypes) reconcileRemovedResources(request *common.Request) error {
 	existingClusterInstancetypes, existingClusterPreferences, err := c.fetchExistingResources(request)
 	if err != nil {
 		return err
 	}
 
-	if err = reconcileRemovedInstancetypes(request, existingClusterInstancetypes, newInstancetypes); err != nil {
+	if err = reconcileRemovedInstancetypes(request, existingClusterInstancetypes, c.virtualMachineClusterInstancetypes); err != nil {
 		return err
 	}
 
-	if err = reconcileRemovedPreferences(request, existingClusterPreferences, newPreferences); err != nil {
+	if err = reconcileRemovedPreferences(request, existingClusterPreferences, c.virtualMachineClusterPreferences); err != nil {
 		return err
 	}
 	return nil
 }
 
 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.
 	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))
@@ -249,19 +254,18 @@ func (c *CommonInstancetypes) reconcileFromURL(request *common.Request) ([]commo
 	// Cache the URL so we can check if it changes with future reconcile attempts above
 	c.resourceURL = *request.Instance.Spec.CommonInstancetypes.URL
 	request.Logger.Info(fmt.Sprintf("Reconciling common-instancetypes from URL %s", c.resourceURL))
-	clusterInstancetypesFromURL, clusterPreferencesFromURL, err := c.FetchResourcesFromURL(c.resourceURL)
+	var err error
+	c.virtualMachineClusterInstancetypes, c.virtualMachineClusterPreferences, err = c.FetchResourcesFromURL(c.resourceURL)
 	if err != nil {
 		return nil, err
 	}
 
 	// Remove any resources no longer provided by the URL, this should only happen when switching from the internal bundle to external URL for now.
-	if err = c.reconcileRemovedResources(request, clusterInstancetypesFromURL, clusterPreferencesFromURL); err != nil {
+	if err = c.reconcileRemovedResources(request); err != nil {
 		return nil, err
 	}
 
 	// Generate the normal set of reconcile funcs to create or update the provided resources
-	c.virtualMachineClusterInstancetypes = clusterInstancetypesFromURL
-	c.virtualMachineClusterPreferences = clusterPreferencesFromURL
 	reconcileFuncs, err := c.reconcileFuncs(request)
 	if err != nil {
 		return nil, err
@@ -270,33 +274,58 @@ 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")
-	clusterInstancetypesFromBundle, clusterPreferencesFromBundle, err := c.fetchResourcesFromBundle()
+	var err error
+	c.virtualMachineClusterInstancetypes, c.virtualMachineClusterPreferences, err = c.fetchResourcesFromBundle()
 	if err != nil {
 		return nil, err
 	}
 
 	// Remove any resources no longer provided by the bundle
-	if err = c.reconcileRemovedResources(request, clusterInstancetypesFromBundle, clusterPreferencesFromBundle); err != nil {
+	if err = c.reconcileRemovedResources(request); err != nil {
 		return nil, err
 	}
 
-	c.virtualMachineClusterInstancetypes = clusterInstancetypesFromBundle
-	c.virtualMachineClusterPreferences = clusterPreferencesFromBundle
 	reconcileFuncs, err := c.reconcileFuncs(request)
 	if err != nil {
 		return nil, err
 	}
 	return common.CollectResourceStatus(request, reconcileFuncs...)
 }
 
+func isFeatureGateEnabled(request *common.Request) bool {
+	if request.Instance.Spec.FeatureGates == nil || request.Instance.Spec.FeatureGates.DeployCommonInstancetypes == nil {
+		return true
+	}
+	return *request.Instance.Spec.FeatureGates.DeployCommonInstancetypes
+}
+
 func (c *CommonInstancetypes) Reconcile(request *common.Request) ([]common.ReconcileResult, error) {
 	if request.Instance.Spec.CommonInstancetypes != nil && request.Instance.Spec.CommonInstancetypes.URL != nil {
 		return c.reconcileFromURL(request)
 	}
 	return c.reconcileFromBundle(request)
 }
 
+func (c *CommonInstancetypes) cleanupReconcile(request *common.Request) ([]common.ReconcileResult, error) {
+	cleanupResults, err := c.Cleanup(request)
+	if err != nil {
+		return nil, err
+	}
+	var results []common.ReconcileResult
+	for _, cleanupResult := range cleanupResults {
+		if !cleanupResult.Deleted {
+			results = append(results, common.ResourceDeletedResult(cleanupResult.Resource, common.OperationResultDeleted))
+		}
+	}
+	return results, nil
+}
+
 func (c *CommonInstancetypes) Cleanup(request *common.Request) ([]common.CleanupResult, error) {
 	var objects []client.Object
 

internal/operands/common-instancetypes/reconcile_test.go

@@ -400,6 +400,114 @@ var _ = Describe("Common-Instancetypes operand", func() {
 		ExpectResourceExists(instancetype, request)
 		ExpectResourceExists(preference, request)
 	})
+
+	It("should not deploy internal bundle resources when featureGate is disabled", func() {
+		request.Instance.Spec.FeatureGates = &ssp.FeatureGates{
+			DeployCommonInstancetypes: pointer.Bool(false),
+		}
+
+		_, err = operand.Reconcile(&request)
+		Expect(err).ToNot(HaveOccurred())
+
+		virtualMachineClusterInstancetypes, err := FetchBundleResource[instancetypev1beta1.VirtualMachineClusterInstancetype](instancetypePath)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(virtualMachineClusterInstancetypes).ToNot(BeEmpty())
+
+		virtualMachineClusterPreferences, err := FetchBundleResource[instancetypev1beta1.VirtualMachineClusterPreference](preferencePath)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(virtualMachineClusterPreferences).ToNot(BeEmpty())
+
+		assertResoucesDoNotExist(request, virtualMachineClusterInstancetypes, virtualMachineClusterPreferences)
+	})
+
+	It("should cleanup internal bundle resources from when featureGate is disabled after being enabled", func() {
+		_, err = operand.Reconcile(&request)
+		Expect(err).ToNot(HaveOccurred())
+
+		virtualMachineClusterInstancetypes, err := FetchBundleResource[instancetypev1beta1.VirtualMachineClusterInstancetype](instancetypePath)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(virtualMachineClusterInstancetypes).ToNot(BeEmpty())
+
+		virtualMachineClusterPreferences, err := FetchBundleResource[instancetypev1beta1.VirtualMachineClusterPreference](preferencePath)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(virtualMachineClusterPreferences).ToNot(BeEmpty())
+
+		assertResoucesExist(request, virtualMachineClusterInstancetypes, virtualMachineClusterPreferences)
+
+		// Assert that CrdList can see the required CRDs before we Reconcile and Cleanup
+		Expect(request.CrdList.CrdExists(virtualMachineClusterInstancetypeCrd)).To(BeTrue())
+		Expect(request.CrdList.CrdExists(virtualMachineClusterPreferenceCrd)).To(BeTrue())
+
+		request.Instance.Spec.FeatureGates = &ssp.FeatureGates{
+			DeployCommonInstancetypes: pointer.Bool(false),
+		}
+
+		_, err = operand.Reconcile(&request)
+		Expect(err).ToNot(HaveOccurred())
+
+		assertResoucesDoNotExist(request, virtualMachineClusterInstancetypes, virtualMachineClusterPreferences)
+	})
+
+	It("should not deploy external URI resources resources when featureGate is disabled", func() {
+		// Generate a mock ResMap and resources for the test
+		mockResMap, virtualMachineClusterInstancetypes, virtualMachineClusterPreferences, err := newMockResources(10, 10)
+		Expect(err).ToNot(HaveOccurred())
+
+		// Use a mock Run function to return our fake ResMap
+		operand.KustomizeRunFunc = func(_ filesys.FileSystem, _ string) (resmap.ResMap, error) {
+			return mockResMap, nil
+		}
+
+		request.Instance.Spec.FeatureGates = &ssp.FeatureGates{
+			DeployCommonInstancetypes: pointer.Bool(false),
+		}
+
+		// Update the SSP CR to use a URL so that it calls our mock KustomizeRunFunc
+		request.Instance.Spec.CommonInstancetypes = &ssp.CommonInstancetypes{
+			URL: pointer.String("https://foo.com/bar?ref=1"),
+		}
+
+		// Run Reconcile and assert the results
+		_, err = operand.Reconcile(&request)
+		Expect(err).ToNot(HaveOccurred())
+
+		assertResoucesDoNotExist(request, virtualMachineClusterInstancetypes, virtualMachineClusterPreferences)
+	})
+
+	It("should cleanup external URI resources from when featureGate is disabled after being enabled", func() {
+		// Generate a mock ResMap and resources for the test
+		mockResMap, virtualMachineClusterInstancetypes, virtualMachineClusterPreferences, err := newMockResources(10, 10)
+		Expect(err).ToNot(HaveOccurred())
+
+		// Use a mock Run function to return our fake ResMap
+		operand.KustomizeRunFunc = func(_ filesys.FileSystem, _ string) (resmap.ResMap, error) {
+			return mockResMap, nil
+		}
+
+		// Update the SSP CR to use a URL so that it calls our mock KustomizeRunFunc
+		request.Instance.Spec.CommonInstancetypes = &ssp.CommonInstancetypes{
+			URL: pointer.String("https://foo.com/bar?ref=1"),
+		}
+
+		// Run Reconcile and assert the results
+		_, err = operand.Reconcile(&request)
+		Expect(err).ToNot(HaveOccurred())
+
+		assertResoucesExist(request, virtualMachineClusterInstancetypes, virtualMachineClusterPreferences)
+
+		// Assert that CrdList can see the required CRDs before we Reconcile and Cleanup
+		Expect(request.CrdList.CrdExists(virtualMachineClusterInstancetypeCrd)).To(BeTrue())
+		Expect(request.CrdList.CrdExists(virtualMachineClusterPreferenceCrd)).To(BeTrue())
+
+		request.Instance.Spec.FeatureGates = &ssp.FeatureGates{
+			DeployCommonInstancetypes: pointer.Bool(false),
+		}
+
+		_, err = operand.Reconcile(&request)
+		Expect(err).ToNot(HaveOccurred())
+
+		assertResoucesDoNotExist(request, virtualMachineClusterInstancetypes, virtualMachineClusterPreferences)
+	})
 })
 
 func addConversionFunctions(s *runtime.Scheme) error {

tests/common_instancetypes_test.go

@@ -102,6 +102,28 @@ var _ = Describe("Common Instance Types", func() {
 			Expect(apiClient.Get(ctx, client.ObjectKey{Name: preferenceToUpdate.Name}, preferenceToUpdate)).To(Succeed())
 			Expect(preferenceToUpdate.Spec.CPU).To(Equal(updatedPreferenceCPU))
 		})
+		It("should cleanup resources when feature gate is disabled", func() {
+			sspObj := getSsp()
+			sspObj.Spec.FeatureGates = &ssp.FeatureGates{
+				DeployCommonInstancetypes: pointer.Bool(false),
+			}
+			createOrUpdateSsp(sspObj)
+			waitUntilDeployed()
+
+			virtualMachineClusterInstancetypes, err := common_instancetypes.FetchBundleResource[instancetypev1beta1.VirtualMachineClusterInstancetype]("../" + common_instancetypes.BundleDir + common_instancetypes.ClusterInstancetypesBundle)
+			Expect(err).ToNot(HaveOccurred())
+
+			virtualMachineClusterPreferences, err := common_instancetypes.FetchBundleResource[instancetypev1beta1.VirtualMachineClusterPreference]("../" + common_instancetypes.BundleDir + common_instancetypes.ClusterPreferencesBundle)
+			Expect(err).ToNot(HaveOccurred())
+
+			for _, instancetype := range virtualMachineClusterInstancetypes {
+				Expect(apiClient.Get(ctx, client.ObjectKey{Name: instancetype.Name}, &instancetypev1beta1.VirtualMachineClusterInstancetype{})).ToNot(Succeed())
+			}
+
+			for _, preference := range virtualMachineClusterPreferences {
+				Expect(apiClient.Get(ctx, client.ObjectKey{Name: preference.Name}, &instancetypev1beta1.VirtualMachineClusterPreference{})).ToNot(Succeed())
+			}
+		})
 	})
 	Context("webhook", func() {
 		DescribeTable("should reject URL", func(URL string) {