api: Rename Payload to Image in ClusterVersion status #98

smarterclayton · 2019-01-23T17:26:17Z

The payload is delivered as an image, and we have clarified our terminology in multiple spots to say:

An update is delivered in an image
An image carrying an OpenShift update is a release image
A release image is associated with a version of the product
The contents of the release image include the CVO binary and an update payload
The update payload is a set of on disk manifests and associated metadata

This PR includes the openshift/api changes and reacts to those updates.

smarterclayton · 2019-01-23T17:47:41Z

/test integration

abhinavdahiya · 2019-01-23T17:51:29Z

both abd3ec0 and dd266e1 are both changing vendor/github.com/openshift/api/config/v1/types_cluster_version.go ;)

abhinavdahiya · 2019-01-23T17:52:28Z

pkg/cvo/internal/operatorstatus.go

-		glog.V(3).Infof("ClusterOperator %s/%s is not done for version %s; it is version=%v, available=%v, progressing=%v, failing=%v",
-			eos.Namespace, eos.Name, os.Status.Version,
-			eos.Status.Version, available, progressing, failing)
+		glog.V(3).Infof("ClusterOperator %s/%s is not done for version %s; it is available=%v, progressing=%v, failing=%v",


version %s is extra

abhinavdahiya · 2019-01-23T17:53:15Z

pkg/cvo/metrics.go

@@ -133,7 +133,12 @@ func (m *operatorMetrics) Collect(ch chan<- prometheus.Metric) {
 	// output cluster operator version and condition info
 	operators, _ := m.optr.clusterOperatorLister.List(labels.Everything())
 	for _, op := range operators {
-		g := m.clusterOperatorUp.WithLabelValues(op.Namespace, op.Name, op.Status.Version)
+		var firstVersion string


are we assuming the operator version is going to be the first?

No, I just didn't want to remove the metric value. We're going to need to define it, I'll add a todo

One attack vector for clusters is to deny access to new updates or to deliberately send an older user. This commit ensures the date the payload was created is reported back through metrics and up via telemetry. It also adds a new `completed` type for the cluster_version metric that reports the last successfully applied cluster version and the date it was successfully applied the first time (from the new status history field). Finally, we correct the naming for the update type to match the name of the status field to `desired`.

When we removed the apis this should have been disabled. Leaving the comment in case we have a need to do this in the future.

Also contains change of CV.Status.Version -> []Version

The contents of an update must be an image ref. We should name the field what it is. Depends on openshift/api#175 merging first, then we can bump and go.

With the change from `Version string` to `Versions []Version` the comparison is not yet clear. Disable this until the design for version checking lands.

smarterclayton · 2019-01-23T18:10:47Z

Fixed up version info, added two, rebased, also fixed a metric panic (previous failure for e2e-aws) where describe wasn't right.

I'm going to look at trying to add a metric integration test in a follow up (but it has to be serial because of global state, which sucks)

smarterclayton · 2019-01-23T22:18:47Z

/retest

abhinavdahiya · 2019-01-23T22:22:26Z

/lgtm

openshift-ci-robot · 2019-01-23T22:22:29Z

[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

Needs approval from an approver in each of these files:

~~OWNERS~~ [abhinavdahiya,smarterclayton]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

smarterclayton · 2019-01-24T01:13:36Z

/retest

openshift-bot · 2019-01-24T04:51:33Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2019-01-24T06:52:58Z

@smarterclayton: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws	`02a389b`	link	`/test e2e-aws`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

Fixing a TODO from 02a389b (sync: Temporarily stop checking version in ClusterOperator.Status, 2019-01-23, openshift#98). The semantics of the 'operator' entry have been clear since at least openshift/api@40c55f8085 (config/v1/types_cluster_operator: ClusterOperatorStatus doc wordsmithing, 2019-10-28, openshift/api#501).

openshift-ci-robot requested review from crawford and wking January 23, 2019 17:26

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 23, 2019

abhinavdahiya reviewed Jan 23, 2019

View reviewed changes

smarterclayton force-pushed the rename_payload branch 2 times, most recently from 08089e6 to 7444e77 Compare January 23, 2019 17:56

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 23, 2019

smarterclayton added 5 commits January 23, 2019 12:59

hack: update-codegen.sh no longer needs to do anything

93e711d

When we removed the apis this should have been disabled. Leaving the comment in case we have a need to do this in the future.

vendor: Update to latest openshift/api with Image instead of Payload

ae336fb

Also contains change of CV.Status.Version -> []Version

api: Rename payload field to image and update doc references

3956b76

The contents of an update must be an image ref. We should name the field what it is. Depends on openshift/api#175 merging first, then we can bump and go.

sync: Temporarily stop checking version in ClusterOperator.Status

02a389b

With the change from `Version string` to `Versions []Version` the comparison is not yet clear. Disable this until the design for version checking lands.

smarterclayton force-pushed the rename_payload branch from 7444e77 to 02a389b Compare January 23, 2019 18:00

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 23, 2019

openshift-ci-robot assigned abhinavdahiya Jan 23, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2019

openshift-merge-robot merged commit d291428 into openshift:master Jan 24, 2019

abhinavdahiya mentioned this pull request Jan 25, 2019

refactor: Extract payload related functions for clarity #97

Merged

lucab mentioned this pull request Jan 30, 2019

*: rename payload -> image openshift/cincinnati#49

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: Rename Payload to Image in ClusterVersion status #98

api: Rename Payload to Image in ClusterVersion status #98

smarterclayton commented Jan 23, 2019

smarterclayton commented Jan 23, 2019

abhinavdahiya commented Jan 23, 2019

abhinavdahiya Jan 23, 2019

abhinavdahiya Jan 23, 2019

smarterclayton Jan 23, 2019

smarterclayton commented Jan 23, 2019 •

edited

smarterclayton commented Jan 23, 2019

abhinavdahiya commented Jan 23, 2019

openshift-ci-robot commented Jan 23, 2019

smarterclayton commented Jan 24, 2019

openshift-bot commented Jan 24, 2019

openshift-ci-robot commented Jan 24, 2019

api: Rename Payload to Image in ClusterVersion status #98

api: Rename Payload to Image in ClusterVersion status #98

Conversation

smarterclayton commented Jan 23, 2019

smarterclayton commented Jan 23, 2019

abhinavdahiya commented Jan 23, 2019

abhinavdahiya Jan 23, 2019

Choose a reason for hiding this comment

abhinavdahiya Jan 23, 2019

Choose a reason for hiding this comment

smarterclayton Jan 23, 2019

Choose a reason for hiding this comment

smarterclayton commented Jan 23, 2019 • edited

smarterclayton commented Jan 23, 2019

abhinavdahiya commented Jan 23, 2019

openshift-ci-robot commented Jan 23, 2019

smarterclayton commented Jan 24, 2019

openshift-bot commented Jan 24, 2019

openshift-ci-robot commented Jan 24, 2019

smarterclayton commented Jan 23, 2019 •

edited