-
Notifications
You must be signed in to change notification settings - Fork 35
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
Upgrade to controller runtime 0.7.0 #18
Upgrade to controller runtime 0.7.0 #18
Conversation
714ec89
to
35def43
Compare
/hold Per community call discussion, this may need to wait a little bit depending on when controller-runtime upgrade work is done in Gatekeeper to minimize conflicts. |
@ritazh I understand, I can use my version of the code in the mean time so I am not in a hurry to get this merged :) I'll keep it up to date with master every once in a while |
We're trying to upgrade HNC to controller-runtime v0.7.0 too, but we're blocked on this PR going in. @ritazh , would you mind if we merged this change? As long as Gatekeeper is importing cert-controller at a pinned verion, it shouldn't affect your builds until you explicitly upgrade to the latest version, right? |
Signed-off-by: Stijn De Haes <stijndehaes@gmail.com>
35def43
to
e0beabd
Compare
I've imported the latest version of @stijndehaes 's change into HNC as a part of also upgrading HNC to controller-runtime v0.7.0, and everything appears to be working (our basic tests have passed; the rest of the suite is still running). I'll update this if there are any problems but at this point I'm fairly confident that the change is good. Thanks! |
We can unhold this now. open-policy-agent/gatekeeper#1073 is out for upgrading CR to v0.7.0 in Gatekeeper |
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.
Signed-off-by: Stijn De Haes <stijndehaes@gmail.com>
Signed-off-by: Stijn De Haes <stijndehaes@gmail.com>
Signed-off-by: Stijn De Haes <stijndehaes@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #18 +/- ##
==========================================
+ Coverage 51.44% 51.87% +0.42%
==========================================
Files 1 1
Lines 381 374 -7
==========================================
- Hits 196 194 -2
+ Misses 129 124 -5
Partials 56 56
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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
@shomron LGTY? |
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 have upgraded this to controller runtime 0.7.0, I already provided the PR for feedback. Tests run succesful, but I still want to test the code in a pod on a cluster. Will update my comment when I have done this.
Ok I have it working on a cluster, also noticed it might be interesting to migrate to code to use v1 of
admissionregistration.k8s.io
. If you guys are interested I'll do this in a next PR