cvo: Record history of changes to version and report target version#63
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
approved
smarterclayton
force-pushed
the
version_history
branch
3 times, most recently
from
a81633b to
5fb1f18
Compare
openshift-ci-robot
added
size/XXL
|
/test e2e-aws |
openshift-ci-robot
added
the
needs-rebase
smarterclayton
force-pushed
the
version_history
branch
from
f9a72a6 to
b39aabf
Compare
openshift-ci-robot
removed
the
needs-rebase
smarterclayton
force-pushed
the
version_history
branch
2 times, most recently
from
0e39999 to
77ed8fd
Compare
|
/retest |
|
/test e2e-aws |
| StartedTime metav1.Time `json:"startedTime"` | ||
| CompletionTime *metav1.Time `json:"completionTime"` | ||
|
|
||
| // version is a semantic versioning identifying the update version. When this |
There was a problem hiding this comment.
can we just reuse Update or just embed it.. ?
There was a problem hiding this comment.
We could embed it, although sometimes I dislike embedding when the structs are used in different contexts to avoid accidentally getting a field we don't want. I don't care too much.
There was a problem hiding this comment.
Best reason to have different is to have different godoc / apidoc. I think in this case we want different godoc.
| @@ -465,11 +464,14 @@ func (optr *Operator) getOrCreateClusterVersion() (*configv1.ClusterVersion, boo | |||
|
|
|||
| // versionString returns a string describing the current version. | |||
There was a problem hiding this comment.
ques: .status.desired and .status.history[0] both give us current version that CVO is reconciling towards, correct? and we have the .status.desired as that is easy to show in oc get clusterversion ?
|
@smarterclayton except #63 (comment) i think this change looks good. |
|
Let me create the PR against openshift/api and we can discuss a bit more there. |
The CVO needs to clearly communicate to users what the recent changes to the version have been and what version it is targeting right now. Add a `history` field that tracks transitions in desiredUpdate, and a status field `desired` that reports when the operator is trying to reach a stable state. --- version: Allow user to set only version string and infer payload from history As a convenience to users and UI, allow only the version field to be specified in the desiredUpdate field. If a matching version exists in the history or the availableUpdates field, the update will proceed. If it does not, the CV will be marked as invalid. Take care to record the initial version in the history so the original payload is always available to recover if the user makes a mistake. Use a hash of the payload on disk rather than a version number to prevent collisions between mislabeled versions.
smarterclayton
force-pushed
the
version_history
branch
from
77ed8fd to
558ae02
Compare
smarterclayton
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
Updated with latest API changes from upstream. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
The logic I'm removing is a convenience from 558ae02 (cvo: Record history of changes to version and report target version, 2019-01-08, openshift#63). But the current consensus is that: * Rollbacks are risky enough that we don't want to make them that convenient. * Rollbacks have never been supported, so removing the convenient pullspec lookup is a backwards-compatible-enough change. This also brings the ClusterVersion operator's approach into line with how 'oc adm upgrade --to ...' has always worked [1]. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1878925
5 participants
The CVO needs to clearly communicate to users what the recent changes to
the version have been and what version it is targeting right now.
Add a
historyfield that tracks transitions in desiredUpdate, and astatus field
desiredthat reports when the operator is trying to reach astable state.
currentbecomes a pointer - it is set ofdesiredis unset and vice versa.Example:
Cancel is interesting today - if you catch it before the CVO reboots, it goes back to 4.0.1. If you catch it after, you stay at 4.0.2. Since we want to encourage roll forward, it kind of makes sense, just feels ugly to an end user.
As a second change, allow a user to specify only
versionon a desired update and infer the payload from the availableUpdates or history. If the version doesn't match, report an invalid condition.Change how directory filenames are created by using an md5 sum of the payload to avoid scenarios where the payload image string changes for the same version but we use the old content.