BUILD-281: add library-go watch/update of new OCM feature gate field by gabemontero · Pull Request #227 · openshift/cluster-openshift-controller-manager-operator

gabemontero · 2021-09-10T16:40:26Z

build csi volumes (to facilitate builds with RH entitlements) and the underlying csi driver shared resource CSO operator are tech preview feature gate items for the upcoming release

gabemontero · 2021-09-10T16:41:05Z


 replace vbom.ml/util => github.com/fvbommel/util v0.0.0-20180919145318-efcd4e0f9787
+
+replace github.com/openshift/api => github.com/gabemontero/api v0.0.0-20210910154316-eaa29e1f16df


this captures the latest state of openshift/api#1007

when that merges, we'll just change the version for openshift/api up above

gabemontero · 2021-09-10T16:41:48Z

+
+replace github.com/openshift/api => github.com/gabemontero/api v0.0.0-20210910154316-eaa29e1f16df
+
+replace github.com/openshift/library-go => github.com/gabemontero/library-go v0.0.0-20210910154638-8565ba189275


this captures the latest state of openshift/library-go#1201

when that merges, we'll just change the version for openshift/library-go up above

gabemontero · 2021-09-10T16:48:23Z

 		network.ObserveExternalIPAutoAssignCIDRs,
 		deployimages.ObserveControllerManagerImagesConfig,
+		//TODO when https://github.com/openshift/api/pull/982 merges we can add "CSIDriverSharedResource" to that
+		// currently includes "BuildCSIVolumes"


@adambkaplan - conceivably, we don't need both CSIDriverSharedResource that you cited in BUILD-292 and BuildCSIVolumes that you cited in BUILD-281, unless you think CSI shared resource stuff could move out of tech preview mode in a release prior to when Build's consumption of it move out of tech preview mode.

UPDATE: @adambkaplan and I talked and he reminded me that long term we will want to support multiple CSI volume types for builds

hence the multiple feature gates do make sense.

gabemontero · 2021-09-10T17:47:47Z

the e2e-aws-operator failure needs investigation .... not immediately apparent to me what caused that

gabemontero · 2021-09-10T18:19:32Z

Probably related to

pods/openshift-controller-manager-operator_openshift-controller-manager-operator-8b94fc78-h8x6z_openshift-controller-manager-operator_previous.log.gz:E0910 17:03:36.813554 1 runtime.go:78] Observed a panic: &runtime.TypeAssertionError{_interface:(*runtime._type)(nil), concrete:(*runtime._type)(0x24befe0), asserted:(*runtime._type)(0x22f9cc0), missingMethod:"FeatureGateLister"} (interface conversion: configobservation.Listers is not featuregates.FeatureGateLister: missing method FeatureGateLister)

gabemontero · 2021-09-10T18:22:36Z

Full stack:

