-
Notifications
You must be signed in to change notification settings - Fork 169
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.
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
adding retry logic on yum commands to avoid resource locking #3511
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, minor formatting change request.
yum -y install podman-docker | ||
for attempt in {1..5}; do | ||
yum update -y --disablerepo='*' --enablerepo='rhui-microsoft-azure*' && break | ||
if [[ ${attempt} -lt 5 ]]; then sleep 10; else exit 1; fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we break this up into multiple lines?
Single line is harder to read and not really needed here.
echo "running yum update" | ||
for attempt in {1..5}; do | ||
yum -y -x WALinuxAgent -x WALinuxAgent-udev update --allowerasing && break | ||
if [[ ${attempt} -lt 5 ]]; then sleep 10; else exit 1; fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above for the remainder of if statements added
yum -y -x WALinuxAgent -x WALinuxAgent-udev update | ||
yum -y install podman-docker | ||
for attempt in {1..5}; do | ||
yum update -y --disablerepo='*' --enablerepo='rhui-microsoft-azure*' && break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also a fan of breaking up script arguments into multiple lines when there are multiple.
Example:
yum update \
-y \
--disablerepo='*' \
--enablerepo='rhui-microsoft-azure*' && break
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, legibility changes can be made in a follow up improvement PR.
/azp run ci |
Commenter does not have sufficient privileges for PR 3511 in repo Azure/ARO-RP |
/azp run ci |
Azure Pipelines successfully started running 1 pipeline(s). |
Which issue this PR addresses:
This PR attempts to address the latest failure in the rpVMSS.sh script by adding retry logic to the yum update commands found therein. Failure can be observed in the ARO production pipeline e2e step.
What this PR does / why we need it:
Adds retry logic to yum update commands in rpVMSS.sh in order to allow for subsequent attempts if a resource lock is present,.
Test plan for issue: