Bump openshift/api #817

creydr · 2022-08-09T15:36:31Z

Bump openshift/api and its k8s dependencies

Hints for reviewers:
I adjusted the unit tests for the new capabilities.
The TestCVO_InitImplicitlyEnabledCaps seemed to be a bit flacky, because of some unordered slice (ImplicitlyEnabledCaps):

--- FAIL: TestCVO_InitImplicitlyEnabledCaps (0.00s)
    cvo_scenarios_test.go:3891: unexpected status item 0
        Expected: cvo.SyncWorkerStatus{Generation:1, Failure:error(nil), Done:0, Total:3, Completed:0, Reconciling:false, Initial:false, VersionHash:"DL-FFQ2Uem8=", LastProgress:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), Actual:v1.Release{Version:"1.0.1-abc", Image:"image/image:1", URL:"https://example.com/v1.0.1-abc", Channels:[]string(nil)}, Verified:false, loadPayloadStatus:cvo.LoadPayloadStatus{Step:"PayloadLoaded", Message:"Payload loaded version=\"1.0.1-abc\" image=\"image/image:1\"", Failure:error(nil), Update:v1.Update{Version:"1.0.1-abc", Image:"image/image:1", Force:false}, Verified:false, LastTransitionTime:time.Time{wall:0x0, ext:62135596801, loc:(*time.Location)(0x2c9d820)}}, CapabilitiesStatus:cvo.CapabilityStatus{Status:v1.ClusterVersionCapabilitiesStatus{EnabledCapabilities:[]v1.ClusterVersionCapability{"Console", "Insights", "baremetal", "marketplace", "openshift-samples"}, KnownCapabilities:[]v1.ClusterVersionCapability{"Console", "Insights", "baremetal", "marketplace", "openshift-samples"}}, ImplicitlyEnabledCaps:[]v1.ClusterVersionCapability{"marketplace", "openshift-samples", "Console", "Insights"}}}
        Actual: cvo.SyncWorkerStatus{Generation:1, Failure:error(nil), Done:0, Total:3, Completed:0, Reconciling:false, Initial:false, VersionHash:"DL-FFQ2Uem8=", LastProgress:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), Actual:v1.Release{Version:"1.0.1-abc", Image:"image/image:1", URL:"https://example.com/v1.0.1-abc", Channels:[]string(nil)}, Verified:false, loadPayloadStatus:cvo.LoadPayloadStatus{Step:"PayloadLoaded", Message:"Payload loaded version=\"1.0.1-abc\" image=\"image/image:1\"", Failure:error(nil), Update:v1.Update{Version:"1.0.1-abc", Image:"image/image:1", Force:false}, Verified:false, LastTransitionTime:time.Time{wall:0x0, ext:62135596801, loc:(*time.Location)(0x2c9d820)}}, CapabilitiesStatus:cvo.CapabilityStatus{Status:v1.ClusterVersionCapabilitiesStatus{EnabledCapabilities:[]v1.ClusterVersionCapability{"Console", "Insights", "baremetal", "marketplace", "openshift-samples"}, KnownCapabilities:[]v1.ClusterVersionCapability{"Console", "Insights", "baremetal", "marketplace", "openshift-samples"}}, ImplicitlyEnabledCaps:[]v1.ClusterVersionCapability{"openshift-samples", "Console", "Insights", "marketplace"}}}

in https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_cluster-version-operator/817/pull-ci-openshift-cluster-version-operator-master-unit/1557028368216494080/artifacts/test/build-log.txt
(can be seen as the unit tests in this PR are flaky before commit 8278319).

So I added a sort in the test before comparing the actual with expected status:

cluster-version-operator/pkg/cvo/cvo_scenarios_test.go

Line 3933 in 0fa94e7

sort.Sort(ByCapability(actual.CapabilitiesStatus.ImplicitlyEnabledCaps))

in 8278319

Support for resourcelock.ConfigMapLock will be removed in one of the next k8s client-go releases. Therefor we migrate to ConfigMapsLeasesResourceLock. Signed-off-by: Christoph Stäbler <[email protected]>

openshift-ci · 2022-08-09T15:36:36Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

creydr · 2022-08-09T15:37:04Z

/test ?

openshift-ci · 2022-08-09T15:37:17Z

@creydr: The following commands are available to trigger required jobs:

/test e2e-agnostic
/test e2e-agnostic-operator
/test e2e-agnostic-upgrade
/test gofmt
/test images
/test lint
/test unit

Use /test all to run all jobs.

Details

In response to this:

/test ?

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.

creydr · 2022-08-09T15:37:43Z

/test unit

creydr · 2022-08-11T12:37:36Z

/test all

creydr · 2022-08-11T14:51:48Z

/assign @LalatenduMohanty
I was able to fix the flaky unit test. Could you PTAL?
Thanks in advance

creydr · 2022-08-11T20:16:28Z

/retest

creydr · 2022-08-12T06:18:06Z

/retest

rluders · 2022-08-15T10:51:44Z

Well, this one seems to contain the changes from #812 and #820, so I guess that we want to proceed by getting this one merged. Don't we?

@creydr @jottofar @wking WDYT?

$ go get github.com/openshift/api@51f399230d604fa013c2bb341040c4ad126e7309 $ go get github.com/openshift/client-go@984ee5ebedcf $ go get github.com/openshift/library-go@84c00ba9649a $ go mod tidy $ go mod vendor $ git add -A go.* vendor Signed-off-by: Christoph Stäbler <[email protected]>

