From 41d07926bd87e8f1c9426db20189f00429852625 Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Thu, 6 Apr 2023 10:57:49 -0400 Subject: [PATCH] OpenStack support in CPMS --- docs/user/failure-domains.md | 13 + docs/user/installation.md | 28 + .../controller.go | 7 + .../controller_test.go | 583 ++++++++++++++++++ .../openstack.go | 117 ++++ .../controlplanemachinesetgenerator/utils.go | 2 + .../v1beta1/failuredomain/failuredomain.go | 64 +- .../failuredomain/failuredomain_test.go | 127 ++++ .../v1beta1/providerconfig/openstack.go | 80 +++ .../v1beta1/providerconfig/openstack_test.go | 117 ++++ .../v1beta1/providerconfig/providerconfig.go | 22 + .../providerconfig/providerconfig_test.go | 132 ++++ .../controlplanemachineset/webhooks.go | 8 + .../controlplanemachineset/webhooks_test.go | 146 +++++ test/e2e/framework/framework.go | 42 +- 15 files changed, 1484 insertions(+), 4 deletions(-) create mode 100644 pkg/controllers/controlplanemachinesetgenerator/openstack.go create mode 100644 pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/openstack.go create mode 100644 pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/openstack_test.go diff --git a/docs/user/failure-domains.md b/docs/user/failure-domains.md index b12c874e1..f3b92ab71 100644 --- a/docs/user/failure-domains.md +++ b/docs/user/failure-domains.md @@ -83,3 +83,16 @@ An Azure failure domain will look something like the example below: ```yaml - zone: "" ``` + +## OpenStack + +On OpenStack, the failure domains represented in the control plane machine set can be considered analogous to the +OpenStack availability zones (for Nova and Cinder). + +An OpenStack failure domain will look something like the example below: +```yaml +- computeAvailabilityZone: "" + storageAvailabilityZone: "" +``` + +They are both optional and also can be set alone. \ No newline at end of file diff --git a/docs/user/installation.md b/docs/user/installation.md index 6ee949863..46d846f41 100644 --- a/docs/user/installation.md +++ b/docs/user/installation.md @@ -238,3 +238,31 @@ failureDomains: > Note: The `targetPools` field may not be set on the GCP providerSpec. This field is required for control plane machines and you should populate this on both the Machine and the ControlPlaneMachineSet resource specs. + +#### Configuring a control plane machine set on OpenStack + +Two fields are supported for now: `computeAvailabilityZone` (instance AZ) and `storageAvailabilityZone` (root volume AZ). +Gather the existing control plane machines and note the value of the zones of each if they exist. +Aside from these fields, the remaining in spec the machines should be identical. + +Copy the value from one of the machines into the `providerSpec.value` (6) on the example above. +Remove the AZ fields from the `providerSpec.value` once you have done that. + +For each AZ you have in the cluster (normally 3), configure a failure domain like below: +```yaml +- computeAvailabilityZone: "" + storageAvailabilityZone: "" +``` + +With these zones, the complete `failureDomains` (3 and 4) on the example above should look something like below: +```yaml +failureDomains: + platform: OpenStack + openstack: + - computeAvailabilityZone: "us-central1-a" + storageAvailabilityZone: "us-central1-a" + - computeAvailabilityZone: "us-central1-b" + storageAvailabilityZone: "us-central1-b" + - computeAvailabilityZone: "us-central1-c" + storageAvailabilityZone: "us-central1-c" +``` diff --git a/pkg/controllers/controlplanemachinesetgenerator/controller.go b/pkg/controllers/controlplanemachinesetgenerator/controller.go index e5da6d464..3056639c4 100644 --- a/pkg/controllers/controlplanemachinesetgenerator/controller.go +++ b/pkg/controllers/controlplanemachinesetgenerator/controller.go @@ -210,6 +210,8 @@ func (r *ControlPlaneMachineSetGeneratorReconciler) reconcile(ctx context.Contex } // generateControlPlaneMachineSet generates a control plane machine set based on the current cluster state. +// +//nolint:cyclop func (r *ControlPlaneMachineSetGeneratorReconciler) generateControlPlaneMachineSet(logger logr.Logger, platformType configv1.PlatformType, machines []machinev1beta1.Machine, machineSets []machinev1beta1.MachineSet) (*machinev1.ControlPlaneMachineSet, error) { var ( @@ -233,6 +235,11 @@ func (r *ControlPlaneMachineSetGeneratorReconciler) generateControlPlaneMachineS if err != nil { return nil, fmt.Errorf("unable to generate control plane machine set spec: %w", err) } + case configv1.OpenStackPlatformType: + cpmsSpecApplyConfig, err = generateControlPlaneMachineSetOpenStackSpec(machines, machineSets) + if err != nil { + return nil, fmt.Errorf("unable to generate control plane machine set spec: %w", err) + } default: logger.V(1).WithValues("platform", platformType).Info(unsupportedPlatform) return nil, errUnsupportedPlatform diff --git a/pkg/controllers/controlplanemachinesetgenerator/controller_test.go b/pkg/controllers/controlplanemachinesetgenerator/controller_test.go index 0e431f322..26dcb3e6b 100644 --- a/pkg/controllers/controlplanemachinesetgenerator/controller_test.go +++ b/pkg/controllers/controlplanemachinesetgenerator/controller_test.go @@ -25,6 +25,7 @@ import ( configv1 "github.com/openshift/api/config/v1" machinev1 "github.com/openshift/api/machine/v1" + machinev1alpha1 "github.com/openshift/api/machine/v1alpha1" machinev1beta1 "github.com/openshift/api/machine/v1beta1" "github.com/openshift/cluster-api-actuator-pkg/testutils" configv1resourcebuilder "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/config/v1" @@ -1870,3 +1871,585 @@ var _ = Describe("controlplanemachinesetgenerator controller on GCP", func() { }) }) }) + +var _ = Describe("controlplanemachinesetgenerator controller on OpenStack", func() { + + var ( + az1FailureDomainBuilderOpenStack = machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az1").WithStorageAvailabilityZone("az1") + + az2FailureDomainBuilderOpenStack = machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az2").WithStorageAvailabilityZone("az2") + + az3FailureDomainBuilderOpenStack = machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az3").WithStorageAvailabilityZone("az3") + + az4FailureDomainBuilderOpenStack = machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az4") + + az5FailureDomainBuilderOpenStack = machinev1resourcebuilder.OpenStackFailureDomain().WithStorageAvailabilityZone("az5") + + az1ProviderSpecBuilderOpenStack = machinev1beta1resourcebuilder.OpenStackProviderSpec().WithZone("az1").WithRootVolume(&machinev1alpha1.RootVolume{ + Zone: "az1", + }) + + az2ProviderSpecBuilderOpenStack = machinev1beta1resourcebuilder.OpenStackProviderSpec().WithZone("az2").WithRootVolume(&machinev1alpha1.RootVolume{ + Zone: "az2", + }) + + az3ProviderSpecBuilderOpenStack = machinev1beta1resourcebuilder.OpenStackProviderSpec().WithZone("az3").WithRootVolume(&machinev1alpha1.RootVolume{ + Zone: "az3", + }) + + az4ProviderSpecBuilderOpenStack = machinev1beta1resourcebuilder.OpenStackProviderSpec().WithZone("az4") + + az5ProviderSpecBuilderOpenStack = machinev1beta1resourcebuilder.OpenStackProviderSpec().WithRootVolume(&machinev1alpha1.RootVolume{ + Zone: "az5", + }) + + cpms3FailureDomainsBuilderOpenStack = machinev1resourcebuilder.OpenStackFailureDomains().WithFailureDomainBuilders( + az1FailureDomainBuilderOpenStack, + az2FailureDomainBuilderOpenStack, + az3FailureDomainBuilderOpenStack, + ) + + cpms5FailureDomainsBuilderOpenStack = machinev1resourcebuilder.OpenStackFailureDomains().WithFailureDomainBuilders( + az1FailureDomainBuilderOpenStack, + az2FailureDomainBuilderOpenStack, + az3FailureDomainBuilderOpenStack, + az4FailureDomainBuilderOpenStack, + az5FailureDomainBuilderOpenStack, + ) + + cpmsInactive3FDsBuilderOpenStack = machinev1resourcebuilder.ControlPlaneMachineSet(). + WithState(machinev1.ControlPlaneMachineSetStateInactive). + WithMachineTemplateBuilder( + machinev1resourcebuilder.OpenShiftMachineV1Beta1Template(). + WithProviderSpecBuilder( + az1ProviderSpecBuilderOpenStack.WithZone("").WithRootVolume(&machinev1alpha1.RootVolume{}).WithFlavor("m1.large"), + ). + WithFailureDomainsBuilder(machinev1resourcebuilder.OpenStackFailureDomains().WithFailureDomainBuilders( + az1FailureDomainBuilderOpenStack, + az2FailureDomainBuilderOpenStack, + az3FailureDomainBuilderOpenStack, + )), + ) + + cpmsInactive5FDsBuilderOpenStack = machinev1resourcebuilder.ControlPlaneMachineSet(). + WithState(machinev1.ControlPlaneMachineSetStateInactive). + WithMachineTemplateBuilder( + machinev1resourcebuilder.OpenShiftMachineV1Beta1Template(). + WithProviderSpecBuilder( + az1ProviderSpecBuilderOpenStack.WithZone("").WithRootVolume(&machinev1alpha1.RootVolume{}).WithFlavor("m1.large"), + ). + WithFailureDomainsBuilder(machinev1resourcebuilder.OpenStackFailureDomains().WithFailureDomainBuilders( + az1FailureDomainBuilderOpenStack, + az2FailureDomainBuilderOpenStack, + az3FailureDomainBuilderOpenStack, + az4FailureDomainBuilderOpenStack, + az5FailureDomainBuilderOpenStack, + )), + ) + + cpmsActiveOutdatedBuilderOpenStack = machinev1resourcebuilder.ControlPlaneMachineSet(). + WithState(machinev1.ControlPlaneMachineSetStateActive). + WithMachineTemplateBuilder( + machinev1resourcebuilder.OpenShiftMachineV1Beta1Template(). + WithProviderSpecBuilder( + az1ProviderSpecBuilderOpenStack.WithZone("").WithRootVolume(&machinev1alpha1.RootVolume{}).WithFlavor("m1.large"), + ). + WithFailureDomainsBuilder(machinev1resourcebuilder.OpenStackFailureDomains().WithFailureDomainBuilders( + az1FailureDomainBuilderOpenStack, + az2FailureDomainBuilderOpenStack, + az3FailureDomainBuilderOpenStack, + )), + ) + + cpmsActiveUpToDateBuilderOpenStack = machinev1resourcebuilder.ControlPlaneMachineSet(). + WithState(machinev1.ControlPlaneMachineSetStateActive). + WithMachineTemplateBuilder( + machinev1resourcebuilder.OpenShiftMachineV1Beta1Template(). + WithProviderSpecBuilder( + az1ProviderSpecBuilderOpenStack.WithZone("").WithRootVolume(&machinev1alpha1.RootVolume{}).WithFlavor("m1.large"), + ). + WithFailureDomainsBuilder(cpms5FailureDomainsBuilderOpenStack), + ) + ) + + var mgrCancel context.CancelFunc + var mgrDone chan struct{} + var mgr manager.Manager + var reconciler *ControlPlaneMachineSetGeneratorReconciler + + var namespaceName string + var cpms *machinev1.ControlPlaneMachineSet + var machine0, machine1, machine2 *machinev1beta1.Machine + var machineSet0, machineSet1, machineSet2, machineSet3, machineSet4 *machinev1beta1.MachineSet + + startManager := func(mgr *manager.Manager) (context.CancelFunc, chan struct{}) { + mgrCtx, mgrCancel := context.WithCancel(context.Background()) + mgrDone := make(chan struct{}) + + go func() { + defer GinkgoRecover() + defer close(mgrDone) + + Expect((*mgr).Start(mgrCtx)).To(Succeed()) + }() + + return mgrCancel, mgrDone + } + + stopManager := func() { + mgrCancel() + // Wait for the mgrDone to be closed, which will happen once the mgr has stopped + <-mgrDone + } + + create3MachineSets := func() { + machineSetBuilder := machinev1beta1resourcebuilder.MachineSet().WithNamespace(namespaceName) + machineSet0 = machineSetBuilder.WithProviderSpecBuilder(az1ProviderSpecBuilderOpenStack).WithGenerateName("machineset-az1-").Build() + machineSet1 = machineSetBuilder.WithProviderSpecBuilder(az2ProviderSpecBuilderOpenStack).WithGenerateName("machineset-az2-").Build() + machineSet2 = machineSetBuilder.WithProviderSpecBuilder(az3ProviderSpecBuilderOpenStack).WithGenerateName("machineset-az3-").Build() + + Expect(k8sClient.Create(ctx, machineSet0)).To(Succeed()) + Expect(k8sClient.Create(ctx, machineSet1)).To(Succeed()) + Expect(k8sClient.Create(ctx, machineSet2)).To(Succeed()) + } + + create5MachineSets := func() { + create3MachineSets() + + machineSetBuilder := machinev1beta1resourcebuilder.MachineSet().WithNamespace(namespaceName) + machineSet3 = machineSetBuilder.WithProviderSpecBuilder(az4ProviderSpecBuilderOpenStack).WithGenerateName("machineset-az4-").Build() + machineSet4 = machineSetBuilder.WithProviderSpecBuilder(az5ProviderSpecBuilderOpenStack).WithGenerateName("machineset-az5-").Build() + + Expect(k8sClient.Create(ctx, machineSet3)).To(Succeed()) + Expect(k8sClient.Create(ctx, machineSet4)).To(Succeed()) + } + + create3CPMachines := func() *[]machinev1beta1.Machine { + // Create 3 control plane machines with differing Provider Specs, + // so then we can reliably check which machine Provider Spec is picked for the ControlPlaneMachineSet. + machineBuilder := machinev1beta1resourcebuilder.Machine().AsMaster().WithNamespace(namespaceName) + machine0 = machineBuilder.WithProviderSpecBuilder(az1ProviderSpecBuilderOpenStack.WithFlavor("m1.large")).WithName("master-0").Build() + machine1 = machineBuilder.WithProviderSpecBuilder(az2ProviderSpecBuilderOpenStack.WithFlavor("m1.large")).WithName("master-1").Build() + machine2 = machineBuilder.WithProviderSpecBuilder(az3ProviderSpecBuilderOpenStack.WithFlavor("m1.large")).WithName("master-2").Build() + + // Create Machines with some wait time between them + // to achieve staggered CreationTimestamp(s). + Expect(k8sClient.Create(ctx, machine0)).To(Succeed()) + Expect(k8sClient.Create(ctx, machine1)).To(Succeed()) + Expect(k8sClient.Create(ctx, machine2)).To(Succeed()) + + return &[]machinev1beta1.Machine{*machine0, *machine1, *machine2} + } + + createAZ4Machine := func() *machinev1beta1.Machine { + machineBuilder := machinev1beta1resourcebuilder.Machine().AsMaster().WithNamespace(namespaceName) + machine := machineBuilder.WithProviderSpecBuilder(az4ProviderSpecBuilderOpenStack.WithFlavor("m1.large")).WithName("master-3").Build() + + Expect(k8sClient.Create(ctx, machine)).To(Succeed()) + + return machine + } + + createAZ5Machine := func() *machinev1beta1.Machine { + machineBuilder := machinev1beta1resourcebuilder.Machine().AsMaster().WithNamespace(namespaceName) + machine := machineBuilder.WithProviderSpecBuilder(az5ProviderSpecBuilderOpenStack.WithFlavor("m1.large")).WithName("master-4").Build() + + Expect(k8sClient.Create(ctx, machine)).To(Succeed()) + + return machine + } + + BeforeEach(func() { + + By("Setting up a namespace for the test") + ns := corev1resourcebuilder.Namespace().WithGenerateName("control-plane-machine-set-controller-").Build() + Expect(k8sClient.Create(ctx, ns)).To(Succeed()) + namespaceName = ns.GetName() + + By("Setting up a new infrastructure for the test") + // Create infrastructure object. + infra := configv1resourcebuilder.Infrastructure().WithName(infrastructureName).AsOpenStack("test").Build() + infraStatus := infra.Status.DeepCopy() + Expect(k8sClient.Create(ctx, infra)).To(Succeed()) + // Update Infrastructure Status. + Eventually(komega.UpdateStatus(infra, func() { + infra.Status = *infraStatus + })).Should(Succeed()) + + By("Setting up a manager and controller") + var err error + mgr, err = ctrl.NewManager(cfg, ctrl.Options{ + Scheme: testScheme, + MetricsBindAddress: "0", + Port: testEnv.WebhookInstallOptions.LocalServingPort, + Host: testEnv.WebhookInstallOptions.LocalServingHost, + CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, + }) + Expect(err).ToNot(HaveOccurred(), "Manager should be able to be created") + reconciler = &ControlPlaneMachineSetGeneratorReconciler{ + Client: mgr.GetClient(), + Namespace: namespaceName, + } + Expect(reconciler.SetupWithManager(mgr)).To(Succeed(), "Reconciler should be able to setup with manager") + + }) + + AfterEach(func() { + testutils.CleanupResources(Default, ctx, cfg, k8sClient, namespaceName, + &corev1.Node{}, + &machinev1beta1.Machine{}, + &configv1.Infrastructure{}, + &machinev1beta1.MachineSet{}, + &machinev1.ControlPlaneMachineSet{}, + ) + }) + + JustBeforeEach(func() { + By("Starting the manager") + mgrCancel, mgrDone = startManager(&mgr) + }) + + JustAfterEach(func() { + By("Stopping the manager") + stopManager() + }) + + Context("when a Control Plane Machine Set doesn't exist", func() { + BeforeEach(func() { + cpms = &machinev1.ControlPlaneMachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterControlPlaneMachineSetName, + Namespace: namespaceName, + }, + } + }) + + Context("with 5 Machine Sets", func() { + BeforeEach(func() { + By("Creating MachineSets") + create5MachineSets() + }) + + Context("with 3 existing control plane machines", func() { + BeforeEach(func() { + By("Creating Control Plane Machines") + create3CPMachines() + }) + + It("should create the ControlPlaneMachineSet with the expected fields", func() { + By("Checking the Control Plane Machine Set has been created") + Eventually(komega.Get(cpms)).Should(Succeed()) + Expect(cpms.Spec.State).To(Equal(machinev1.ControlPlaneMachineSetStateInactive)) + Expect(*cpms.Spec.Replicas).To(Equal(int32(3))) + }) + + It("should create the ControlPlaneMachineSet with the provider spec matching the youngest machine provider spec", func() { + By("Checking the Control Plane Machine Set has been created") + Eventually(komega.Get(cpms)).Should(Succeed()) + // In this case expect the machine Provider Spec of the youngest machine to be used here. + // In this case it should be `machine-2` given that's the one we created last. + cpmsProviderSpec, err := providerconfig.NewProviderConfigFromMachineSpec(cpms.Spec.Template.OpenShiftMachineV1Beta1Machine.Spec) + Expect(err).To(BeNil()) + + machineProviderSpec, err := providerconfig.NewProviderConfigFromMachineSpec(machine2.Spec) + Expect(err).To(BeNil()) + + // Remove from the machine Provider Spec the fields that won't be + // present on the ControlPlaneMachineSet Provider Spec. + openStackMachineProviderConfig := machineProviderSpec.OpenStack().Config() + openStackMachineProviderConfig.AvailabilityZone = "" + openStackMachineProviderConfig.RootVolume.Zone = "" + + Expect(cpmsProviderSpec.OpenStack().Config()).To(Equal(openStackMachineProviderConfig)) + }) + + Context("With additional MachineSets duplicating failure domains", func() { + BeforeEach(func() { + By("Creating additional MachineSets") + create3MachineSets() + }) + + It("should create the ControlPlaneMachineSet with only one copy of each failure domain", func() { + By("Checking the Control Plane Machine Set has been created") + Eventually(komega.Get(cpms)).Should(Succeed()) + + Expect(cpms.Spec.Template.OpenShiftMachineV1Beta1Machine.FailureDomains).To(Equal(cpms5FailureDomainsBuilderOpenStack.BuildFailureDomains())) + }) + }) + }) + }) + + Context("with 3 Machine Sets", func() { + BeforeEach(func() { + By("Creating MachineSets") + create3MachineSets() + }) + + Context("with 3 existing control plane machines", func() { + BeforeEach(func() { + By("Creating Control Plane Machines") + create3CPMachines() + }) + + It("should create the ControlPlaneMachineSet with the expected fields", func() { + By("Checking the Control Plane Machine Set has been created") + Eventually(komega.Get(cpms)).Should(Succeed()) + Expect(cpms.Spec.State).To(Equal(machinev1.ControlPlaneMachineSetStateInactive)) + Expect(*cpms.Spec.Replicas).To(Equal(int32(3))) + }) + + It("should create the ControlPlaneMachineSet with the provider spec matching the youngest machine provider spec", func() { + By("Checking the Control Plane Machine Set has been created") + Eventually(komega.Get(cpms)).Should(Succeed()) + // In this case expect the machine Provider Spec of the youngest machine to be used here. + // In this case it should be `machine-2` given that's the one we created last. + cpmsProviderSpec, err := providerconfig.NewProviderConfigFromMachineSpec(cpms.Spec.Template.OpenShiftMachineV1Beta1Machine.Spec) + Expect(err).To(BeNil()) + + machineProviderSpec, err := providerconfig.NewProviderConfigFromMachineSpec(machine2.Spec) + Expect(err).To(BeNil()) + + // Remove from the machine Provider Spec the fields that won't be + // present on the ControlPlaneMachineSet Provider Spec. + openStackMachineProviderConfig := machineProviderSpec.OpenStack().Config() + openStackMachineProviderConfig.AvailabilityZone = "" + openStackMachineProviderConfig.RootVolume.Zone = "" + + Expect(cpmsProviderSpec.OpenStack().Config()).To(Equal(openStackMachineProviderConfig)) + }) + + It("should create the ControlPlaneMachineSet with only one copy of each of the 3 failure domains", func() { + By("Checking the Control Plane Machine Set has been created") + Eventually(komega.Get(cpms)).Should(Succeed()) + + Expect(cpms.Spec.Template.OpenShiftMachineV1Beta1Machine.FailureDomains).To(Equal(cpms3FailureDomainsBuilderOpenStack.BuildFailureDomains())) + }) + + Context("With additional Machines adding additional failure domains", func() { + BeforeEach(func() { + By("Creating additional Machines") + createAZ4Machine() + createAZ5Machine() + }) + + It("should create the ControlPlaneMachineSet with only one copy of each the 5 failure domains", func() { + By("Checking the Control Plane Machine Set has been created") + Eventually(komega.Get(cpms)).Should(Succeed()) + + Expect(cpms.Spec.Template.OpenShiftMachineV1Beta1Machine.FailureDomains).To(Equal(cpms5FailureDomainsBuilderOpenStack.BuildFailureDomains())) + }) + }) + }) + }) + + Context("with only 1 existing control plane machine", func() { + var logger testutils.TestLogger + isSupportedControlPlaneMachinesNumber := false + + BeforeEach(func() { + By("Creating 1 Control Plane Machine") + machineBuilder := machinev1beta1resourcebuilder.Machine().AsMaster().WithNamespace(namespaceName) + machine2 = machineBuilder.WithProviderSpecBuilder(az3ProviderSpecBuilderOpenStack.WithFlavor("m1.large")).WithName("master-2").Build() + Expect(k8sClient.Create(ctx, machine2)).To(Succeed()) + machines := []machinev1beta1.Machine{*machine2} + + By("Invoking the check on whether the number of control plane machines in the cluster is supported") + logger = testutils.NewTestLogger() + isSupportedControlPlaneMachinesNumber = reconciler.isSupportedControlPlaneMachinesNumber(logger.Logger(), machines) + }) + + It("should have not created the ControlPlaneMachineSet", func() { + Consistently(komega.Get(cpms)).Should(MatchError("controlplanemachinesets.machine.openshift.io \"" + clusterControlPlaneMachineSetName + "\" not found")) + }) + + It("should detect the cluster has an unsupported number of control plane machines", func() { + Expect(isSupportedControlPlaneMachinesNumber).To(BeFalse()) + }) + + It("sets an appropriate log line", func() { + Eventually(logger.Entries()).Should(ConsistOf( + testutils.LogEntry{ + Level: 1, + KeysAndValues: []interface{}{"count", 1}, + Message: unsupportedNumberOfControlPlaneMachines, + }, + )) + }) + + }) + + Context("with an unsupported platform", func() { + var logger testutils.TestLogger + BeforeEach(func() { + By("Creating MachineSets") + create5MachineSets() + + By("Creating Control Plane Machines") + machines := create3CPMachines() + + logger = testutils.NewTestLogger() + generatedCPMS, err := reconciler.generateControlPlaneMachineSet(logger.Logger(), configv1.NonePlatformType, *machines, nil) + Expect(generatedCPMS).To(BeNil()) + Expect(err).To(MatchError(errUnsupportedPlatform)) + }) + + It("should have not created the ControlPlaneMachineSet", func() { + Consistently(komega.Get(cpms)).Should(MatchError("controlplanemachinesets.machine.openshift.io \"" + clusterControlPlaneMachineSetName + "\" not found")) + }) + + It("sets an appropriate log line", func() { + Eventually(logger.Entries()).Should(ConsistOf( + testutils.LogEntry{ + Level: 1, + KeysAndValues: []interface{}{"platform", configv1.NonePlatformType}, + Message: unsupportedPlatform, + }, + )) + }) + + }) + }) + + Context("when a Control Plane Machine Set exists with 5 Machine Sets", func() { + BeforeEach(func() { + By("Creating MachineSets") + create5MachineSets() + By("Creating Control Plane Machines") + create3CPMachines() + }) + + Context("with state Inactive and outdated", func() { + BeforeEach(func() { + By("Creating an outdated and Inactive Control Plane Machine Set") + // Create an Inactive ControlPlaneMachineSet with a Provider Spec that + // doesn't match the one of the youngest control plane machine (i.e. it's outdated). + cpms = cpmsInactive3FDsBuilderOpenStack.WithNamespace(namespaceName).Build() + Expect(k8sClient.Create(ctx, cpms)).To(Succeed()) + }) + + It("should recreate ControlPlaneMachineSet with the provider spec matching the youngest machine provider spec", func() { + // In this case expect the machine Provider Spec of the youngest machine to be used here. + // In this case it should be `machine-1` given that's the one we created last. + machineProviderSpec, err := providerconfig.NewProviderConfigFromMachineSpec(machine2.Spec) + Expect(err).To(BeNil()) + + // Remove from the machine Provider Spec the fields that won't be + // present on the ControlPlaneMachineSet Provider Spec. + openStackMachineProviderConfig := machineProviderSpec.OpenStack().Config() + openStackMachineProviderConfig.AvailabilityZone = "" + openStackMachineProviderConfig.RootVolume.Zone = "" + + oldUID := cpms.UID + + Eventually(komega.Object(cpms), time.Second*30).Should( + HaveField("Spec.Template.OpenShiftMachineV1Beta1Machine.Spec", + WithTransform(func(in machinev1beta1.MachineSpec) machinev1alpha1.OpenstackProviderSpec { + mPS, err := providerconfig.NewProviderConfigFromMachineSpec(in) + if err != nil { + return machinev1alpha1.OpenstackProviderSpec{} + } + + return mPS.OpenStack().Config() + }, Equal(openStackMachineProviderConfig))), + "The control plane machine provider spec should match the youngest machine's provider spec", + ) + + Expect(oldUID).NotTo(Equal(cpms.UID), + "The control plane machine set UID should differ with the old one, as it should've been deleted and recreated") + }) + + Context("With additional MachineSets duplicating failure domains", func() { + BeforeEach(func() { + By("Creating additional MachineSets") + create3MachineSets() + }) + + It("should update, but not duplicate the failure domains on the ControlPlaneMachineSet", func() { + Eventually(komega.Object(cpms)).Should(HaveField("Spec.Template.OpenShiftMachineV1Beta1Machine.FailureDomains", Equal(cpms5FailureDomainsBuilderOpenStack.BuildFailureDomains()))) + }) + }) + }) + + Context("with state Inactive and up to date", func() { + BeforeEach(func() { + By("Creating an up to date and Inactive Control Plane Machine Set") + // Create an Inactive ControlPlaneMachineSet with a Provider Spec that + // match the youngest control plane machine (i.e. it's up to date). + cpms = cpmsInactive5FDsBuilderOpenStack.WithNamespace(namespaceName).Build() + Expect(k8sClient.Create(ctx, cpms)).To(Succeed()) + }) + + It("should keep the ControlPlaneMachineSet up to date and not change it", func() { + cpmsVersion := cpms.ObjectMeta.ResourceVersion + Consistently(komega.Object(cpms)).Should(HaveField("ObjectMeta.ResourceVersion", cpmsVersion)) + }) + + }) + + Context("with state Active and outdated", func() { + BeforeEach(func() { + By("Creating an outdated and Active Control Plane Machine Set") + // Create an Active ControlPlaneMachineSet with a Provider Spec that + // doesn't match the one of the youngest control plane machine (i.e. it's outdated). + cpms = cpmsActiveOutdatedBuilderOpenStack.WithNamespace(namespaceName).Build() + Expect(k8sClient.Create(ctx, cpms)).To(Succeed()) + }) + + It("should keep the CPMS unchanged", func() { + cpmsVersion := cpms.ObjectMeta.ResourceVersion + Consistently(komega.Object(cpms)).Should(HaveField("ObjectMeta.ResourceVersion", cpmsVersion)) + }) + }) + + Context("with state Active and up to date", func() { + BeforeEach(func() { + By("Creating an up to date and Active Control Plane Machine Set") + // Create an Active ControlPlaneMachineSet with a Provider Spec that + // doesn't match the one of the youngest control plane machine (i.e. it's up to date). + cpms = cpmsActiveUpToDateBuilderOpenStack.WithNamespace(namespaceName).Build() + Expect(k8sClient.Create(ctx, cpms)).To(Succeed()) + }) + + It("should keep the ControlPlaneMachineSet unchanged", func() { + cpmsVersion := cpms.ObjectMeta.ResourceVersion + Consistently(komega.Object(cpms)).Should(HaveField("ObjectMeta.ResourceVersion", cpmsVersion)) + }) + + }) + }) + + Context("when a Control Plane Machine Set exists with 3 Machine Sets", func() { + BeforeEach(func() { + By("Creating MachineSets") + create3MachineSets() + By("Creating Control Plane Machines") + create3CPMachines() + }) + + Context("with state Inactive and outdated", func() { + BeforeEach(func() { + By("Creating an outdated and Inactive Control Plane Machine Set") + // Create an Inactive ControlPlaneMachineSet with a Provider Spec that + // doesn't match the failure domains configured. + cpms = cpmsInactive5FDsBuilderOpenStack.WithNamespace(namespaceName).Build() + Expect(k8sClient.Create(ctx, cpms)).To(Succeed()) + }) + + It("should update ControlPlaneMachineSet with the expected failure domains", func() { + Eventually(komega.Object(cpms)).Should(HaveField("Spec.Template.OpenShiftMachineV1Beta1Machine.FailureDomains", Equal(cpms3FailureDomainsBuilderOpenStack.BuildFailureDomains()))) + }) + + Context("With additional Machines adding additional failure domains", func() { + BeforeEach(func() { + By("Creating additional MachineSets") + createAZ4Machine() + createAZ5Machine() + }) + + It("should include additional failure domains from Machines, not present in the Machine Sets", func() { + Eventually(komega.Object(cpms)).Should(HaveField("Spec.Template.OpenShiftMachineV1Beta1Machine.FailureDomains", Equal(cpms5FailureDomainsBuilderOpenStack.BuildFailureDomains()))) + }) + }) + }) + }) +}) diff --git a/pkg/controllers/controlplanemachinesetgenerator/openstack.go b/pkg/controllers/controlplanemachinesetgenerator/openstack.go new file mode 100644 index 000000000..7a6e426b7 --- /dev/null +++ b/pkg/controllers/controlplanemachinesetgenerator/openstack.go @@ -0,0 +1,117 @@ +/* +Copyright 2022 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controlplanemachinesetgenerator + +import ( + "encoding/json" + "fmt" + + configv1 "github.com/openshift/api/config/v1" + machinev1 "github.com/openshift/api/machine/v1" + machinev1beta1 "github.com/openshift/api/machine/v1beta1" + machinev1builder "github.com/openshift/client-go/machine/applyconfigurations/machine/v1" + machinev1beta1builder "github.com/openshift/client-go/machine/applyconfigurations/machine/v1beta1" + "github.com/openshift/cluster-control-plane-machine-set-operator/pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain" + "github.com/openshift/cluster-control-plane-machine-set-operator/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig" + "k8s.io/apimachinery/pkg/runtime" +) + +// generateControlPlaneMachineSetOpenStackSpec generates an OpenStack flavored ControlPlaneMachineSet Spec. +func generateControlPlaneMachineSetOpenStackSpec(machines []machinev1beta1.Machine, machineSets []machinev1beta1.MachineSet) (machinev1builder.ControlPlaneMachineSetSpecApplyConfiguration, error) { + controlPlaneMachineSetMachineFailureDomainsApplyConfig, err := buildOpenStackFailureDomains(machineSets, machines) + if err != nil { + return machinev1builder.ControlPlaneMachineSetSpecApplyConfiguration{}, fmt.Errorf("failed to build ControlPlaneMachineSet's OpenStack failure domains: %w", err) + } + + controlPlaneMachineSetMachineSpecApplyConfig, err := buildControlPlaneMachineSetOpenStackMachineSpec(machines) + if err != nil { + return machinev1builder.ControlPlaneMachineSetSpecApplyConfiguration{}, fmt.Errorf("failed to build ControlPlaneMachineSet's OpenStack spec: %w", err) + } + + // We want to work with the newest machine. + controlPlaneMachineSetApplyConfigSpec := genericControlPlaneMachineSetSpec(replicas, machines[0].ObjectMeta.Labels[clusterIDLabelKey]) + controlPlaneMachineSetApplyConfigSpec.Template.OpenShiftMachineV1Beta1Machine.FailureDomains = controlPlaneMachineSetMachineFailureDomainsApplyConfig + controlPlaneMachineSetApplyConfigSpec.Template.OpenShiftMachineV1Beta1Machine.Spec = controlPlaneMachineSetMachineSpecApplyConfig + + return controlPlaneMachineSetApplyConfigSpec, nil +} + +// buildOpenStackFailureDomains builds an OpenStackFailureDomain config for the ControlPaneMachineSet from the cluster's Machines and MachineSets. +func buildOpenStackFailureDomains(machineSets []machinev1beta1.MachineSet, machines []machinev1beta1.Machine) (*machinev1builder.FailureDomainsApplyConfiguration, error) { + // Fetch failure domains from the machines + machineFailureDomains, err := providerconfig.ExtractFailureDomainsFromMachines(machines) + if err != nil { + return nil, fmt.Errorf("failed to extract failure domains from machines: %w", err) + } + + // Fetch failure domains from the machineSets + machineSetFailureDomains, err := providerconfig.ExtractFailureDomainsFromMachineSets(machineSets) + if err != nil { + return nil, fmt.Errorf("failed to extract failure domains from machine sets: %w", err) + } + + // We have to get rid of duplicates from the failure domains. + // We construct a set from the failure domains, since a set can't have duplicates. + failureDomains := failuredomain.NewSet(machineFailureDomains...) + // Construction of a union of failure domains of machines and machineSets. + failureDomains.Insert(machineSetFailureDomains...) + + openStackFailureDomains := []machinev1.OpenStackFailureDomain{} + for _, fd := range failureDomains.List() { + openStackFailureDomains = append(openStackFailureDomains, fd.OpenStack()) + } + + cpmsFailureDomain := machinev1.FailureDomains{ + OpenStack: &openStackFailureDomains, + Platform: configv1.OpenStackPlatformType, + } + + cpmsFailureDomainsApplyConfig := &machinev1builder.FailureDomainsApplyConfiguration{} + if err := convertViaJSON(cpmsFailureDomain, cpmsFailureDomainsApplyConfig); err != nil { + return nil, fmt.Errorf("failed to convert machinev1.FailureDomains to machinev1builder.FailureDomainsApplyConfiguration: %w", err) + } + + return cpmsFailureDomainsApplyConfig, nil +} + +// buildControlPlaneMachineSetOpenStackMachineSpec builds an OpenStack flavored MachineSpec for the ControlPlaneMachineSet. +func buildControlPlaneMachineSetOpenStackMachineSpec(machines []machinev1beta1.Machine) (*machinev1beta1builder.MachineSpecApplyConfiguration, error) { + // The machines slice is sorted by the creation time. + // We want to get the provider config for the newest machine. + providerConfig, err := providerconfig.NewProviderConfigFromMachineSpec(machines[0].Spec) + if err != nil { + return nil, fmt.Errorf("failed to extract machine's OpenStack providerSpec: %w", err) + } + + openStackProviderSpec := providerConfig.OpenStack().Config() + // Remove field related to the failure domain. + openStackProviderSpec.AvailabilityZone = "" + openStackProviderSpec.RootVolume.Zone = "" + + rawBytes, err := json.Marshal(openStackProviderSpec) + if err != nil { + return nil, fmt.Errorf("error marshalling OpenStack providerSpec: %w", err) + } + + re := runtime.RawExtension{ + Raw: rawBytes, + } + + return &machinev1beta1builder.MachineSpecApplyConfiguration{ + ProviderSpec: &machinev1beta1builder.ProviderSpecApplyConfiguration{Value: &re}, + }, nil +} diff --git a/pkg/controllers/controlplanemachinesetgenerator/utils.go b/pkg/controllers/controlplanemachinesetgenerator/utils.go index 3ccc0d153..52d58a4bc 100644 --- a/pkg/controllers/controlplanemachinesetgenerator/utils.go +++ b/pkg/controllers/controlplanemachinesetgenerator/utils.go @@ -60,6 +60,8 @@ func sortMachineSetsByCreationTimeAscending(machineSets []machinev1beta1.Machine } // genericControlPlaneMachineSetSpec returns a generic ControlPlaneMachineSet spec, without provider specific details. + +//nolint:unparam func genericControlPlaneMachineSetSpec(replicas int32, clusterID string) machinev1builder.ControlPlaneMachineSetSpecApplyConfiguration { labels := map[string]string{ clusterIDLabelKey: clusterID, diff --git a/pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain.go b/pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain.go index 3dd3c305b..cda1098f0 100644 --- a/pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain.go +++ b/pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain.go @@ -59,6 +59,9 @@ type FailureDomain interface { // GCP returns the GCPFailureDomain if the platform type is GCP. GCP() machinev1.GCPFailureDomain + // OpenStack returns the OpenStackFailureDomain if the platform type is OpenStack. + OpenStack() machinev1.OpenStackFailureDomain + // Equal compares the underlying failure domain. Equal(other FailureDomain) bool } @@ -67,9 +70,10 @@ type FailureDomain interface { type failureDomain struct { platformType configv1.PlatformType - aws machinev1.AWSFailureDomain - azure machinev1.AzureFailureDomain - gcp machinev1.GCPFailureDomain + aws machinev1.AWSFailureDomain + azure machinev1.AzureFailureDomain + gcp machinev1.GCPFailureDomain + openstack machinev1.OpenStackFailureDomain } // String returns a string representation of the failure domain. @@ -81,6 +85,8 @@ func (f failureDomain) String() string { return azureFailureDomainToString(f.azure) case configv1.GCPPlatformType: return gcpFailureDomainToString(f.gcp) + case configv1.OpenStackPlatformType: + return openstackFailureDomainToString(f.openstack) default: return fmt.Sprintf("%sFailureDomain{}", f.platformType) } @@ -106,6 +112,11 @@ func (f failureDomain) GCP() machinev1.GCPFailureDomain { return f.gcp } +// OpenStack returns the OpenStackFailureDomain if the platform type is OpenStack. +func (f failureDomain) OpenStack() machinev1.OpenStackFailureDomain { + return f.openstack +} + // Equal compares the underlying failure domain. func (f failureDomain) Equal(other FailureDomain) bool { if other == nil { @@ -123,6 +134,8 @@ func (f failureDomain) Equal(other FailureDomain) bool { return f.azure == other.Azure() case configv1.GCPPlatformType: return f.gcp == other.GCP() + case configv1.OpenStackPlatformType: + return f.openstack == other.OpenStack() } return true @@ -138,6 +151,8 @@ func NewFailureDomains(failureDomains machinev1.FailureDomains) ([]FailureDomain return newAzureFailureDomains(failureDomains) case configv1.GCPPlatformType: return newGCPFailureDomains(failureDomains) + case configv1.OpenStackPlatformType: + return newOpenStackFailureDomains(failureDomains) case configv1.PlatformType(""): // An empty failure domains definition is allowed. return nil, nil @@ -188,6 +203,20 @@ func newGCPFailureDomains(failureDomains machinev1.FailureDomains) ([]FailureDom return foundFailureDomains, nil } +// newOpenStackFailureDomains constructs a slice of OpenStack FailureDomain from machinev1.FailureDomains. +func newOpenStackFailureDomains(failureDomains machinev1.FailureDomains) ([]FailureDomain, error) { + foundFailureDomains := []FailureDomain{} + if failureDomains.OpenStack == nil { + return foundFailureDomains, errMissingFailureDomain + } + + for _, failureDomain := range *failureDomains.OpenStack { + foundFailureDomains = append(foundFailureDomains, NewOpenStackFailureDomain(failureDomain)) + } + + return foundFailureDomains, nil +} + // NewAWSFailureDomain creates an AWS failure domain from the machinev1.AWSFailureDomain. // Note this is exported to allow other packages to construct individual failure domains // in tests. @@ -214,6 +243,14 @@ func NewGCPFailureDomain(fd machinev1.GCPFailureDomain) FailureDomain { } } +// NewOpenStackFailureDomain creates an OpenStack failure domain from the machinev1.OpenStackFailureDomain. +func NewOpenStackFailureDomain(fd machinev1.OpenStackFailureDomain) FailureDomain { + return &failureDomain{ + platformType: configv1.OpenStackPlatformType, + openstack: fd, + } +} + // NewGenericFailureDomain creates a dummy failure domain for generic platforms that don't support failure domains. func NewGenericFailureDomain() FailureDomain { return failureDomain{} @@ -276,3 +313,24 @@ func gcpFailureDomainToString(fd machinev1.GCPFailureDomain) string { return unknownFailureDomain } + +// openstackFailureDomainToString converts the OpenStackFailureDomain into a string. +func openstackFailureDomainToString(fd machinev1.OpenStackFailureDomain) string { + // ComputeAvailabilityZone only + if fd.ComputeAvailabilityZone != "" && fd.StorageAvailabilityZone == "" { + return fmt.Sprintf("OpenStackFailureDomain{ComputeAvailabilityZone:%s}", fd.ComputeAvailabilityZone) + } + + // StorageAvailabilityZone only + if fd.ComputeAvailabilityZone == "" && fd.StorageAvailabilityZone != "" { + return fmt.Sprintf("OpenStackFailureDomain{StorageAvailabilityZone:%s}", fd.StorageAvailabilityZone) + } + + // ComputeAvailabilityZone and StorageAvailabilityZone + if fd.ComputeAvailabilityZone != "" && fd.StorageAvailabilityZone != "" { + return fmt.Sprintf("OpenStackFailureDomain{ComputeAvailabilityZone:%s, StorageAvailabilityZone:%s}", fd.ComputeAvailabilityZone, fd.StorageAvailabilityZone) + } + + // If a machine has no explicit zone, then the corresponding CPMS failure domain should have no explicit zone. + return "OpenStackFailureDomain{}" +} diff --git a/pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain_test.go b/pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain_test.go index 2d0986d16..9a1aacd10 100644 --- a/pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain_test.go +++ b/pkg/machineproviders/providers/openshift/machine/v1beta1/failuredomain/failuredomain_test.go @@ -173,6 +173,50 @@ var _ = Describe("FailureDomains", func() { }) }) + Context("With OpenStack failure domain configuration", func() { + var failureDomains []FailureDomain + var err error + + BeforeEach(func() { + config := machinev1resourcebuilder.OpenStackFailureDomains().BuildFailureDomains() + + failureDomains, err = NewFailureDomains(config) + }) + + It("should not error", func() { + Expect(err).ToNot(HaveOccurred()) + }) + + It("should construct a list of failure domains", func() { + Expect(failureDomains).To(ConsistOf( + HaveField("String()", "OpenStackFailureDomain{ComputeAvailabilityZone:az0, StorageAvailabilityZone:az0}"), + HaveField("String()", "OpenStackFailureDomain{ComputeAvailabilityZone:az1}"), + HaveField("String()", "OpenStackFailureDomain{StorageAvailabilityZone:az2}"), + HaveField("String()", "OpenStackFailureDomain{}"), + )) + }) + }) + + Context("With invalid OpenStack failure domain configuration", func() { + var failureDomains []FailureDomain + var err error + + BeforeEach(func() { + config := machinev1resourcebuilder.OpenStackFailureDomains().BuildFailureDomains() + config.OpenStack = nil + + failureDomains, err = NewFailureDomains(config) + }) + + It("returns an error", func() { + Expect(err).To(MatchError("missing failure domain configuration")) + }) + + It("returns an empty list of failure domains", func() { + Expect(failureDomains).To(BeEmpty()) + }) + }) + Context("With an unsupported platform type", func() { var failureDomains []FailureDomain var err error @@ -296,6 +340,55 @@ var _ = Describe("FailureDomains", func() { }) }) + Context("an OpenStack failure domain", func() { + var fd failureDomain + + BeforeEach(func() { + fd = failureDomain{ + platformType: configv1.OpenStackPlatformType, + } + }) + + Context("with a Compute and Storage availability zone", func() { + BeforeEach(func() { + fd.openstack = machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az0"). + WithStorageAvailabilityZone("az1").Build() + }) + + It("returns the Compute and Storage availability zones for String()", func() { + Expect(fd.String()).To(Equal("OpenStackFailureDomain{ComputeAvailabilityZone:az0, StorageAvailabilityZone:az1}")) + }) + }) + + Context("with a Compute availability zone only", func() { + BeforeEach(func() { + fd.openstack = machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az0").Build() + }) + + It("returns the Compute availability zone for String()", func() { + Expect(fd.String()).To(Equal("OpenStackFailureDomain{ComputeAvailabilityZone:az0}")) + }) + }) + Context("with a Storage availability zone only", func() { + BeforeEach(func() { + fd.openstack = machinev1resourcebuilder.OpenStackFailureDomain().WithStorageAvailabilityZone("az0").Build() + }) + + It("returns the Storage availability zone for String()", func() { + Expect(fd.String()).To(Equal("OpenStackFailureDomain{StorageAvailabilityZone:az0}")) + }) + }) + Context("with no availability zones", func() { + BeforeEach(func() { + fd.openstack = machinev1resourcebuilder.OpenStackFailureDomain().Build() + }) + + It("returns for String()", func() { + Expect(fd.String()).To(Equal("OpenStackFailureDomain{}")) + }) + }) + }) + Context("Equal", func() { var fd1 failureDomain var fd2 failureDomain @@ -381,6 +474,23 @@ var _ = Describe("FailureDomains", func() { }) }) + Context("With two identical OpenStack failure domains", func() { + BeforeEach(func() { + fd1 = failureDomain{ + platformType: configv1.OpenStackPlatformType, + openstack: machinev1resourcebuilder.OpenStackFailureDomain().WithStorageAvailabilityZone("az0").Build(), + } + fd2 = failureDomain{ + platformType: configv1.OpenStackPlatformType, + openstack: machinev1resourcebuilder.OpenStackFailureDomain().WithStorageAvailabilityZone("az0").Build(), + } + }) + + It("returns true", func() { + Expect(fd1.Equal(fd2)).To(BeTrue()) + }) + }) + Context("With two different Azure failure domains", func() { BeforeEach(func() { fd1 = failureDomain{ @@ -398,6 +508,23 @@ var _ = Describe("FailureDomains", func() { }) }) + Context("With two different OpenStack failure domains", func() { + BeforeEach(func() { + fd1 = failureDomain{ + platformType: configv1.OpenStackPlatformType, + openstack: machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az0").Build(), + } + fd2 = failureDomain{ + platformType: configv1.GCPPlatformType, + openstack: machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az1").Build(), + } + }) + + It("returns false", func() { + Expect(fd1.Equal(fd2)).To(BeFalse()) + }) + }) + Context("With different failure domains platform", func() { BeforeEach(func() { fd1 = failureDomain{ diff --git a/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/openstack.go b/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/openstack.go new file mode 100644 index 000000000..bf996996d --- /dev/null +++ b/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/openstack.go @@ -0,0 +1,80 @@ +/* +Copyright 2022 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package providerconfig + +import ( + "encoding/json" + "fmt" + + v1 "github.com/openshift/api/config/v1" + machinev1 "github.com/openshift/api/machine/v1" + machinev1alpha1 "github.com/openshift/api/machine/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" +) + +// OpenStackProviderConfig holds the provider spec of an OpenStack Machine. +// It allows external code to extract and inject failure domain information, +// as well as gathering the stored config. +type OpenStackProviderConfig struct { + providerConfig machinev1alpha1.OpenstackProviderSpec +} + +// InjectFailureDomain returns a new OpenStackProviderConfig configured with the failure domain +// information provided. +func (a OpenStackProviderConfig) InjectFailureDomain(fd machinev1.OpenStackFailureDomain) OpenStackProviderConfig { + newOpenStackProviderConfig := a + + newOpenStackProviderConfig.providerConfig.AvailabilityZone = fd.ComputeAvailabilityZone + newOpenStackProviderConfig.providerConfig.RootVolume.Zone = fd.StorageAvailabilityZone + + return newOpenStackProviderConfig +} + +// ExtractFailureDomain returns an OpenStackFailureDomain based on the failure domain +// information stored within the OpenStackProviderConfig. +func (a OpenStackProviderConfig) ExtractFailureDomain() machinev1.OpenStackFailureDomain { + return machinev1.OpenStackFailureDomain{ + ComputeAvailabilityZone: a.providerConfig.AvailabilityZone, + StorageAvailabilityZone: a.providerConfig.RootVolume.Zone, + } +} + +// Config returns the stored OpenStackMachineProviderSpec. +func (a OpenStackProviderConfig) Config() machinev1alpha1.OpenstackProviderSpec { + return a.providerConfig +} + +// newOpenStackProviderConfig creates an OpenStack type ProviderConfig from the raw extension. +// It should return an error if the provided RawExtension does not represent +// an OpenStackMachineProviderConfig. +func newOpenStackProviderConfig(raw *runtime.RawExtension) (ProviderConfig, error) { + OpenstackProviderSpec := machinev1alpha1.OpenstackProviderSpec{} + if err := json.Unmarshal(raw.Raw, &OpenstackProviderSpec); err != nil { + return nil, fmt.Errorf("could not unmarshal provider spec: %w", err) + } + + OpenStackProviderConfig := OpenStackProviderConfig{ + providerConfig: OpenstackProviderSpec, + } + + config := providerConfig{ + platformType: v1.OpenStackPlatformType, + openstack: OpenStackProviderConfig, + } + + return config, nil +} diff --git a/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/openstack_test.go b/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/openstack_test.go new file mode 100644 index 000000000..b80044831 --- /dev/null +++ b/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/openstack_test.go @@ -0,0 +1,117 @@ +/* +Copyright 2023 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package providerconfig + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + configv1 "github.com/openshift/api/config/v1" + machinev1alpha1 "github.com/openshift/api/machine/v1alpha1" + machinev1resourcebuilder "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/machine/v1" + machinev1beta1resourcebuilder "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/machine/v1beta1" +) + +var _ = Describe("OpenStack Provider Config", func() { + var providerConfig OpenStackProviderConfig + + zone1 := "az1" + zone2 := "az2" + + rootVolume1 := &machinev1alpha1.RootVolume{ + Zone: zone1, + } + + BeforeEach(func() { + machineProviderConfig := machinev1beta1resourcebuilder.OpenStackProviderSpec(). + WithZone(zone1). + WithRootVolume(rootVolume1). + Build() + + providerConfig = OpenStackProviderConfig{ + providerConfig: *machineProviderConfig, + } + }) + + Context("ExtractFailureDomain", func() { + It("returns the configured failure domain", func() { + expected := machinev1resourcebuilder.OpenStackFailureDomain(). + WithComputeAvailabilityZone(zone1). + WithStorageAvailabilityZone(zone1). + Build() + + Expect(providerConfig.ExtractFailureDomain()).To(Equal(expected)) + }) + }) + + Context("when the failuredomain is changed after initialisation", func() { + var changedProviderConfig OpenStackProviderConfig + + BeforeEach(func() { + changedFailureDomain := machinev1resourcebuilder.OpenStackFailureDomain(). + WithComputeAvailabilityZone(zone2). + WithStorageAvailabilityZone(zone1). + Build() + + changedProviderConfig = providerConfig.InjectFailureDomain(changedFailureDomain) + }) + + Context("ExtractFailureDomain", func() { + It("returns the changed failure domain from the changed config", func() { + expected := machinev1resourcebuilder.OpenStackFailureDomain(). + WithComputeAvailabilityZone(zone2). + WithStorageAvailabilityZone(zone1). + Build() + + Expect(changedProviderConfig.ExtractFailureDomain()).To(Equal(expected)) + }) + + It("returns the original failure domain from the original config", func() { + expected := machinev1resourcebuilder.OpenStackFailureDomain(). + WithComputeAvailabilityZone(zone1). + WithStorageAvailabilityZone(zone1). + Build() + + Expect(providerConfig.ExtractFailureDomain()).To(Equal(expected)) + }) + }) + }) + + Context("newOpenStackProviderConfig", func() { + var providerConfig ProviderConfig + var expectedOpenStackConfig machinev1alpha1.OpenstackProviderSpec + + BeforeEach(func() { + configBuilder := machinev1beta1resourcebuilder.OpenStackProviderSpec() + expectedOpenStackConfig = *configBuilder.Build() + rawConfig := configBuilder.BuildRawExtension() + + var err error + providerConfig, err = newOpenStackProviderConfig(rawConfig) + Expect(err).ToNot(HaveOccurred()) + }) + + It("sets the type to OpenStack", func() { + Expect(providerConfig.Type()).To(Equal(configv1.OpenStackPlatformType)) + }) + + It("returns the correct OpenStack config", func() { + Expect(providerConfig.OpenStack()).ToNot(BeNil()) + Expect(providerConfig.OpenStack().Config()).To(Equal(expectedOpenStackConfig)) + }) + }) +}) diff --git a/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/providerconfig.go b/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/providerconfig.go index 4d2c865f2..7123f7121 100644 --- a/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/providerconfig.go +++ b/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/providerconfig.go @@ -79,6 +79,9 @@ type ProviderConfig interface { // GCP returns the GCPProviderConfig if the platform type is GCP. GCP() GCPProviderConfig + // OpenStack returns the OpenStackProviderConfig if the platform type is OpenStack. + OpenStack() OpenStackProviderConfig + // Generic returns the GenericProviderConfig if we are on a platform that is using generic provider abstraction. Generic() GenericProviderConfig } @@ -115,6 +118,8 @@ func newProviderConfigFromProviderSpec(providerSpec machinev1beta1.ProviderSpec, return newAzureProviderConfig(providerSpec.Value) case configv1.GCPPlatformType: return newGCPProviderConfig(providerSpec.Value) + case configv1.OpenStackPlatformType: + return newOpenStackProviderConfig(providerSpec.Value) case configv1.NonePlatformType: return nil, fmt.Errorf("%w: %s", errUnsupportedPlatformType, platformType) default: @@ -129,6 +134,7 @@ type providerConfig struct { azure AzureProviderConfig gcp GCPProviderConfig generic GenericProviderConfig + openstack OpenStackProviderConfig } // InjectFailureDomain is used to inject a failure domain into the ProviderConfig. @@ -148,6 +154,8 @@ func (p providerConfig) InjectFailureDomain(fd failuredomain.FailureDomain) (Pro newConfig.azure = p.Azure().InjectFailureDomain(fd.Azure()) case configv1.GCPPlatformType: newConfig.gcp = p.GCP().InjectFailureDomain(fd.GCP()) + case configv1.OpenStackPlatformType: + newConfig.openstack = p.OpenStack().InjectFailureDomain(fd.OpenStack()) case configv1.NonePlatformType: return nil, fmt.Errorf("%w: %s", errUnsupportedPlatformType, p.platformType) } @@ -164,6 +172,8 @@ func (p providerConfig) ExtractFailureDomain() failuredomain.FailureDomain { return failuredomain.NewAzureFailureDomain(p.Azure().ExtractFailureDomain()) case configv1.GCPPlatformType: return failuredomain.NewGCPFailureDomain(p.GCP().ExtractFailureDomain()) + case configv1.OpenStackPlatformType: + return failuredomain.NewOpenStackFailureDomain(p.OpenStack().ExtractFailureDomain()) case configv1.NonePlatformType: return nil default: @@ -189,6 +199,8 @@ func (p providerConfig) Diff(other ProviderConfig) ([]string, error) { return deep.Equal(p.azure.providerConfig, other.Azure().providerConfig), nil case configv1.GCPPlatformType: return deep.Equal(p.gcp.providerConfig, other.GCP().providerConfig), nil + case configv1.OpenStackPlatformType: + return deep.Equal(p.openstack.providerConfig, other.OpenStack().providerConfig), nil case configv1.NonePlatformType: return nil, errUnsupportedPlatformType default: @@ -213,6 +225,8 @@ func (p providerConfig) Equal(other ProviderConfig) (bool, error) { return reflect.DeepEqual(p.azure.providerConfig, other.Azure().providerConfig), nil case configv1.GCPPlatformType: return reflect.DeepEqual(p.gcp.providerConfig, other.GCP().providerConfig), nil + case configv1.OpenStackPlatformType: + return reflect.DeepEqual(p.openstack.providerConfig, other.OpenStack().providerConfig), nil case configv1.NonePlatformType: return false, errUnsupportedPlatformType default: @@ -234,6 +248,8 @@ func (p providerConfig) RawConfig() ([]byte, error) { rawConfig, err = json.Marshal(p.azure.providerConfig) case configv1.GCPPlatformType: rawConfig, err = json.Marshal(p.gcp.providerConfig) + case configv1.OpenStackPlatformType: + rawConfig, err = json.Marshal(p.openstack.providerConfig) case configv1.NonePlatformType: return nil, errUnsupportedPlatformType default: @@ -267,6 +283,11 @@ func (p providerConfig) GCP() GCPProviderConfig { return p.gcp } +// OpenStack returns the OpenStackProviderConfig if the platform type is OpenStack. +func (p providerConfig) OpenStack() OpenStackProviderConfig { + return p.openstack +} + // Generic returns the GenericProviderConfig if the platform type is generic. func (p providerConfig) Generic() GenericProviderConfig { return p.generic @@ -279,6 +300,7 @@ func getPlatformTypeFromProviderSpecKind(kind string) configv1.PlatformType { "AWSMachineProviderConfig": configv1.AWSPlatformType, "AzureMachineProviderSpec": configv1.AzurePlatformType, "GCPMachineProviderSpec": configv1.GCPPlatformType, + "OpenstackProviderSpec": configv1.OpenStackPlatformType, } platformType, ok := providerSpecKindToPlatformType[kind] diff --git a/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/providerconfig_test.go b/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/providerconfig_test.go index 4efd94e44..d0a708337 100644 --- a/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/providerconfig_test.go +++ b/pkg/machineproviders/providers/openshift/machine/v1beta1/providerconfig/providerconfig_test.go @@ -23,6 +23,7 @@ import ( configv1 "github.com/openshift/api/config/v1" machinev1 "github.com/openshift/api/machine/v1" + machinev1alpha1 "github.com/openshift/api/machine/v1alpha1" machinev1beta1 "github.com/openshift/api/machine/v1beta1" "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder" machinev1resourcebuilder "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder/machine/v1" @@ -110,6 +111,18 @@ var _ = Describe("Provider Config", func() { providerSpecBuilder: machinev1beta1resourcebuilder.GCPProviderSpec(), providerConfigMatcher: HaveField("GCP().Config()", *machinev1beta1resourcebuilder.GCPProviderSpec().Build()), }), + Entry("with an OpenStack config with failure domains", providerConfigTableInput{ + expectedPlatformType: configv1.OpenStackPlatformType, + failureDomainsBuilder: machinev1resourcebuilder.OpenStackFailureDomains(), + providerSpecBuilder: machinev1beta1resourcebuilder.OpenStackProviderSpec(), + providerConfigMatcher: HaveField("OpenStack().Config()", *machinev1beta1resourcebuilder.OpenStackProviderSpec().Build()), + }), + Entry("with an OpenStack config without failure domains", providerConfigTableInput{ + expectedPlatformType: configv1.OpenStackPlatformType, + failureDomainsBuilder: nil, + providerSpecBuilder: machinev1beta1resourcebuilder.OpenStackProviderSpec(), + providerConfigMatcher: HaveField("OpenStack().Config()", *machinev1beta1resourcebuilder.OpenStackProviderSpec().Build()), + }), ) }) @@ -236,6 +249,62 @@ var _ = Describe("Provider Config", func() { matchPath: "GCP().Config().Zone", matchExpectation: "us-central1-b", }), + Entry("when keeping an OpenStack compute availability zone the same", injectFailureDomainTableInput{ + providerConfig: &providerConfig{ + platformType: configv1.OpenStackPlatformType, + openstack: OpenStackProviderConfig{ + providerConfig: *machinev1beta1resourcebuilder.OpenStackProviderSpec().WithZone("az0").Build(), + }, + }, + failureDomain: failuredomain.NewOpenStackFailureDomain( + machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az0").Build(), + ), + matchPath: "OpenStack().Config().AvailabilityZone", + matchExpectation: "az0", + }), + Entry("when keeping an OpenStack volume availability zone the same", injectFailureDomainTableInput{ + providerConfig: &providerConfig{ + platformType: configv1.OpenStackPlatformType, + openstack: OpenStackProviderConfig{ + providerConfig: *machinev1beta1resourcebuilder.OpenStackProviderSpec().WithRootVolume(&machinev1alpha1.RootVolume{ + Zone: "az1", + }).Build(), + }, + }, + failureDomain: failuredomain.NewOpenStackFailureDomain( + machinev1resourcebuilder.OpenStackFailureDomain().WithStorageAvailabilityZone("az1").Build(), + ), + matchPath: "OpenStack().Config().RootVolume.Zone", + matchExpectation: "az1", + }), + Entry("when changing an OpenStack compute availability zone", injectFailureDomainTableInput{ + providerConfig: &providerConfig{ + platformType: configv1.OpenStackPlatformType, + openstack: OpenStackProviderConfig{ + providerConfig: *machinev1beta1resourcebuilder.OpenStackProviderSpec().WithZone("az0").Build(), + }, + }, + failureDomain: failuredomain.NewOpenStackFailureDomain( + machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az1").Build(), + ), + matchPath: "OpenStack().Config().AvailabilityZone", + matchExpectation: "az1", + }), + Entry("when changing an OpenStack volume availability zone", injectFailureDomainTableInput{ + providerConfig: &providerConfig{ + platformType: configv1.OpenStackPlatformType, + openstack: OpenStackProviderConfig{ + providerConfig: *machinev1beta1resourcebuilder.OpenStackProviderSpec().WithRootVolume(&machinev1alpha1.RootVolume{ + Zone: "az0", + }).Build(), + }, + }, + failureDomain: failuredomain.NewOpenStackFailureDomain( + machinev1resourcebuilder.OpenStackFailureDomain().WithStorageAvailabilityZone("az1").Build(), + ), + matchPath: "OpenStack().Config().RootVolume.Zone", + matchExpectation: "az1", + }), ) }) @@ -287,6 +356,11 @@ var _ = Describe("Provider Config", func() { providerSpecBuilder: machinev1beta1resourcebuilder.GCPProviderSpec(), providerConfigMatcher: HaveField("GCP().Config()", *machinev1beta1resourcebuilder.GCPProviderSpec().Build()), }), + Entry("with an OpenStack config with failure domains", providerConfigTableInput{ + expectedPlatformType: configv1.OpenStackPlatformType, + providerSpecBuilder: machinev1beta1resourcebuilder.OpenStackProviderSpec(), + providerConfigMatcher: HaveField("OpenStack().Config()", *machinev1beta1resourcebuilder.OpenStackProviderSpec().Build()), + }), ) }) @@ -369,6 +443,10 @@ var _ = Describe("Provider Config", func() { }}, } + rootVolume := &machinev1alpha1.RootVolume{ + Zone: "az2", + } + DescribeTable("should correctly extract the failure domain", func(in extractFailureDomainTableInput) { fd := in.providerConfig.ExtractFailureDomain() @@ -418,6 +496,17 @@ var _ = Describe("Provider Config", func() { machinev1resourcebuilder.GCPFailureDomain().WithZone("us-central1-a").Build(), ), }), + Entry("with an OpenStack 2 failure domain", extractFailureDomainTableInput{ + providerConfig: &providerConfig{ + platformType: configv1.OpenStackPlatformType, + openstack: OpenStackProviderConfig{ + providerConfig: *machinev1beta1resourcebuilder.OpenStackProviderSpec().WithRootVolume(rootVolume).WithZone("az2").Build(), + }, + }, + expectedFailureDomain: failuredomain.NewOpenStackFailureDomain( + machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az2").WithStorageAvailabilityZone("az2").Build(), + ), + }), Entry("with a VSphere dummy failure domain", extractFailureDomainTableInput{ providerConfig: &providerConfig{ platformType: configv1.VSpherePlatformType, @@ -438,6 +527,10 @@ var _ = Describe("Provider Config", func() { expectedError error } + rootVolume := &machinev1alpha1.RootVolume{ + Zone: "az0", + } + DescribeTable("should compare provider configs", func(in equalTableInput) { equal, err := in.basePC.Equal(in.comparePC) @@ -556,6 +649,36 @@ var _ = Describe("Provider Config", func() { }, expectedEqual: false, }), + Entry("with matching OpenStack configs", equalTableInput{ + basePC: &providerConfig{ + platformType: configv1.OpenStackPlatformType, + openstack: OpenStackProviderConfig{ + providerConfig: *machinev1beta1resourcebuilder.OpenStackProviderSpec().WithZone("az0").WithRootVolume(rootVolume).Build(), + }, + }, + comparePC: &providerConfig{ + platformType: configv1.OpenStackPlatformType, + openstack: OpenStackProviderConfig{ + providerConfig: *machinev1beta1resourcebuilder.OpenStackProviderSpec().WithZone("az0").WithRootVolume(rootVolume).Build(), + }, + }, + expectedEqual: true, + }), + Entry("with mis-matched OpenStack configs", equalTableInput{ + basePC: &providerConfig{ + platformType: configv1.OpenStackPlatformType, + openstack: OpenStackProviderConfig{ + providerConfig: *machinev1beta1resourcebuilder.OpenStackProviderSpec().WithZone("az0").WithRootVolume(rootVolume).Build(), + }, + }, + comparePC: &providerConfig{ + platformType: configv1.OpenStackPlatformType, + openstack: OpenStackProviderConfig{ + providerConfig: *machinev1beta1resourcebuilder.OpenStackProviderSpec().WithZone("az1").WithRootVolume(rootVolume).Build(), + }, + }, + expectedEqual: false, + }), Entry("with matching Generic configs", equalTableInput{ basePC: &providerConfig{ platformType: configv1.VSpherePlatformType, @@ -650,6 +773,15 @@ var _ = Describe("Provider Config", func() { }, expectedOut: machinev1beta1resourcebuilder.GCPProviderSpec().BuildRawExtension().Raw, }), + Entry("with an OpenStack config", rawConfigTableInput{ + providerConfig: &providerConfig{ + platformType: configv1.OpenStackPlatformType, + openstack: OpenStackProviderConfig{ + providerConfig: *machinev1beta1resourcebuilder.OpenStackProviderSpec().Build(), + }, + }, + expectedOut: machinev1beta1resourcebuilder.OpenStackProviderSpec().BuildRawExtension().Raw, + }), Entry("with a VSphere config", rawConfigTableInput{ providerConfig: providerConfig{ platformType: configv1.VSpherePlatformType, diff --git a/pkg/webhooks/controlplanemachineset/webhooks.go b/pkg/webhooks/controlplanemachineset/webhooks.go index 575283a21..018c9201d 100644 --- a/pkg/webhooks/controlplanemachineset/webhooks.go +++ b/pkg/webhooks/controlplanemachineset/webhooks.go @@ -272,6 +272,8 @@ func validateOpenShiftProviderConfig(parentPath *field.Path, template machinev1. return validateOpenShiftAzureProviderConfig(providerSpecPath.Child("value"), providerConfig.Azure()) case configv1.GCPPlatformType: return validateOpenShiftGCPProviderConfig(providerSpecPath.Child("value"), providerConfig.GCP()) + case configv1.OpenStackPlatformType: + return validateOpenShiftOpenStackProviderConfig(providerSpecPath.Child("value"), providerConfig.OpenStack()) } return []error{} @@ -297,6 +299,12 @@ func validateOpenShiftGCPProviderConfig(parentPath *field.Path, providerConfig p return []error{} } +// validateOpenShiftOpenStackProviderConfig runs OpenStack specific checks on the provider config on the ControlPlaneMachineSet. +// This ensure that the ControlPlaneMachineSet can safely replace OpenStack control plane machines. +func validateOpenShiftOpenStackProviderConfig(parentPath *field.Path, providerConfig providerconfig.OpenStackProviderConfig) []error { + return []error{} +} + // fetchControlPlaneMachines returns all control plane machines in the cluster. func (r *ControlPlaneMachineSetWebhook) fetchControlPlaneMachines(ctx context.Context) ([]machinev1beta1.Machine, error) { machineList := machinev1beta1.MachineList{} diff --git a/pkg/webhooks/controlplanemachineset/webhooks_test.go b/pkg/webhooks/controlplanemachineset/webhooks_test.go index f60ae5b43..52f7be06c 100644 --- a/pkg/webhooks/controlplanemachineset/webhooks_test.go +++ b/pkg/webhooks/controlplanemachineset/webhooks_test.go @@ -25,6 +25,7 @@ import ( configv1 "github.com/openshift/api/config/v1" machinev1 "github.com/openshift/api/machine/v1" + machinev1alpha1 "github.com/openshift/api/machine/v1alpha1" machinev1beta1 "github.com/openshift/api/machine/v1beta1" "github.com/openshift/cluster-api-actuator-pkg/testutils" "github.com/openshift/cluster-api-actuator-pkg/testutils/resourcebuilder" @@ -699,6 +700,86 @@ var _ = Describe("Webhooks", func() { Expect(k8sClient.Create(ctx, cpms)).To(Succeed()) }) }) + + Context("on OpenStack", func() { + var zone1Builder = machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az1").WithStorageAvailabilityZone("az1") + var zone2Builder = machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az2").WithStorageAvailabilityZone("az2") + var zone3Builder = machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az3").WithStorageAvailabilityZone("az3") + var zone4Builder = machinev1resourcebuilder.OpenStackFailureDomain().WithComputeAvailabilityZone("az4").WithStorageAvailabilityZone("az4") + + BeforeEach(func() { + providerSpec := machinev1beta1resourcebuilder.OpenStackProviderSpec() + machineTemplate = machinev1resourcebuilder.OpenShiftMachineV1Beta1Template().WithProviderSpecBuilder(providerSpec) + // Default CPMS builder should be valid, individual tests will override to make it invalid + builder = machinev1resourcebuilder.ControlPlaneMachineSet().WithNamespace(namespaceName).WithMachineTemplateBuilder(machineTemplate) + + machineBuilder := machinev1beta1resourcebuilder.Machine().WithNamespace(namespaceName) + + By("Creating a selection of Machines") + for _, az := range []string{"az1", "az2", "az3"} { + rootVolume := &machinev1alpha1.RootVolume{ + Zone: az, + } + controlPlaneMachineBuilder := machineBuilder.WithGenerateName("control-plane-machine-").AsMaster().WithProviderSpecBuilder(providerSpec.WithZone(az).WithRootVolume(rootVolume)) + + controlPlaneMachine := controlPlaneMachineBuilder.Build() + Expect(k8sClient.Create(ctx, controlPlaneMachine)).To(Succeed()) + } + }) + + It("with a valid failure domains spec", func() { + cpms := builder.WithMachineTemplateBuilder(machineTemplate.WithFailureDomainsBuilder( + machinev1resourcebuilder.OpenStackFailureDomains().WithFailureDomainBuilders( + zone1Builder, + zone2Builder, + zone3Builder, + ), + )).Build() + + Expect(k8sClient.Create(ctx, cpms)).To(Succeed()) + }) + + It("with a mismatched failure domains spec", func() { + cpms := builder.WithMachineTemplateBuilder(machineTemplate.WithFailureDomainsBuilder( + machinev1resourcebuilder.OpenStackFailureDomains().WithFailureDomainBuilders( + zone1Builder, + zone2Builder, + zone4Builder, + ), + )).Build() + + Expect(k8sClient.Create(ctx, cpms)).To(MatchError( + ContainSubstring("spec.template.machines_v1beta1_machine_openshift_io.failureDomains: Forbidden: control plane machines are using unspecified failure domain(s) [OpenStackFailureDomain{ComputeAvailabilityZone:az3, StorageAvailabilityZone:az3}]"), + )) + }) + + It("when reducing the availability", func() { + cpms := builder.WithMachineTemplateBuilder(machineTemplate.WithFailureDomainsBuilder( + machinev1resourcebuilder.OpenStackFailureDomains().WithFailureDomainBuilders( + zone1Builder, + zone2Builder, + ), + )).Build() + + Expect(k8sClient.Create(ctx, cpms)).To(MatchError( + ContainSubstring("spec.template.machines_v1beta1_machine_openshift_io.failureDomains: Forbidden: control plane machines are using unspecified failure domain(s) [OpenStackFailureDomain{ComputeAvailabilityZone:az3, StorageAvailabilityZone:az3}]"), + )) + }) + + It("when increasing the availability", func() { + cpms := builder.WithMachineTemplateBuilder(machineTemplate.WithFailureDomainsBuilder( + machinev1resourcebuilder.OpenStackFailureDomains().WithFailureDomainBuilders( + zone1Builder, + zone2Builder, + zone3Builder, + zone4Builder, + ), + )).Build() + + Expect(k8sClient.Create(ctx, cpms)).To(Succeed()) + }) + }) + }) Context("on update", func() { @@ -947,5 +1028,70 @@ var _ = Describe("Webhooks", func() { })()).Should(MatchError(ContainSubstring("ControlPlaneMachineSet.machine.openshift.io \"cluster\" is invalid: spec.selector: Invalid value: \"object\": selector is immutable")), "The selector should be immutable") }) }) + + Context("on OpenStack", func() { + BeforeEach(func() { + providerSpec := machinev1beta1resourcebuilder.OpenStackProviderSpec() + machineTemplate := machinev1resourcebuilder.OpenShiftMachineV1Beta1Template().WithProviderSpecBuilder(providerSpec) + // Default CPMS builder should be valid + cpms = machinev1resourcebuilder.ControlPlaneMachineSet().WithNamespace(namespaceName).WithMachineTemplateBuilder(machineTemplate).Build() + + machineBuilder := machinev1beta1resourcebuilder.Machine().WithNamespace(namespaceName) + controlPlaneMachineBuilder := machineBuilder.WithGenerateName("control-plane-machine-").AsMaster().WithProviderSpecBuilder(providerSpec) + By("Creating a selection of Machines") + for i := 0; i < 3; i++ { + controlPlaneMachine := controlPlaneMachineBuilder.Build() + Expect(k8sClient.Create(ctx, controlPlaneMachine)).To(Succeed()) + } + + By("Creating a valid ControlPlaneMachineSet") + Expect(k8sClient.Create(ctx, cpms)).To(Succeed()) + }) + + It("with 4 replicas", func() { + // This is an openapi validation but it makes sense to include it here as well + Expect(komega.Update(cpms, func() { + four := int32(4) + cpms.Spec.Replicas = &four + })()).Should(MatchError(ContainSubstring("Unsupported value: 4: supported values: \"3\", \"5\""))) + }) + + It("with 5 replicas", func() { + // Five replicas is a valid value but the existing CPMS has three replicas + Expect(komega.Update(cpms, func() { + five := int32(5) + cpms.Spec.Replicas = &five + })()).Should(MatchError(ContainSubstring("ControlPlaneMachineSet.machine.openshift.io \"cluster\" is invalid: spec.replicas: Invalid value: \"integer\": replicas is immutable")), "Replicas should be immutable") + }) + + It("when modifying the machine labels and the selector still matches", func() { + Expect(komega.Update(cpms, func() { + cpms.Spec.Template.OpenShiftMachineV1Beta1Machine.ObjectMeta.Labels["new"] = dummyValue + })()).Should(Succeed(), "Machine label updates are allowed provided the selector still matches") + }) + + It("when modifying the machine labels so that the selector no longer matches", func() { + Expect(komega.Update(cpms, func() { + cpms.Spec.Template.OpenShiftMachineV1Beta1Machine.ObjectMeta.Labels = map[string]string{ + "different": "labels", + machinev1beta1.MachineClusterIDLabel: "cpms-cluster-test-id-different", + openshiftMachineRoleLabel: masterMachineRole, + openshiftMachineTypeLabel: masterMachineRole, + } + })()).Should(MatchError(ContainSubstring("selector does not match template labels")), "The selector must always match the machine labels") + }) + + It("when modifying the machine labels to remove the cluster ID label", func() { + Expect(komega.Update(cpms, func() { + delete(cpms.Spec.Template.OpenShiftMachineV1Beta1Machine.ObjectMeta.Labels, machinev1beta1.MachineClusterIDLabel) + })()).Should(MatchError(ContainSubstring("ControlPlaneMachineSet.machine.openshift.io \"cluster\" is invalid: spec.template.machines_v1beta1_machine_openshift_io.metadata.labels: Invalid value: \"object\": label 'machine.openshift.io/cluster-api-cluster' is required")), "The labels must always contain a cluster ID label") + }) + + It("when mutating the selector", func() { + Expect(komega.Update(cpms, func() { + cpms.Spec.Selector.MatchLabels["new"] = dummyValue + })()).Should(MatchError(ContainSubstring("ControlPlaneMachineSet.machine.openshift.io \"cluster\" is invalid: spec.selector: Invalid value: \"object\": selector is immutable")), "The selector should be immutable") + }) + }) }) }) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index a80f519fd..ed54baf78 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -228,7 +228,7 @@ func (f *framework) IncreaseProviderSpecInstanceSize(rawProviderSpec *runtime.Ra } // UpdateDefaultedValueFromCPMS updates a defaulted value from the ControlPlaneMachineSet -// for either AWS, Azure or GCP. +// for either AWS, Azure, GCP or OpenStack. func (f *framework) UpdateDefaultedValueFromCPMS(rawProviderSpec *runtime.RawExtension) (*runtime.RawExtension, error) { providerConfig, err := providerconfig.NewProviderConfigFromMachineSpec(machinev1beta1.MachineSpec{ ProviderSpec: machinev1beta1.ProviderSpec{ @@ -246,6 +246,8 @@ func (f *framework) UpdateDefaultedValueFromCPMS(rawProviderSpec *runtime.RawExt return updateCredentialsSecretNameAWS(providerConfig) case configv1.GCPPlatformType: return updateCredentialsSecretNameGCP(providerConfig) + case configv1.OpenStackPlatformType: + return updateCredentialsSecretNameOpenStack(providerConfig) default: return nil, fmt.Errorf("%w: %s", errUnsupportedPlatform, f.platform) } @@ -296,6 +298,23 @@ func updateCredentialsSecretNameGCP(providerConfig providerconfig.ProviderConfig }, nil } +// updateCredentialsSecretNameOpenStack updates the credentialSecret field from the ControlPlaneMachineSet. +func updateCredentialsSecretNameOpenStack(providerConfig providerconfig.ProviderConfig) (*runtime.RawExtension, error) { + cfg := providerConfig.OpenStack().Config() + // TODO(emilien): I'm not sure about this one + // other platforms are using CredentialsSecret, but OpenStack doesn't have it in its providerSpec + cfg.CloudsSecret = nil + + rawBytes, err := json.Marshal(cfg) + if err != nil { + return nil, fmt.Errorf("error marshalling openstack providerSpec: %w", err) + } + + return &runtime.RawExtension{ + Raw: rawBytes, + }, nil +} + // ConvertToControlPlaneMachineSetProviderSpec converts a control plane machine provider spec // to a raw, control plane machine set suitable provider spec. func (f *framework) ConvertToControlPlaneMachineSetProviderSpec(providerSpec machinev1beta1.ProviderSpec) (*runtime.RawExtension, error) { @@ -313,6 +332,8 @@ func (f *framework) ConvertToControlPlaneMachineSetProviderSpec(providerSpec mac return convertAzureProviderConfigToControlPlaneMachineSetProviderSpec(providerConfig) case configv1.GCPPlatformType: return convertGCPProviderConfigToControlPlaneMachineSetProviderSpec(providerConfig) + case configv1.OpenStackPlatformType: + return convertOpenStackProviderConfigToControlPlaneMachineSetProviderSpec(providerConfig) default: return nil, fmt.Errorf("%w: %s", errUnsupportedPlatform, f.platform) } @@ -367,6 +388,23 @@ func convertAzureProviderConfigToControlPlaneMachineSetProviderSpec(providerConf }, nil } +// convertOpenStackProviderConfigToControlPlaneMachineSetProviderSpec converts an OpenStack providerConfig into a +// raw control plane machine set provider spec. +func convertOpenStackProviderConfigToControlPlaneMachineSetProviderSpec(providerConfig providerconfig.ProviderConfig) (*runtime.RawExtension, error) { + openStackPs := providerConfig.OpenStack().Config() + openStackPs.AvailabilityZone = "" + openStackPs.RootVolume.Zone = "" + + rawBytes, err := json.Marshal(openStackPs) + if err != nil { + return nil, fmt.Errorf("error marshalling openstack providerSpec: %w", err) + } + + return &runtime.RawExtension{ + Raw: rawBytes, + }, nil +} + // loadClient returns a new controller-runtime client. func loadClient(sch *runtime.Scheme) (runtimeclient.Client, error) { cfg, err := config.GetConfig() @@ -429,6 +467,8 @@ func getPlatformSupportLevel(k8sClient runtimeclient.Client) (PlatformSupportLev return Manual, platformType, nil case configv1.GCPPlatformType: return Manual, platformType, nil + case configv1.OpenStackPlatformType: + return Manual, platformType, nil default: return Unsupported, platformType, nil }