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
Bug 1854857: initial create errors should map to SamplesExists instead of ImageChangesInProgress #303
Bug 1854857: initial create errors should map to SamplesExists instead of ImageChangesInProgress #303
Conversation
@gabemontero: This pull request references Bugzilla bug 1854857, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gabemontero 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 |
terraform/aws flake on e2e-aws /test e2e-aws |
Hmmm .... the
may get to temporarily disable that test and open a SCL bz/jira |
/test e2e-aws |
https://projects.engineering.redhat.com/browse/RHELPLAN-48796 has been opened for the new django template issue |
/override ci/prow/e2e-aws-image-ecosystem |
@gabemontero: Overrode contexts on behalf of gabemontero: ci/prow/e2e-aws-image-ecosystem In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@gabemontero: All pull requests linked via external trackers have merged: openshift/cluster-samples-operator#303. Bugzilla bug 1854857 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/cherrypick release-4.5 |
@gabemontero: new pull request created: #304 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
So we finally got an intermittent situation when the api server was momentarily unavailable during an upgrade when samples operator tried to upsert new samples.
In setting the error condition, the code (since 4.1 I believe) set the error on the ImageChangesInProgress condition.
However, that results in text in the reason field which would not get cleared as we start tracking import completion in the ImageChangesInProgress reason field. As it never gets cleared, we never set Progressing to false.
The pattern in samples operator is to set these errors in SamplesExists, as that reflects whether samples are fully created/up to date.
This PR hence fixes this single inconsistency. I went back and confirmed this was the only place this situation was occurring.
Also added some additional unit tests to distinguish between api server get errors and upsert errors.