2021-09-10T17:03:57.359070088Z E0910 17:03:57.359041       1 runtime.go:78] Observed a panic: &runtime.TypeAssertionError{_interface:(*runtime._type)(nil), concrete:(*runtime._type)(0x24befe0), asserted:(*runtime._type)(0x22f9cc0), missingMethod:"FeatureGateLister"} (interface conversion: configobservation.Listers is not featuregates.FeatureGateLister: missing method FeatureGateLister)
2021-09-10T17:03:57.359070088Z goroutine 702 [running]:
2021-09-10T17:03:57.359070088Z k8s.io/apimachinery/pkg/util/runtime.logPanic(0x22f2c60, 0xc0015a4270)
2021-09-10T17:03:57.359070088Z 	k8s.io/apimachinery@v0.22.1/pkg/util/runtime/runtime.go:74 +0x95
2021-09-10T17:03:57.359070088Z k8s.io/apimachinery/pkg/util/runtime.HandleCrash(0xc00100bd58, 0x1, 0x1)
2021-09-10T17:03:57.359070088Z 	k8s.io/apimachinery@v0.22.1/pkg/util/runtime/runtime.go:48 +0x86
2021-09-10T17:03:57.359070088Z panic(0x22f2c60, 0xc0015a4270)
2021-09-10T17:03:57.359070088Z 	runtime/panic.go:965 +0x1b9
2021-09-10T17:03:57.359070088Z github.com/openshift/library-go/pkg/operator/configobserver/featuregates.(*featureFlags).ObserveFeatureFlags(0xc0009828a0, 0x2987ad8, 0xc000582ba0, 0x29d4638, 0xc000856240, 0xc0015a4090, 0x0, 0x0, 0x0, 0x0)
2021-09-10T17:03:57.359070088Z 	github.com/openshift/library-go@v0.0.0-20210817120645-b59564e63303/pkg/operator/configobserver/featuregates/observe_featuregates.go:49 +0xcb
2021-09-10T17:03:57.359070088Z github.com/openshift/library-go/pkg/operator/configobserver.ConfigObserver.sync(0xc0009828d0, 0x6, 0x6, 0x29c88f8, 0xc000993a60, 0x2987ad8, 0xc000582ba0, 0x0, 0x0, 0x0, ...)
2021-09-10T17:03:57.359070088Z 	github.com/openshift/library-go@v0.0.0-20210817120645-b59564e63303/pkg/operator/configobserver/config_observer_controller.go:139 +0x370
2021-09-10T17:03:57.359070088Z github.com/openshift/library-go/pkg/controller/factory.(*baseController).reconcile(0xc000b80900, 0x29bf970, 0xc000eb5c80, 0x29ba810, 0xc0015a4000, 0x0, 0x0)
2021-09-10T17:03:57.359070088Z 	github.com/openshift/library-go@v0.0.0-20210817120645-b59564e63303/pkg/controller/factory/base_controller.go:205 +0x5c
2021-09-10T17:03:57.359070088Z github.com/openshift/library-go/pkg/controller/factory.(*baseController).processNextWorkItem(0xc000b80900, 0x29bf970, 0xc000eb5c80)
2021-09-10T17:03:57.359070088Z 	github.com/openshift/library-go@v0.0.0-20210817120645-b59564e63303/pkg/controller/factory/base_controller.go:264 +0x253
2021-09-10T17:03:57.359070088Z github.com/openshift/library-go/pkg/controller/factory.(*baseController).runWorker.func1(0x29bf970, 0xc000eb5c80)
2021-09-10T17:03:57.359070088Z 	github.com/openshift/library-go@v0.0.0-20210817120645-b59564e63303/pkg/controller/factory/base_controller.go:196 +0xb9
2021-09-10T17:03:57.359070088Z k8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext.func1()
2021-09-10T17:03:57.359070088Z 	k8s.io/apimachinery@v0.22.1/pkg/util/wait/wait.go:185 +0x37
2021-09-10T17:03:57.359070088Z k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0xc0012abef8)
2021-09-10T17:03:57.359070088Z 	k8s.io/apimachinery@v0.22.1/pkg/util/wait/wait.go:155 +0x5f
2021-09-10T17:03:57.359070088Z k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc00100bef8, 0x296ba80, 0xc000156a50, 0xc000eb5c01, 0xc000ec1f80)
2021-09-10T17:03:57.359070088Z 	k8s.io/apimachinery@v0.22.1/pkg/util/wait/wait.go:156 +0x9b
2021-09-10T17:03:57.359070088Z k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc000c74ef8, 0x3b9aca00, 0x0, 0x1, 0xc000ec1f80)
2021-09-10T17:03:57.359070088Z 	k8s.io/apimachinery@v0.22.1/pkg/util/wait/wait.go:133 +0x98
2021-09-10T17:03:57.359070088Z k8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext(0x29bf970, 0xc000eb5c80, 0xc000c74f58, 0x3b9aca00, 0x0, 0x1)
2021-09-10T17:03:57.359070088Z 	k8s.io/apimachinery@v0.22.1/pkg/util/wait/wait.go:185 +0xa6
2021-09-10T17:03:57.359070088Z k8s.io/apimachinery/pkg/util/wait.UntilWithContext(...)
2021-09-10T17:03:57.359070088Z 	k8s.io/apimachinery@v0.22.1/pkg/util/wait/wait.go:99
2021-09-10T17:03:57.359070088Z github.com/openshift/library-go/pkg/controller/factory.(*baseController).runWorker(0xc000b80900, 0x29bf970, 0xc000eb5c80)
2021-09-10T17:03:57.359070088Z 	github.com/openshift/library-go@v0.0.0-20210817120645-b59564e63303/pkg/controller/factory/base_controller.go:187 +0x74
2021-09-10T17:03:57.359070088Z github.com/openshift/library-go/pkg/controller/factory.(*baseController).Run.func2(0xc000b80900, 0xc0012837b0, 0x29bf970, 0xc000eb5c80)
2021-09-10T17:03:57.359070088Z 	github.com/openshift/library-go@v0.0.0-20210817120645-b59564e63303/pkg/controller/factory/base_controller.go:117 +0x75
2021-09-10T17:03:57.359070088Z created by github.com/openshift/library-go/pkg/controller/factory.(*baseController).Run
2021-09-10T17:03:57.359070088Z 	github.com/openshift/library-go@v0.0.0-20210817120645-b59564e63303/pkg/controller/factory/base_controller.go:112 +0x2f6
2021-09-10T17:03:57.359143072Z I0910 17:03:57.359117       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"openshift-controller-manager-operator", Name:"openshift-controller-manager-operator", UID:"8fc9ca17-7b15-4ce3-9a69-6922853ec228", APIVersion:"apps/v1", ResourceVersion:"", FieldPath:""}): type: 'Warning' reason: 'Openshift-Controller-Manager-OperatorPanic' Panic observed: interface conversion: configobservation.Listers is not featuregates.FeatureGateLister: missing method FeatureGateLister

