Update library-go to set OpenStack provider to external by JoelSpeed · Pull Request #3270 · openshift/machine-config-operator

JoelSpeed · 2022-07-27T13:30:17Z

This PR bumps the openshift library go dependency to include openshift/library-go#1383.

This will mean that OpenStack is always set to external and will force the migration from in-tree to out-of-tree for OpenStack.

yuqi-zhang · 2022-07-27T14:15:34Z

/retest

generally looks fine, seems to be some container failures in the test suite, presumably not related?

yuqi-zhang · 2022-07-27T14:18:10Z

er, no

Building github.com/openshift/machine-config-operator/cmd/machine-config-daemon (machine-config-daemon-4.6.0-202006240615.p0-1565-gaa2411be-dirty, aa2411be8b779c280e7cf2e27956b7a7b43fbdb1) for linux/amd64
# k8s.io/client-go/applyconfigurations/meta/v1
vendor/k8s.io/client-go/applyconfigurations/meta/v1/unstructured.go:64:38: cannot use doc (variable of type *"github.com/googleapis/gnostic/openapiv2".Document) as type *"github.com/google/gnostic/openapiv2".Document in argument to proto.NewOpenAPIData
make: *** [Makefile:39: _build-machine-config-daemon] Error 2
error: build error: error building at STEP "RUN make install DESTDIR=./instroot && tar -C instroot -cf instroot.tar .": error while running runtime: exit status 2

JoelSpeed · 2022-07-27T14:20:58Z

er, no

Building github.com/openshift/machine-config-operator/cmd/machine-config-daemon (machine-config-daemon-4.6.0-202006240615.p0-1565-gaa2411be-dirty, aa2411be8b779c280e7cf2e27956b7a7b43fbdb1) for linux/amd64
# k8s.io/client-go/applyconfigurations/meta/v1
vendor/k8s.io/client-go/applyconfigurations/meta/v1/unstructured.go:64:38: cannot use doc (variable of type *"github.com/googleapis/gnostic/openapiv2".Document) as type *"github.com/google/gnostic/openapiv2".Document in argument to proto.NewOpenAPIData
make: *** [Makefile:39: _build-machine-config-daemon] Error 2
error: build error: error building at STEP "RUN make install DESTDIR=./instroot && tar -C instroot -cf instroot.tar .": error while running runtime: exit status 2

Oops, I'll take a look at this now

cgwalters · 2022-07-27T14:35:36Z

This overlaps with #3252
which has some prep work already split out in #3251

JoelSpeed · 2022-07-27T14:43:23Z

This overlaps with #3252 which has some prep work already split out in #3251

Thanks for pointing that out, I've just spent a few minutes getting things to build. I think we are getting a similar result looking at the resulting go mods.

I've had to migrate the resource lock that you are using to a ConfigMapsLeases resource lock which migrates users from configmaps over to leases as part of a deprecation cycle. Kept that as a separate commit for review.

I'll review your changes and double check what's left to do

yuqi-zhang · 2022-07-27T19:56:48Z

Latest failure shows

hack/../pkg is out of date. Please run make update

some test failures

Are we good to merge #3251 first? It should be a subset now (on a slightly older version for some packages)

JoelSpeed · 2022-07-27T20:09:47Z

Yep, something came up so I didn't get to finish this today. If you merge the other PR first I can rebase and fix the rest in the morning

yuqi-zhang · 2022-07-27T20:16:16Z

Ok thanks, let me review the other PR and try to get it in

MaysaMacedo · 2022-07-28T10:04:33Z

@JoelSpeed It seems it's just missing a make update now?

JoelSpeed · 2022-07-28T12:15:55Z

All the tests seem to be good now, lets see how the E2E goes

yuqi-zhang · 2022-07-28T13:47:40Z

/retest

MaysaMacedo · 2022-07-28T17:45:43Z

/retest

MaysaMacedo · 2022-07-29T14:22:50Z

I see the following in the machine config controller logs:

2022-07-28T18:21:57.275897107Z E0728 18:21:57.275869       1 leaderelection.go:334] error initially creating leader election record: leases.coordination.k8s.io is forbidden: User "system:serviceaccount:openshift-machine-config-operator:machine-config-controller" cannot create resource "leases" in API group "coordination.k8s.io" in the namespace "openshift-machine-config-operator"
2022-07-28T18:22:33.666472558Z E0728 18:22:33.666145       1 leaderelection.go:330] error retrieving resource lock openshift-machine-config-operator/machine-config-controller: leases.coordination.k8s.io "machine-config-controller" is forbidden: User "system:serviceaccount:openshift-machine-config-operator:machine-config-controller" cannot get resource "leases" in API group "coordination.k8s.io" in the namespace "openshift-machine-config-operator"

