Skip to content

Commit

Permalink
common-instancetypes: Add DeployCommonInstancetypes featureGate
Browse files Browse the repository at this point in the history
This change adds a simple DeployCommonInstancetypes feature gate to the
SSP CR. This feature gate defaults to true to keep the original
behaviour of the common-instancetypes operand.

When disabled, any previously deployed resources are removed.

Co-authored-by: Felix Matouschek <[email protected]>
Signed-off-by: Lee Yarwood <[email protected]>
  • Loading branch information
lyarwood and 0xFelix committed Oct 30, 2023
1 parent 2f3368b commit 7648d29
Show file tree
Hide file tree
Showing 9 changed files with 191 additions and 8 deletions.
7 changes: 5 additions & 2 deletions api/v1beta2/ssp_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand All @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion config/crd/bases/ssp.kubevirt.io_ssps.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion data/crd/ssp.kubevirt.io_ssps.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
31 changes: 31 additions & 0 deletions internal/operands/common-instancetypes/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ 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.
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))
Expand Down Expand Up @@ -269,6 +274,11 @@ 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()
Expand All @@ -288,13 +298,34 @@ func (c *CommonInstancetypes) reconcileFromBundle(request *common.Request) ([]co
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

Expand Down
108 changes: 108 additions & 0 deletions internal/operands/common-instancetypes/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
22 changes: 22 additions & 0 deletions tests/common_instancetypes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,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) {
Expand Down
7 changes: 5 additions & 2 deletions vendor/kubevirt.io/ssp-operator/api/v1beta2/ssp_types.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 7648d29

Please sign in to comment.