gabemontero · 2021-09-13T18:33:11Z

now down to unrelated sig flakes

/test e2e-aws

gabemontero · 2021-09-13T19:50:12Z

/test e2e-upgrade

gabemontero · 2021-09-13T23:46:01Z

/retest

gabemontero · 2021-09-14T12:56:56Z

/test e2e-aws

gabemontero · 2021-09-29T13:22:55Z

/assign @adambkaplan

OK I've vendored in the merged openshift/api and openshift/library-go changes for our new openshiftcontrolplane feature gate []string field

…config

gabemontero · 2021-09-29T15:24:25Z

e2e-aws perm fail seems unrelated ... let's see

/test e2e-aws

gabemontero · 2021-09-29T17:28:58Z

/retest

gabemontero · 2021-09-29T21:12:14Z

/retest

adambkaplan

/approve

Nits on the underscore, but otherwise looks good.

adambkaplan · 2021-09-30T19:13:12Z

 	BuildConfigLister  configlistersv1.BuildLister
 	ConfigMapLister    corelistersv1.ConfigMapLister
 	NetworkLister      configlistersv1.NetworkLister
+	FeatureGateLister_ configlistersv1.FeatureGateLister


Accidental underscore?

Suggested change

FeatureGateLister_ configlistersv1.FeatureGateLister

FeatureGateLister configlistersv1.FeatureGateLister

adambkaplan · 2021-09-30T19:13:22Z

 }
