Update controller-runtime to (unreleased) master #1451
Conversation
openshift-ci
bot
added
the
approved
2uasimojo
force-pushed
the
controller-runtime-latest
branch
from
8f2a1a3
to
d5b29d7
Compare
We were using a [controller-runtime fork](github.com/openshift-hive/controller-runtime) so we could [disable a metric](openshift-hive/controller-runtime@e847e4b) that was causing a cardinality explosion and memory problems. Upstream controller-runtime has now [disabled that metric by default](kubernetes-sigs/controller-runtime#1587); so this commit switches back to using upstream. NOTE: We're using an unreleased upstream version.
2uasimojo
force-pushed
the
controller-runtime-latest
branch
from
d5b29d7
to
54262c1
Compare
|
Looks like some legit failures due to new code, but this will be great once passing. |
|
@dgoodwin you okay with using unreleased? |
|
I can go either way, do we know how long until release expected? |
|
According to Alvaro, it will "take some time". |
|
Probably best to keep what we have now and is stable until it's released, and then make the move to 0.9.x which we also have a card for. |
|
Okay. (It'll be 0.10.x when it happens, since this is a breaking change.) |
|
Well. |
2uasimojo
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
I'm working on updating the tests. The major change in controller-runtime that our reconcile tests now have to accommodate is that the fake client now does Delete more closely to reality, in terms of handling finalizers properly. Many (many) tests were written against the old (incorrect) behavior: deleting the object regardless of finalizers. |
Yeah that's fine. |
Update tests for controller-runtime changes from 0.8.2 to 0.9.2(+). Most of these changes react to kubernetes-sigs/controller-runtime#1399, which made the fake client's Delete behave more like the real thing: - If the object has finalizers, set `DeletionTimestamp` but don't actually delete. - If there are no finalizers, actually delete. - Updates that remove the last finalizer from an object with a `DeletionTimestamp` actually delete the object. Then there's kubernetes-sigs/controller-runtime#1390 which deduplicates objects in the reconcile queue for Update actions.
2uasimojo
force-pushed
the
controller-runtime-latest
branch
from
7980531
to
5246a66
Compare
Codecov Report
@@ Coverage Diff @@
## master #1451 +/- ##
==========================================
+ Coverage 40.50% 41.10% +0.60%
==========================================
Files 334 334
Lines 26774 30059 +3285
==========================================
+ Hits 10844 12355 +1511
- Misses 14893 16658 +1765
- Partials 1037 1046 +9
|
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 2uasimojo, dgoodwin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2uasimojo
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/test e2e |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
We removed this test via f0292ed / openshift#19059 while awaiting a controller-runtime bump. Said bump was done via openshift/hive#1451, so we're ready to restore it. HIVE-1560
We were using a controller-runtime fork so we could disable a metric that was causing a cardinality explosion and memory problems.
Upstream controller-runtime has now disabled that metric by default; so this commit switches back to using upstream.
NOTE: We're using an unreleased upstream version.
part of HIVE-1560