Skip to content
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

reconcile cluster roles instead of overwriting #15654

Merged
merged 3 commits into from Aug 8, 2017

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Aug 7, 2017

fixes #15648

Moving to post-start hooks introduced a policy creation race. This pull fixes the race by unconditionally reconciling like we will in 3.7 when we switch to RBAC. I also made the reconcile cluster roles respect the annotation we use to protect the resources from reconciliation.

@openshift/security fyi

@openshift-merge-robot openshift-merge-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 7, 2017
@enj
Copy link
Contributor

enj commented Aug 7, 2017

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 7, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, enj

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@deads2k
Copy link
Contributor Author

deads2k commented Aug 7, 2017

failure is real

@deads2k deads2k added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Aug 7, 2017
@deads2k
Copy link
Contributor Author

deads2k commented Aug 7, 2017

@enj added a new commit if you want a second look. namespaced roles in our openshift namespace. Falls out like the other namespaced ones in the RBAC refactor.

@deads2k deads2k added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Aug 7, 2017
@enj
Copy link
Contributor

enj commented Aug 7, 2017

@enj added a new commit if you want a second look. namespaced roles in our openshift namespace. Falls out like the other namespaced ones in the RBAC refactor

LGTM

@deads2k
Copy link
Contributor Author

deads2k commented Aug 7, 2017

/test end_to_end

@deads2k
Copy link
Contributor Author

deads2k commented Aug 7, 2017

/retest

@deads2k
Copy link
Contributor Author

deads2k commented Aug 7, 2017

yum problem
/test end_to_end

@deads2k
Copy link
Contributor Author

deads2k commented Aug 7, 2017

#8571
/test end_to_end

@deads2k
Copy link
Contributor Author

deads2k commented Aug 8, 2017

/retest

@deads2k
Copy link
Contributor Author

deads2k commented Aug 8, 2017

/test extended_conformance_install_update

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@0xmichalis
Copy link
Contributor

/test extended_conformance_install_update

@0xmichalis
Copy link
Contributor

/retest

1 similar comment
@0xmichalis
Copy link
Contributor

/retest

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. queue/fix retest-not-required size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
6 participants