Skip to content

Modify rc-13 workaround to use an environment variable #69

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Sep 6, 2024
Merged

Modify rc-13 workaround to use an environment variable #69

merged 2 commits into from
Sep 6, 2024

Conversation

markgoddard
Copy link

  • Revert "Workaround: Disable SSH pipelining"
  • Work around Ansible rc-13 race condition (mk2)

@markgoddard
Revert "Workaround: Disable SSH pipelining"
cf3baaf 
This reverts commit 61460d8.
@markgoddard
Work around Ansible rc-13 race condition (mk2)
7cc0ade 
There is a race condition in Ansible that can result in this failure:
  msg: |-
  MODULE FAILURE
  See stdout/stderr for the exact error
rc: -13
See ansible/ansible#78344 and
ansible/ansible#81777.

In stackhpc/stackhpc-kayobe-config#1108 we applied
a workaround to increase the ControlPersist timeout to 1 hour, but this
does not always work.

Here we use a different workaround of disabling SSH pipelining. This has
performance implications for Ansible, but is a reasonable trade-off for
reliability.

We set the config option as an environment variable rather than in
ansible.cfg in Kayobe configuration, to avoid a merge conflict on upgrade.
@markgoddard markgoddard requested a review from a team as a code owner September 2, 2024 15:36
@markgoddard markgoddard self-assigned this Sep 2, 2024
dougszumski
dougszumski reviewed Sep 2, 2024
View reviewed changes
Alex-Welsh
Alex-Welsh approved these changes Sep 6, 2024
View reviewed changes
Copy link
Member

@Alex-Welsh Alex-Welsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though we should probably do some performance testing to see if this is worth setting in our default ansible.cfg

@markgoddard markgoddard merged commit ee002f2 into main Sep 6, 2024
@markgoddard markgoddard deleted the workaround-rc-13 branch September 6, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dougszumski dougszumski dougszumski left review comments

@Alex-Welsh Alex-Welsh Alex-Welsh approved these changes

Assignees

@markgoddard markgoddard

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@markgoddard @dougszumski @Alex-Welsh