Unify SSH key mount path format across all dataplane services #1752
+261
−243
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, the SSH key mount path differed between global services (DeployOnAllNodeSets=true) and non-global services: - Global services: /runner/env/ssh_key/ssh_key_<nodesetname> - Non-global services: /runner/env/ssh_key The non-global services path happened to work because ansible-runner has a built-in mechanism that looks for an SSH key at /runner/env/ssh_key and automatically loads it into ssh-agent. However, this relied on ansible-runner's implicit behavior rather than the explicit ansible_ssh_private_key_file variable set in the inventory. The inventory always sets ansible_ssh_private_key_file to /runner/env/ssh_key/ssh_key_<nodesetname> regardless of service type (see inventory.go line 178). This inconsistency meant non-global services were mounting the SSH key at a different path than what Ansible expected from the inventory variable, relying on ansible-runner's fallback behavior. However, there were errors in ansible logs as there were no files in /runner/env/ssh_key/ssh_key_<nodesetname> which was confusing to users. This change unifies the SSH key mount path to always use the format: /runner/env/ssh_key/ssh_key_<nodesetname> This ensures: 1. The mount path matches the ansible_ssh_private_key_file variable set in the inventory for all service types 2. Explicit and consistent SSH key configuration rather than relying on ansible-runner's implicit ssh-agent loading 3. Simplified code by removing the conditional branching 4. Consistent behavior between global and non-global services For global services, multiple SSH keys are mounted (one per nodeset) in the ssh_key folder. For non-global services, only the matching nodeset's key is mounted, but at the same path format. Assisted-by: Claude-4.5-opus Signed-off-by: rabi <ramishra@redhat.com>
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rabi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…itions
1. 'Failed deployment followed by completed deployment' test:
- The test was creating both deployments simultaneously, causing a race
where the test would try to update the status of a job that hadn't
been created yet
- Fixed by ensuring sequential execution: wait for the first deployment's
job to fail before creating the second deployment
2. 'should not reconcile after failure' test:
- The test was manually setting deployment status to 'backoff limit
exceeded' without waiting for the controller to finish its initial
reconciliation
- Fixed by waiting for ObservedGeneration to match Generation before
manually setting the failure status, ensuring the controller has
completed processing
Assisted-by: Claude-4.5-opus
Signed-off-by: rabi <ramishra@redhat.com>
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/d0734dccfadd4586a253cb68910e1637 ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 53m 54s |
Contributor
Author
|
recheck |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the SSH key mount path differed between global services (DeployOnAllNodeSets=true) and non-global services:
The non-global services path happened to work because ansible-runner has a built-in mechanism that looks for an SSH key at /runner/env/ssh_key and automatically loads it into ssh-agent. However, this relied on ansible-runner's implicit behavior rather than the explicit ansible_ssh_private_key_file variable set in the inventory.
The inventory always sets ansible_ssh_private_key_file to /runner/env/ssh_key/ssh_key_ regardless of service type (see inventory.go line 178). This inconsistency meant non-global services were mounting the SSH key at a different path than what Ansible expected from the inventory variable, relying on ansible-runner's fallback behavior. However, there were errors in ansible logs as there were no files in /runner/env/ssh_key/ssh_key_ which was confusing to users.
This change unifies the SSH key mount path to always use the format:
/runner/env/ssh_key/ssh_key_< nodesetname >
This ensures:
For global services, multiple SSH keys are mounted (one per nodeset) in the ssh_key folder. For non-global services, only the matching nodeset's key is mounted, but at the same path format.
Assisted-by: Claude-4.5-opus