-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Remove ImageStreamReferenceIndex from BuildInformer #14635
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
Remove ImageStreamReferenceIndex from BuildInformer #14635
Conversation
@bparees this was the issue with the index. I copied this code from the BuildConfig informer and included an index that would always return an error for builds. Because this index function returned an error, if it was the first index to be processed for a particular build, the other index for the namespace wasn't getting processed either. In upstream, maybe the right behavior would be to log the error and continue instead of returning: |
[test] |
Since there is already a lack of compensation for partially inserted keys, aggregating all the errors together seems reasonable. I'd probably suggest logging it using |
Oh, also, you should switch to the generated informers. |
Evaluated for origin test up to e65b77b |
yeah my understanding is @smarterclayton is planning to do that wholesale for all our controllers shortly. |
[testextended][extended:core(builds)] |
@csrwng do you want to include re-enabling the cache for pruning w/ this PR then? |
I'd rather not have too many dependencies. This PR can go in without requiring the other ones. |
k |
lgtm pending passing extended tests |
Evaluated for origin testextended up to e65b77b |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2208/) (Base Commit: 653b218) |
continuous-integration/openshift-jenkins/testextended FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/625/) (Base Commit: 653b218) (Extended Tests: core(builds)) |
extended test failures are known flakes |
continuous-integration/openshift-jenkins/merge Waiting: You are in the build queue at position: 6 |
Evaluated for origin merge up to e65b77b |
The ImageStreamReferenceIndex doesn't apply to builds. Removes that index from the builds informer.