Refactor BuildConfig controller to use Informers #14596
Conversation
|
@openshift/devex ptal |
| eventBroadcaster := record.NewBroadcaster() | ||
| eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: v1core.New(kubeExternalClient.Core().RESTClient()).Events("")}) | ||
|
|
||
| buildClient := buildclient.NewOSClientBuildClient(openshiftClient) |
There was a problem hiding this comment.
so this one is not using cached builds?
There was a problem hiding this comment.
it's not ... it's used for the patch and delete operations
|
i guess it doesn't fix all of #14049, just the buildconfig side. the build controller logic that calls it would still need to change, unless that's already covered by your changes to the build controller. |
This PR includes changes to the build controller to use the informer for pruning |
|
[testextended][extended:core(builds)] |
|
lgtm if extended tests pass |
|
[test] |
csrwng
force-pushed
the
buildconfig_controller
branch
6 times, most recently
from
75250a7 to
3725d7c
Compare
csrwng
force-pushed
the
buildconfig_controller
branch
from
3725d7c to
c6e150c
Compare
|
#14229 |
|
the failed extended test is for build pruning so that looks suspicious given the changes in this pr. |
The test passes locally. Will look more closely at the logs from the test and see if there's some flake introduced by the use of the cache. |
|
@bparees so using the cache, it looks like the build that was just updated to completed is not updated in the cache yet when we list the builds for pruning. Would you be ok with reverting the use of the cache for the build lister? |
|
@bparees added commit to revert to using the rest client to list builds for pruning, ptal |
|
lgtm |
|
woo. squish+merge |
csrwng
force-pushed
the
buildconfig_controller
branch
from
664395d to
da9b120
Compare
|
[merge][severity:blocker] |
|
Evaluated for origin testextended up to da9b120 |
|
continuous-integration/openshift-jenkins/merge Waiting: You are in the build queue at position: 3 |
|
Evaluated for origin merge up to da9b120 |
|
continuous-integration/openshift-jenkins/test Waiting: Determining build queue position |
|
continuous-integration/openshift-jenkins/testextended FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/623/) (Base Commit: 39a2785) (Extended Tests: core(builds)) |
|
acceptable extended test flake. |
|
@csrwng is there an issue or a comment on an issue somewhere, so that we can remember to come back to the pruning cache thing? |
|
@jim-minter we do now :) #14638 |
|
[test] |
|
Evaluated for origin test up to da9b120 |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2255/) (Base Commit: eb50c84) |
|
Flake #13980 |
Switches to using informers for the BuildConfig controller.
Switches to using caches for Build pruning.
Fixes #14049
Depends on #14289