Seems some permissions are missing?

JoelSpeed · 2022-08-01T08:48:50Z

Seems some permissions are missing?

Yep, I think I've got that sorted in the latest commit

openshift-ci · 2022-08-01T13:04:21Z

@JoelSpeed: all tests passed!

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.

MaysaMacedo · 2022-08-01T13:39:51Z

/cc @cgwalters @sinnykumari Can you take another look at this PR? It's green now. Thanks!

sinnykumari

Overall LGTM

MaysaMacedo · 2022-08-02T12:46:32Z

It LGTM. The installation and e2e test with OpenStack, finalized fine.

sinnykumari · 2022-08-02T12:52:56Z

Thanks Maysa for testing!
/lgtm

MaysaMacedo · 2022-08-02T12:53:27Z

/lgtm

openshift-ci · 2022-08-02T12:58:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed, MaysaMacedo, sinnykumari

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [sinnykumari]

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

openshift-ci · 2022-08-02T12:58:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed, MaysaMacedo, sinnykumari

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [sinnykumari]

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

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 Jul 27, 2022

openshift-ci Bot requested review from jkyros and sinnykumari July 27, 2022 13:35

JoelSpeed force-pushed the openstack-external branch from c51c0bc to 263ea1c Compare July 27, 2022 13:59

JoelSpeed changed the title ~~[WIP] Update library-go to set OpenStack provider to external~~ Update library-go to set OpenStack provider to external Jul 27, 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 Jul 27, 2022

JoelSpeed force-pushed the openstack-external branch from 756bfd7 to e7baa2a Compare July 27, 2022 14:52

JoelSpeed added 3 commits July 28, 2022 08:45

Update library-go to set OpenStack provider to external

452dfa6

Upgrade resource lock to use configmaps leases

d5ddbfd

OpenStack cloud tests should now expect external

4f6c283

JoelSpeed force-pushed the openstack-external branch from e7baa2a to 4f6c283 Compare July 28, 2022 08:02

Update generated files

af48cb5

MaysaMacedo mentioned this pull request Aug 1, 2022

Remove e2e-openstack-ccm-install jobs openshift/release#30952

Merged

Ensure machine-config-controller has correct RBAC for lease migration

c9ceacf

JoelSpeed force-pushed the openstack-external branch from c3bd1b8 to c9ceacf Compare August 1, 2022 10:37

sinnykumari approved these changes Aug 1, 2022

View reviewed changes

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 1, 2022

openshift-ci Bot assigned sinnykumari and MaysaMacedo Aug 2, 2022

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 2, 2022

openshift-merge-robot merged commit b928774 into openshift:master Aug 2, 2022

sinnykumari mentioned this pull request Aug 15, 2022

Bump to latest openshift/client-go #3252

Closed

Conversation

JoelSpeed commented Jul 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuqi-zhang commented Jul 27, 2022

Uh oh!

yuqi-zhang commented Jul 27, 2022

Uh oh!

JoelSpeed commented Jul 27, 2022

Uh oh!

cgwalters commented Jul 27, 2022

Uh oh!

JoelSpeed commented Jul 27, 2022

Uh oh!

yuqi-zhang commented Jul 27, 2022

Uh oh!

JoelSpeed commented Jul 27, 2022

Uh oh!

yuqi-zhang commented Jul 27, 2022

Uh oh!

MaysaMacedo commented Jul 28, 2022

Uh oh!

JoelSpeed commented Jul 28, 2022

Uh oh!

yuqi-zhang commented Jul 28, 2022

Uh oh!

MaysaMacedo commented Jul 28, 2022

Uh oh!

MaysaMacedo commented Jul 29, 2022

Uh oh!

JoelSpeed commented Aug 1, 2022

Uh oh!

openshift-ci Bot commented Aug 1, 2022

Uh oh!

MaysaMacedo commented Aug 1, 2022

Uh oh!

sinnykumari left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

sinnykumari commented Aug 2, 2022

Uh oh!

MaysaMacedo commented Aug 2, 2022

Uh oh!

openshift-ci Bot commented Aug 2, 2022

Uh oh!

openshift-ci Bot commented Aug 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JoelSpeed commented Jul 27, 2022 •

edited

Loading

MaysaMacedo commented Aug 2, 2022 •

edited

Loading