-
Notifications
You must be signed in to change notification settings - Fork 115
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
Predeploy RoleBinding before unmanaged pods #354
Conversation
bc9cba8
to
4c03936
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.
Thanks for this contribution! I've kicked off a CI run, but its going to be delayed due to the github webhook backlog
end | ||
|
||
def deploy_succeeded? | ||
exists? |
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.
As far as I can tell there is no status field for this resource so the success condition seems reasonable
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 just have a small request regarding the tests. Thanks for the contribution!
63ab19f
to
3de9a14
Compare
Could you please rebase this? Code LGTM but tests are failing because you're using the new constant |
RoleBindings should be deployed before unmanaged pods. This matters when RoleBindings define Pod Security Policies. This might prevent unmanaged pods from starting unless RoleBinding has been set up. Regular pods retry so the race condition would not break anything.
3de9a14
to
79d1aef
Compare
Rebased. I used Docker for Mac for testing from #356, which almost worked as it didn't see to like context name "docker-for-desktop" rather than "docker-for-mac". Didn't realise that it also changed the constant name. |
RoleBindings should be deployed before unmanaged pods. This matters when RoleBindings define Pod Security Policies. This might prevent unmanaged pods from starting unless RoleBinding has been set up and the deployment fails.
For normal deployments only scenario it already works. Even if there is a race condition, it works as the pods retry.
Should there be a separate resource type? Other similar resource types seemed to have also the corresponding class defined.