pkg/cvo: Set NoDesiredImage reason when desired.Image is empty #371
Conversation
openshift-ci-robot
added
the
approved
wking
force-pushed
the
no-desired-image
branch
3 times, most recently
from
4e4014e to
89b9af1
Compare
Dunno what that's about. /retest |
|
Prow must have hiccuped, because e2e-aws failed (on /refresh |
|
/assign @LalatenduMohanty |
docs/user/status.md
Outdated
|
|
||
| The CVO has not been given a release image to reconcile. | ||
|
|
||
| This should not be possible, because clearing [`desiredUpdate`][api-desired-update] should return you to the current CVO's release image. |
There was a problem hiding this comment.
When you say this should not be possible, do you mean this is a bug?
There was a problem hiding this comment.
Also I will suggest pointing to code i.e. [api-desired-update]: https://github.com/openshift/api/blob/34f54f12813aaed8822bb5bc56e97cbbfa92171d/config/v1/types_cluster_version.go#L40-L54 is not a good idea. As code can change and it will result in to a broken link.
There was a problem hiding this comment.
When you say this should not be possible, do you mean this is a bug?
I mean that if it happens, it is a CVO bug.
As code can change and it will result in to a broken link.
My link pins a specific API commit by hash, so it does not float with new API code changes.
There was a problem hiding this comment.
I mean that if it happens, it is a CVO bug.
In that case, lets mention that if that happens it is a CVO bug.
My link pins a specific API commit by hash, so it does not float with new API code changes.
Still this does not sound like a good idea because what happens the API changes and the code snippet does not reflect the current code correctly.
There was a problem hiding this comment.
Still this does not sound like a good idea because what happens the API changes and the code snippet does not reflect the current code correctly.
Then we yell at the API folks for the breaking change ;). But there's no way to float with master and also auto-guard against breaking changes (or even line renumbers). This commit-pinned link uses the same approach we use for vendored code, and I think that makes sense, and we periodically review and bump our pinned commit as needed.
There was a problem hiding this comment.
Because every Failing=True condition should have a reason. Also wordsmith the user-facing docs to replace "synchronize" with "reconcile", because our merge logic is more nuanced than the complete match "synchronize" implies for me. The ClusterOperatorNotAvailable special casing landed with convertErrorToProgressing in c2ac20f (status: Report the operators that have not yet deployed, 2019-04-09, openshift#158).
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: LalatenduMohanty, wking 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 |
Because every
Failing=Truecondition should have a reason.Also wordsmith the user-facing docs to replace "synchronize" with "reconcile", because our merge logic is more nuanced than the complete match "synchronize" implies for me.