-
Notifications
You must be signed in to change notification settings - Fork 58
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
fix release manager rollout image listing when using Helm #1182
fix release manager rollout image listing when using Helm #1182
Conversation
8d4066f
to
52062e2
Compare
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.
Nice @gerardcl!
Overall this code looks good to me (if it works when running test). I have a few minor suggestions which you could adopt at your discretion.
Also wanted to make you aware of PR #1143 which touches the same files although I am not sure what the latest status is.
Perhaps the changes you are suggesting would also make sense in DeployOdsComponent which currently is flaky for Helm deployments.
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.
@gerardcl For your consideration. If it's good for you the way it is, it's also good for me.
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.
Good job, @gerardcl !
Relates to opendevstack/ods-quickstarters#1013 , as this bug must be fixed.
The issue happens rarely but impacts the use of Helm strategy within the Release Manager usage; it is a race condition happening when having rollout defined as deployment strategy, and with the provided fix we make sure we always get the latest expected image.
FYI, the fix has also been tested internally, thanks to @BraisVQ