+
+func (l Listers) FeatureGateLister() configlistersv1.FeatureGateLister {
+	return l.FeatureGateLister_


Suggested change

return l.FeatureGateLister_

return l.FeatureGateLister

adambkaplan · 2021-09-30T19:14:58Z

 		builds.ObserveBuildControllerConfig,
 		network.ObserveExternalIPAutoAssignCIDRs,
 		deployimages.ObserveControllerManagerImagesConfig,
+		featuregates.NewObserveFeatureFlagsFunc(sets.NewString("BuildCSIVolumes"), sets.String{}, []string{"featureGates"}),


adambkaplan · 2021-09-30T19:15:15Z

+			ImageConfigLister:  configInformers.Config().V1().Images().Lister(),
+			BuildConfigLister:  configInformers.Config().V1().Builds().Lister(),
+			NetworkLister:      configInformers.Config().V1().Networks().Lister(),
+			FeatureGateLister_: configInformers.Config().V1().FeatureGates().Lister(),


Suggested change

FeatureGateLister_: configInformers.Config().V1().FeatureGates().Lister(),

FeatureGateLister: configInformers.Config().V1().FeatureGates().Lister(),

adambkaplan · 2021-09-30T19:17:46Z

/assign @jkhelil

Assigning follow up review to Jawed since he looked into the build controller enhancement before sprint planning.

gabemontero · 2021-09-30T19:44:33Z

/approve

Nits on the underscore, but otherwise looks good.

underscores are actually needed @adambkaplan or you get a compile error (method / field name the same)

after getting stuck with this, I saw the analogous fix in the kube api server controller

I'll try and (re)find the link

gabemontero · 2021-09-30T19:47:02Z

/approve
Nits on the underscore, but otherwise looks good.

underscores are actually needed @adambkaplan or you get a compile error (method / field name the same)

after getting stuck with this, I saw the analogous fix in the kube api server controller

I'll try and (re)find the link

yeah here is the link: https://github.com/openshift/cluster-kube-apiserver-operator/blob/master/pkg/operator/configobservation/interfaces.go#L17-L24

jkhelil · 2021-10-01T12:33:10Z

/lgtm

adambkaplan · 2021-10-01T13:09:08Z

/lgtm

openshift-ci · 2021-10-01T13:09:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, gabemontero, jkhelil

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~~ [adambkaplan]

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 Sep 10, 2021

openshift-ci Bot requested review from deads2k and sttts September 10, 2021 16:40

gabemontero commented Sep 10, 2021

View reviewed changes

gabemontero mentioned this pull request Sep 10, 2021

BUILD-281: add feature gate, OCM configuration bit, to enable build csi volume tech preview openshift/api#1007

Merged

gabemontero force-pushed the bld-csi-vol-mnt-cfg-ocm branch 2 times, most recently from 5502a71 to 9355044 Compare September 13, 2021 17:05

gabemontero force-pushed the bld-csi-vol-mnt-cfg-ocm branch from 9355044 to f96d2d9 Compare September 22, 2021 18:00

gabemontero mentioned this pull request Sep 28, 2021

BUILD-281: bump(*): openshift/api; update feature flag unit tests openshift/library-go#1201

Merged

bump(*): openshift/api,library-go

aa3ccc9

gabemontero force-pushed the bld-csi-vol-mnt-cfg-ocm branch from 745b53f to 11ae14a Compare September 29, 2021 13:21

gabemontero changed the title ~~WIP: BUILD-281: add library-go watch/update of new OCM feature gate field~~ BUILD-281: add library-go watch/update of new OCM feature gate field Sep 29, 2021

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 Sep 29, 2021

openshift-ci Bot assigned adambkaplan Sep 29, 2021

turn on library-go feature gate observation for new field in the OCM …

be69d56

…config

gabemontero force-pushed the bld-csi-vol-mnt-cfg-ocm branch from 11ae14a to be69d56 Compare September 29, 2021 13:24

adambkaplan approved these changes Sep 30, 2021

View reviewed changes

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 30, 2021

openshift-ci Bot assigned jkhelil Sep 30, 2021

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2021

openshift-merge-robot merged commit 179b82c into openshift:master Oct 1, 2021

gabemontero deleted the bld-csi-vol-mnt-cfg-ocm branch October 1, 2021 14:42


		replace vbom.ml/util => github.com/fvbommel/util v0.0.0-20180919145318-efcd4e0f9787

		replace github.com/openshift/api => github.com/gabemontero/api v0.0.0-20210910154316-eaa29e1f16df


		replace github.com/openshift/api => github.com/gabemontero/api v0.0.0-20210910154316-eaa29e1f16df

		replace github.com/openshift/library-go => github.com/gabemontero/library-go v0.0.0-20210910154638-8565ba189275

	FeatureGateLister_ configlistersv1.FeatureGateLister
	FeatureGateLister configlistersv1.FeatureGateLister

	FeatureGateLister_: configInformers.Config().V1().FeatureGates().Lister(),
	FeatureGateLister: configInformers.Config().V1().FeatureGates().Lister(),

Conversation

gabemontero commented Sep 10, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabemontero commented Sep 10, 2021

Uh oh!

gabemontero commented Sep 10, 2021

Uh oh!

gabemontero commented Sep 10, 2021

Uh oh!

gabemontero commented Sep 13, 2021

Uh oh!

gabemontero commented Sep 13, 2021

Uh oh!

gabemontero commented Sep 13, 2021

Uh oh!

gabemontero commented Sep 14, 2021

Uh oh!

gabemontero commented Sep 29, 2021

Uh oh!

gabemontero commented Sep 29, 2021

Uh oh!

gabemontero commented Sep 29, 2021

Uh oh!

gabemontero commented Sep 29, 2021

Uh oh!

adambkaplan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adambkaplan commented Sep 30, 2021

Uh oh!

gabemontero commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gabemontero commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkhelil commented Oct 1, 2021

Uh oh!

adambkaplan commented Oct 1, 2021

Uh oh!

openshift-ci Bot commented Oct 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gabemontero commented Sep 30, 2021 •

edited

Loading

gabemontero commented Sep 30, 2021 •

edited

Loading