Merge pull request #1752 from rabi/ssh_key_location

Unify SSH key mount path format across all dataplane services

Author: openshift-merge-bot[bot]

internal/dataplane/util/ansible_execution.go

@@ -316,19 +316,14 @@ func SetAeeSSHMounts(
 
 	for _, sshKeyNodeName := range sshKeys {
 		sshKeySecret := sshKeySecrets[sshKeyNodeName]
-		if service.Spec.DeployOnAllNodeSets {
-			sshKeyName = fmt.Sprintf("ssh-key-%s", sshKeyNodeName)
-			sshKeyMountSubPath = fmt.Sprintf("ssh_key_%s", sshKeyNodeName)
-			sshKeyMountPath = fmt.Sprintf("/runner/env/ssh_key/%s", sshKeyMountSubPath)
-		} else {
-			if sshKeyNodeName != nodeSet.GetName() {
-				continue
-			}
-			sshKeyName = "ssh-key"
-			sshKeyMountSubPath = "ssh_key"
-			sshKeyMountPath = "/runner/env/ssh_key"
+		if !service.Spec.DeployOnAllNodeSets && sshKeyNodeName != nodeSet.GetName() {
+			continue
 		}
 
+		sshKeyName = fmt.Sprintf("ssh-key-%s", sshKeyNodeName)
+		sshKeyMountSubPath = fmt.Sprintf("ssh_key_%s", sshKeyNodeName)
+		sshKeyMountPath = fmt.Sprintf("/runner/env/ssh_key/%s", sshKeyMountSubPath)
+
 		CreateVolume(ansibleEEMounts, sshKeyName, sshKeyMountSubPath, sshKeySecret, "ssh-privatekey")
 		CreateVolumeMount(ansibleEEMounts, sshKeyName, sshKeyMountPath, sshKeyMountSubPath)
 	}

test/functional/dataplane/openstackdataplanedeployment_controller_test.go

@@ -1828,15 +1828,19 @@ var _ = Describe("Dataplane Deployment Test", func() {
 		})
 
 		It("should not reconcile after failure", func() {
-			// Directly set the deployment status to backoff limit exceeded
-			// This simulates a deployment that has failed after retries
+			// Wait for the controller to complete initial reconciliation and reach a stable state
+			// by waiting for the deployment to have conditions set and be in a known state
 			Eventually(func(g Gomega) {
 				deployment := GetDataplaneDeployment(dataplaneDeploymentName)
+				g.Expect(deployment.Status.Conditions).ToNot(BeEmpty())
+				// Wait for ObservedGeneration to be set, indicating controller has processed the resource
+				g.Expect(deployment.Status.ObservedGeneration).To(Equal(deployment.Generation))
+			}, th.Timeout, th.Interval).Should(Succeed())
 
-				// Initialize conditions if not present
-				if deployment.Status.Conditions == nil {
-					deployment.Status.Conditions = condition.Conditions{}
-				}
+			// Now manually set the deployment status to backoff limit exceeded
+			// This simulates a deployment that has failed after retries
+			Eventually(func(g Gomega) {
+				deployment := GetDataplaneDeployment(dataplaneDeploymentName)
 
 				// Initialize NodeSetConditions if not present
 				if deployment.Status.NodeSetConditions == nil {
@@ -1852,11 +1856,19 @@ var _ = Describe("Dataplane Deployment Test", func() {
 					"Simulated backoff limit exceeded for testing")
 
 				deployment.Status.Deployed = false
-				deployment.Status.ObservedGeneration = deployment.Generation
 
 				g.Expect(th.K8sClient.Status().Update(th.Ctx, deployment)).To(Succeed())
 			}, th.Timeout, th.Interval).Should(Succeed())
 
+			// Wait for the status to stabilize - ensure the controller recognizes the failure state
+			Eventually(func(g Gomega) {
+				deployment := GetDataplaneDeployment(dataplaneDeploymentName)
+				deploymentCondition := deployment.Status.Conditions.Get(condition.DeploymentReadyCondition)
+				g.Expect(deploymentCondition).ToNot(BeNil())
+				g.Expect(string(deploymentCondition.Severity)).To(Equal(string(condition.SeverityError)))
+				g.Expect(string(deploymentCondition.Reason)).To(Equal(string(condition.JobReasonBackoffLimitExceeded)))
+			}, th.Timeout, th.Interval).Should(Succeed())
+
 			// Verify the deployment has the correct failure condition
 			deployment := GetDataplaneDeployment(dataplaneDeploymentName)
 			deploymentCondition := deployment.Status.Conditions.Get(condition.DeploymentReadyCondition)

test/functional/dataplane/openstackdataplanenodeset_controller_test.go

@@ -1387,11 +1387,11 @@ var _ = Describe("Dataplane NodeSet Test", func() {
 				ansibleEE := GetAnsibleee(ansibleeeName)
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes).To(HaveLen(3))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[0].Name).To(Equal("bootstrap-combined-ca-bundle"))
-				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[1].Name).To(Equal("ssh-key"))
+				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[1].Name).To(Equal("ssh-key-edpm-compute-nodeset"))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[2].Name).To(Equal("inventory"))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[0].VolumeSource.Secret.SecretName).To(Equal("combined-ca-bundle"))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[1].VolumeSource.Secret.SecretName).To(Equal("dataplane-ansible-ssh-private-key-secret"))
-				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[1].VolumeSource.Secret.Items[0].Path).To(Equal("ssh_key"))
+				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[1].VolumeSource.Secret.Items[0].Path).To(Equal("ssh_key_edpm-compute-nodeset"))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[1].VolumeSource.Secret.Items[0].Key).To(Equal("ssh-privatekey"))
 
 			}, th.Timeout, th.Interval).Should(Succeed())
