diff --git a/controllers/dataplane/openstackdataplanedeployment_controller.go b/controllers/dataplane/openstackdataplanedeployment_controller.go index 75a940c9a..f3a1f2e15 100644 --- a/controllers/dataplane/openstackdataplanedeployment_controller.go +++ b/controllers/dataplane/openstackdataplanedeployment_controller.go @@ -251,17 +251,16 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context, globalSSHKeySecrets[nodeSet.Name] = nodeSet.Spec.NodeTemplate.AnsibleSSHPrivateKeySecret } - if instance.Spec.ServicesOverride == nil { - if nodesetServiceMap, err = deployment.DedupServices(ctx, helper, nodeSets.Items); err != nil { - util.LogErrorForObject(helper, err, "OpenStackDeployment error for deployment", instance) - instance.Status.Conditions.MarkFalse( - condition.DeploymentReadyCondition, - condition.ErrorReason, - condition.SeverityError, - dataplanev1.ServiceErrorMessage, - err.Error()) - return ctrl.Result{}, err - } + if nodesetServiceMap, err = deployment.DedupeServices(ctx, helper, nodeSets.Items, + instance.Spec.ServicesOverride); err != nil { + util.LogErrorForObject(helper, err, "OpenStackDeployment error for deployment", instance) + instance.Status.Conditions.MarkFalse( + condition.DeploymentReadyCondition, + condition.ErrorReason, + condition.SeverityError, + dataplanev1.ServiceErrorMessage, + err.Error()) + return ctrl.Result{}, err } version, err := dataplaneutil.GetVersion(ctx, helper, instance.Namespace) @@ -310,7 +309,7 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context, // deploy with the OpenStackDataPlaneNodeSet's Services. var deployResult *ctrl.Result if len(instance.Spec.ServicesOverride) != 0 { - deployResult, err = deployer.Deploy(instance.Spec.ServicesOverride) + deployResult, err = deployer.Deploy(nodesetServiceMap[deployment.AllNodeSets]) } else { deployResult, err = deployer.Deploy(nodesetServiceMap[nodeSet.Name]) } diff --git a/pkg/dataplane/const.go b/pkg/dataplane/const.go index 79f24c64a..8589ff56d 100644 --- a/pkg/dataplane/const.go +++ b/pkg/dataplane/const.go @@ -71,4 +71,8 @@ const ( //HostnameLabel label for marking secrets to be watched for changes HostnameLabel = "hostname" + + //AllNodeSets Key used in nodeset service map for all nodesets when + //using serviceOverride. + AllNodeSets = "AllNodeSets" ) diff --git a/pkg/dataplane/service.go b/pkg/dataplane/service.go index 1b639d959..e537ef67b 100644 --- a/pkg/dataplane/service.go +++ b/pkg/dataplane/service.go @@ -25,6 +25,7 @@ import ( "golang.org/x/exp/slices" yaml "gopkg.in/yaml.v3" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -161,31 +162,60 @@ func EnsureServices(ctx context.Context, helper *helper.Helper, instance *datapl return nil } -// Dedup services for deploy +// Dedupe services to deploy // Multiple Services of same ServiceType/ServiceName in a nodeset // Global Services in multiple NodeSets for a deployment -func DedupServices(ctx context.Context, helper *helper.Helper, nodesets []dataplanev1.OpenStackDataPlaneNodeSet) (map[string][]string, error) { - var globalServices []string +func DedupeServices(ctx context.Context, helper *helper.Helper, + nodesets []dataplanev1.OpenStackDataPlaneNodeSet, + serviceOverride []string) (map[string][]string, error) { var nodeSetServiceMap = make(map[string][]string, 0) - for _, nodeset := range nodesets { - var services []string - var nodeSetServiceTypes []string - for _, svc := range nodeset.Spec.Services { - service, err := GetService(ctx, helper, svc) + var globalServices []string + if len(serviceOverride) != 0 { + services, err := dedupe(ctx, helper, serviceOverride, globalServices) + if err != nil { + return nil, err + } + nodeSetServiceMap[AllNodeSets] = services + } else { + for _, nodeset := range nodesets { + services, err := dedupe(ctx, helper, nodeset.Spec.Services, globalServices) if err != nil { - helper.GetLogger().Error(err, fmt.Sprintf("Configured service %s does not exist", svc)) return nil, err } - if !slices.Contains(nodeSetServiceTypes, service.Spec.EDPMServiceType) && !slices.Contains(services, svc) { - nodeSetServiceTypes = append(nodeSetServiceTypes, service.Spec.EDPMServiceType) - services = append(services, svc) + nodeSetServiceMap[nodeset.Name] = services + } + } + return nodeSetServiceMap, nil +} + +func dedupe(ctx context.Context, helper *helper.Helper, + services []string, globalServices []string) ([]string, error) { + var dedupedServices []string + var nodeSetServiceTypes []string + for _, svc := range services { + service, err := GetService(ctx, helper, svc) + if err != nil { + if k8s_errors.IsNotFound(err) && !slices.Contains(dedupedServices, svc) { + helper.GetLogger().Error(err, fmt.Sprintf("Configured service %s does not exist", svc)) + // Add the service to the service list as it would fail later when deploying and + // Update the statuses accordingly. + dedupedServices = append(dedupedServices, svc) + continue } - if service.Spec.DeployOnAllNodeSets && !slices.Contains(globalServices, svc) { - globalServices = append(globalServices, svc) - services = append(services, svc) + return dedupedServices, err + + } + if !slices.Contains(nodeSetServiceTypes, service.Spec.EDPMServiceType) && !slices.Contains(dedupedServices, svc) { + if service.Spec.DeployOnAllNodeSets { + if !slices.Contains(globalServices, svc) { + globalServices = append(globalServices, svc) + } else { + continue + } } + nodeSetServiceTypes = append(nodeSetServiceTypes, service.Spec.EDPMServiceType) + dedupedServices = append(dedupedServices, svc) } - nodeSetServiceMap[nodeset.Name] = services } - return nodeSetServiceMap, nil + return dedupedServices, nil } diff --git a/tests/functional/dataplane/base_test.go b/tests/functional/dataplane/base_test.go index 832e31a56..02ba09633 100644 --- a/tests/functional/dataplane/base_test.go +++ b/tests/functional/dataplane/base_test.go @@ -213,6 +213,20 @@ func DefaultDataPlaneDeploymentSpec() map[string]interface{} { } } +// Build OpenStackDataPlnaeDeploymentSpec with duplicate services +func DuplicateServiceDeploymentSpec() map[string]interface{} { + + return map[string]interface{}{ + "nodeSets": []string{ + "edpm-compute-nodeset", + }, + "servicesOverride": []string{ + "foo-service", + "duplicate-service", + }, + } +} + func DefaultNetConfigSpec() map[string]interface{} { return map[string]interface{}{ "networks": []map[string]interface{}{{ diff --git a/tests/functional/dataplane/openstackdataplanedeployment_controller_test.go b/tests/functional/dataplane/openstackdataplanedeployment_controller_test.go index 5782bf597..bffd85d6c 100644 --- a/tests/functional/dataplane/openstackdataplanedeployment_controller_test.go +++ b/tests/functional/dataplane/openstackdataplanedeployment_controller_test.go @@ -706,7 +706,105 @@ var _ = Describe("Dataplane Deployment Test", func() { ) }) }) + When("A dataplaneDeployment is created with duplicate service in deployment service override", func() { + BeforeEach(func() { + CreateDataplaneServicesWithSameServiceType(dataplaneServiceName) + CreateSSHSecret(dataplaneSSHSecretName) + DeferCleanup(th.DeleteInstance, th.CreateSecret(neutronOvnMetadataSecretName, map[string][]byte{ + "fake_keys": []byte("blih"), + })) + DeferCleanup(th.DeleteInstance, th.CreateSecret(novaNeutronMetadataSecretName, map[string][]byte{ + "fake_keys": []byte("blih"), + })) + DeferCleanup(th.DeleteInstance, th.CreateSecret(novaCellComputeConfigSecretName, map[string][]byte{ + "fake_keys": []byte("blih"), + })) + DeferCleanup(th.DeleteInstance, th.CreateSecret(novaMigrationSSHKey, map[string][]byte{ + "ssh-privatekey": []byte("fake-ssh-private-key"), + "ssh-publickey": []byte("fake-ssh-public-key"), + })) + DeferCleanup(th.DeleteInstance, th.CreateSecret(ceilometerConfigSecretName, map[string][]byte{ + "fake_keys": []byte("blih"), + })) + // DefaultDataPlanenodeSetSpec comes with two mock services, one marked for deployment on all nodesets + // But we will not create them to test this scenario + DeferCleanup(th.DeleteInstance, CreateNetConfig(dataplaneNetConfigName, DefaultNetConfigSpec())) + DeferCleanup(th.DeleteInstance, CreateDNSMasq(dnsMasqName, DefaultDNSMasqSpec())) + SimulateDNSMasqComplete(dnsMasqName) + DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(dataplaneNodeSetName, DefaultDataPlaneNodeSetSpec(dataplaneNodeSetName.Name))) + DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(dataplaneDeploymentName, DuplicateServiceDeploymentSpec())) + SimulateIPSetComplete(dataplaneNodeName) + SimulateDNSDataComplete(dataplaneNodeSetName) + }) + It("Should have Spec fields initialized", func() { + dataplaneDeploymentInstance := GetDataplaneDeployment(dataplaneDeploymentName) + expectedSpec := dataplanev1.OpenStackDataPlaneDeploymentSpec{ + NodeSets: []string{"edpm-compute-nodeset"}, + AnsibleTags: "", + AnsibleLimit: "", + AnsibleSkipTags: "", + BackoffLimit: &DefaultBackoffLimit, + DeploymentRequeueTime: 15, + ServicesOverride: []string{dataplaneServiceName.Name, "duplicate-service"}, + } + Expect(dataplaneDeploymentInstance.Spec).Should(Equal(expectedSpec)) + }) + It("should have conditions set to true", func() { + baremetal := baremetalv1.OpenStackBaremetalSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: dataplaneNodeSetName.Name, + Namespace: dataplaneNodeName.Namespace, + }, + } + // Set baremetal provisioning conditions to True + // This must succeed, as the "alpha-nodeset" exists + Eventually(func(g Gomega) { + // OpenStackBaremetalSet has the same name as OpenStackDataPlaneNodeSet + g.Expect(th.K8sClient.Get(th.Ctx, dataplaneNodeSetName, &baremetal)).To(Succeed()) + baremetal.Status.Conditions.MarkTrue( + condition.ReadyCondition, + condition.ReadyMessage) + g.Expect(th.K8sClient.Status().Update(th.Ctx, &baremetal)).To(Succeed()) + + }, th.Timeout, th.Interval).Should(Succeed()) + + // Create config map for OVN service + ovnConfigMapName := types.NamespacedName{ + Namespace: namespace, + Name: "ovncontroller-config", + } + mapData := map[string]interface{}{ + "ovsdb-config": "test-ovn-config", + } + th.CreateConfigMap(ovnConfigMapName, mapData) + service := GetService(dataplaneServiceName) + aeeName, _ := dataplaneutil.GetAnsibleExecutionNameAndLabels(service, + dataplaneDeploymentName.Name, dataplaneNodeSetName.Name) + ansibleeeName := types.NamespacedName{ + Name: aeeName, + Namespace: dataplaneDeploymentName.Namespace, + } + Eventually(func(g Gomega) { + ansibleEE := GetAnsibleee(ansibleeeName) + ansibleEE.Status.JobStatus = ansibleeev1.JobStatusSucceeded + g.Expect(th.K8sClient.Status().Update(th.Ctx, ansibleEE)).To(Succeed()) + }, th.Timeout, th.Interval).Should(Succeed()) + + th.ExpectCondition( + dataplaneDeploymentName, + ConditionGetterFunc(DataplaneDeploymentConditionGetter), + condition.DeploymentReadyCondition, + corev1.ConditionTrue, + ) + th.ExpectCondition( + dataplaneDeploymentName, + ConditionGetterFunc(DataplaneDeploymentConditionGetter), + condition.InputReadyCondition, + corev1.ConditionTrue, + ) + }) + }) When("A dataplaneDeployment is created with non-existent service in nodeset", func() { BeforeEach(func() { CreateSSHSecret(dataplaneSSHSecretName)