Co-authored-by: Ricardo Lüders <[email protected]> Signed-off-by: Christoph Stäbler <[email protected]>

LalatenduMohanty

The PR looks fine to me except the sequence of commits. The commit for Migrate to ConfigMapsLeasesResourceLock should be after the API bump commit

creydr · 2022-08-15T18:01:42Z

The PR looks fine to me except the sequence of commits. The commit for Migrate to ConfigMapsLeasesResourceLock should be after the API bump commit

The reason for having it before was that we would always have a "working" build. (the ConfigMapLock is not available in the new dependencies - was removed/made private in client-go v0.24)

LalatenduMohanty

/lgtm

openshift-ci-robot · 2022-08-15T21:32:31Z

/retest-required

Remaining retests: 2 against base HEAD c699d55 and 8 for PR HEAD c6ddf92 in total

openshift-ci-robot · 2022-08-15T23:32:38Z

/retest-required

Remaining retests: 2 against base HEAD 348add6 and 7 for PR HEAD c6ddf92 in total

openshift-ci-robot · 2022-08-16T04:32:33Z

/retest-required

Remaining retests: 1 against base HEAD 348add6 and 6 for PR HEAD c6ddf92 in total

openshift-ci-robot · 2022-08-16T09:32:35Z

/retest-required

Remaining retests: 0 against base HEAD 348add6 and 5 for PR HEAD c6ddf92 in total

openshift-ci · 2022-08-16T11:34:40Z

@creydr: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-agnostic-upgrade	`c6ddf92`	link	true	`/test e2e-agnostic-upgrade`

Full PR test history. Your PR dashboard.

Details

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.

LalatenduMohanty · 2022-08-16T15:20:24Z

/lgtm cancel as #801 has more recent bump

openshift-ci · 2022-08-16T15:27:05Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: creydr
Once this PR has been reviewed and has the lgtm label, please ask for approval from lalatendumohanty by writing /assign @lalatendumohanty in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

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

creydr · 2022-08-16T15:35:02Z

/close
in favor of #801

openshift-ci · 2022-08-16T15:35:44Z

@creydr: Closed this PR.

Details

In response to this:

/close
in favor of #801

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.

Migrate to ConfigMapsLeasesResourceLock

5e4af14

Support for resourcelock.ConfigMapLock will be removed in one of the next k8s client-go releases. Therefor we migrate to ConfigMapsLeasesResourceLock. Signed-off-by: Christoph Stäbler <[email protected]>

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 9, 2022

creydr force-pushed the update-openshift-api branch 2 times, most recently from 9bcfa02 to 0d0351f Compare August 11, 2022 12:37

creydr force-pushed the update-openshift-api branch 3 times, most recently from 0fa94e7 to 8278319 Compare August 11, 2022 14:41

creydr marked this pull request as ready for review August 11, 2022 14:43

creydr changed the title ~~WIP: Bump openshift/api~~ Bump openshift/api Aug 11, 2022

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 11, 2022

openshift-ci bot requested review from jottofar and wking August 11, 2022 14:46

creydr mentioned this pull request Aug 11, 2022

vendor: bump openshift/api to pick up Insights capability #812

Closed

openshift-ci bot assigned LalatenduMohanty Aug 11, 2022

creydr force-pushed the update-openshift-api branch from 8278319 to 7b0b01a Compare August 15, 2022 16:06

creydr and others added 2 commits August 15, 2022 18:14

Adjust unit tests to match new capabilities

c6ddf92

Co-authored-by: Ricardo Lüders <[email protected]> Signed-off-by: Christoph Stäbler <[email protected]>

creydr force-pushed the update-openshift-api branch from 7b0b01a to c6ddf92 Compare August 15, 2022 16:25

LalatenduMohanty suggested changes Aug 15, 2022

View reviewed changes

LalatenduMohanty approved these changes Aug 15, 2022

View reviewed changes

openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 15, 2022

qJkee mentioned this pull request Aug 16, 2022

[CNF-5645] bump api version to add new capability #801

Merged

openshift-ci bot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 16, 2022

openshift-ci bot closed this Aug 16, 2022

Bump openshift/api #817

Bump openshift/api #817

Uh oh!

Conversation

creydr commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Aug 9, 2022

Uh oh!

creydr commented Aug 9, 2022

Uh oh!

openshift-ci bot commented Aug 9, 2022

Uh oh!

creydr commented Aug 9, 2022

Uh oh!

creydr commented Aug 11, 2022

Uh oh!

creydr commented Aug 11, 2022

Uh oh!

creydr commented Aug 11, 2022

Uh oh!

creydr commented Aug 12, 2022

Uh oh!

rluders commented Aug 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LalatenduMohanty left a comment

Choose a reason for hiding this comment

Uh oh!

creydr commented Aug 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LalatenduMohanty left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Aug 15, 2022

Uh oh!

openshift-ci-robot commented Aug 15, 2022

Uh oh!

openshift-ci-robot commented Aug 16, 2022

Uh oh!

openshift-ci-robot commented Aug 16, 2022

Uh oh!

openshift-ci bot commented Aug 16, 2022

Uh oh!

LalatenduMohanty commented Aug 16, 2022

Uh oh!

openshift-ci bot commented Aug 16, 2022

Uh oh!

creydr commented Aug 16, 2022

Uh oh!

openshift-ci bot commented Aug 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

creydr commented Aug 9, 2022 •

edited

Loading

rluders commented Aug 15, 2022 •

edited

Loading

creydr commented Aug 15, 2022 •

edited

Loading