@@ -1455,12 +1455,12 @@ var _ = Describe("Dataplane NodeSet Test", func() {
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes).To(HaveLen(4))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[0].Name).To(Equal("edpm-ansible"))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[1].Name).To(Equal("bootstrap-combined-ca-bundle"))
-				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[2].Name).To(Equal("ssh-key"))
+				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[2].Name).To(Equal("ssh-key-edpm-compute-nodeset"))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[3].Name).To(Equal("inventory"))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[0].VolumeSource.PersistentVolumeClaim.ClaimName).To(Equal("edpm-ansible"))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[1].VolumeSource.Secret.SecretName).To(Equal("combined-ca-bundle"))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[2].VolumeSource.Secret.SecretName).To(Equal("dataplane-ansible-ssh-private-key-secret"))
-				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[2].VolumeSource.Secret.Items[0].Path).To(Equal("ssh_key"))
+				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[2].VolumeSource.Secret.Items[0].Path).To(Equal("ssh_key_edpm-compute-nodeset"))
 				g.Expect(ansibleEE.Spec.Template.Spec.Volumes[2].VolumeSource.Secret.Items[0].Key).To(Equal("ssh-privatekey"))
 
 			}, th.Timeout, th.Interval).Should(Succeed())
@@ -1734,16 +1734,11 @@ var _ = Describe("Dataplane NodeSet Test", func() {
 				DeferCleanup(th.DeleteInstance, CreateDNSMasq(dnsMasqName, DefaultDNSMasqSpec()))
 				DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(testNodeSetName, nodeSetSpec))
 
-				// Create first deployment (will fail)
+				// Create only the first deployment here (will fail)
 				firstDeploymentSpec := DefaultDataPlaneDeploymentSpec()
 				firstDeploymentSpec["nodeSets"] = []string{testNodeSetName.Name}
 				DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(dataplaneDeploymentName, firstDeploymentSpec))
 
-				// Create second deployment (will complete successfully)
-				secondDeploymentSpec := DefaultDataPlaneDeploymentSpec()
-				secondDeploymentSpec["nodeSets"] = []string{testNodeSetName.Name}
-				DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(secondDeploymentName, secondDeploymentSpec))
-
 				CreateSSHSecret(dataplaneSSHSecretName)
 				CreateCABundleSecret(caBundleSecretName)
 				SimulateDNSMasqComplete(dnsMasqName)
@@ -1752,7 +1747,7 @@ var _ = Describe("Dataplane NodeSet Test", func() {
 			})
 
 			It("Should show all deployments in status and process completed deployment", func() {
-				// Fail the first deployment
+				// Wait for first AnsibleEE job to be created, then fail it
 				Eventually(func(g Gomega) {
 					ansibleeeName := types.NamespacedName{
 						Name:      "bootstrap-" + dataplaneDeploymentName.Name + "-" + testNodeSetName.Name,
@@ -1763,7 +1758,21 @@ var _ = Describe("Dataplane NodeSet Test", func() {
 					g.Expect(th.K8sClient.Status().Update(th.Ctx, ansibleEE)).To(Succeed())
 				}, th.Timeout, th.Interval).Should(Succeed())
 
-				// Complete the second deployment
+				// Wait for the first deployment to be marked as failed
+				Eventually(func(g Gomega) {
+					deployment := GetDataplaneDeployment(dataplaneDeploymentName)
+					g.Expect(deployment.Status.Conditions).ToNot(BeEmpty())
+					readyCondition := deployment.Status.Conditions.Get(condition.ReadyCondition)
+					g.Expect(readyCondition).ToNot(BeNil())
+					g.Expect(readyCondition.Status).To(Equal(corev1.ConditionFalse))
+				}, th.Timeout, th.Interval).Should(Succeed())
+
+				// Now create the second deployment (will complete successfully)
+				secondDeploymentSpec := DefaultDataPlaneDeploymentSpec()
+				secondDeploymentSpec["nodeSets"] = []string{testNodeSetName.Name}
+				DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(secondDeploymentName, secondDeploymentSpec))
+
+				// Wait for second AnsibleEE job to be created, then complete it
 				Eventually(func(g Gomega) {
 					ansibleeeName := types.NamespacedName{
 						Name:      "bootstrap-" + secondDeploymentName.Name + "-" + testNodeSetName.Name,