Enable Operator Discovery #1431

njhale · 2020-04-07T15:00:27Z

Description of the change:

Add the operators.coreos.com/v2alpha1 API group and Operator resource with component selection.

openshift-ci-robot · 2020-04-07T15:01:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: njhale

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

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

cmd/olm/manager.go

exdx · 2020-04-07T18:29:08Z

The operator discovering other operators. The meta has arrived

ecordell

Looking good!

ecordell · 2020-04-08T14:36:52Z

cmd/olm/main.go

@@ -14,12 +14,13 @@ import (
 	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
 	"github.com/prometheus/client_golang/prometheus/promhttp"


nit: import order

ecordell · 2020-04-08T14:38:14Z

cmd/olm/main.go

@@ -227,5 +234,10 @@ func main() {
 		go monitor.Run(op.Done())
 	}

+	// Start the controller manager
+	if err := mgr.Start(ctx.Done()); err != nil {


this will bypass our clusteroperator reporting, won't it?

What part in particular? No reporting that currently exists has been removed, is there anything of the Operator API/controller we want/need to add to reporting at this point?

Ah, sorry - I just meant that the healthiness of this control loop does not affect the clusteroperator's health. That is not necessarily something we need to figure out right now

ecordell · 2020-04-08T14:38:56Z

cmd/olm/manager.go

+	}
+
+	// Setup a new controller to reconcile Operators
+	setupLog.Info("configuring controller")


I would remove these info lines or change to debug

I bumped the log level to 4, so we shouldn't see these unless -v=4 at minimum.

deploy/chart/crds/0000_50_olm_00-installplans.yaml

deploy/chart/crds/0000_50_olm_00-subscriptions.yaml

ecordell · 2020-04-08T14:41:56Z

pkg/controller/operators/operator_controller.go

+		Watches(&source.Kind{Type: &apiregistrationv1.APIService{}}, enqueueOperator).
+		Watches(&source.Kind{Type: &operatorsv1alpha1.Subscription{}}, enqueueOperator).
+		Watches(&source.Kind{Type: &operatorsv1alpha1.InstallPlan{}}, enqueueOperator).
+		Watches(&source.Kind{Type: &operatorsv1alpha1.ClusterServiceVersion{}}, enqueueOperator).


This list needs to be kept in sync with the supported bundle types - i.e. configmap and secret support will merge soon.

true -- not sure how we want to handle it. This is a stopgap for generic "watch all" behavior.

ecordell · 2020-04-08T14:43:46Z

pkg/feature/feature.go

+	// OperatorLifecycleManagerV2 enables OLM's v2 APIs.
+	// owner: @njhale
+	// alpha: v0.15.0
+	OperatorLifecycleManagerV2 featuregate.Feature = "OperatorLifecycleManagerV2"


We probably want to be more granular than this. i.e. OperatorV1

My thinking here is that we're probably going to enable all of the v2 APIs at once when we jump to v2. Do you see a situation where we want to enable APIs independently? e.g. InstallV2 w/o OperatorV2?

Dockerfile

Makefile

ecordell · 2020-04-14T00:34:36Z

Makefile

+# Install kubebuilder if not found
+KUBEBUILDER_DIR := /tmp/kubebuilder_2.3.1_$(OS)_$(ARCH)/bin 
+kubebuilder:
+ifeq (, $(shell which kubebuilder))


personally I manage kubebuilder with brew. is it better ux to just log a message asking you to install it?

probably, but we want this to run automatically and it's not clear to me that openshift-ci runs unit tests in an image that we can control the build of (the src image). If we do, then I'm all for that route instead.

We control the base image there - see base.Dockerfile. It would be great to not download this every time and be dependent on the network

sure, I can add it there.

Makefile

ecordell · 2020-04-14T00:42:56Z

pkg/controller/operators/decorators/operator.go

+	return
+}
+
+type OperatorFactory interface {


nit: naming here is getting a bit confusing for the callsites, right? operator := factory.NewOperator(operator)?

ecordell · 2020-04-14T00:46:01Z

pkg/controller/operators/operator_controller.go

+	}
+
+	return &OperatorReconciler{
+		Client: cli,


ecordell · 2020-04-14T00:49:19Z

pkg/controller/operators/suite_test.go

+		[]Reporter{printer.NewlineReporter{}})
+}
+
+var _ = BeforeSuite(func() {


does this conflict with the test runner that Ben worked on in (see: x.mk)?

I don't think so. This is scoped to the suite defined in this package, not to mention that the test sets options for the e2e tests and not unit tests.

ecordell · 2020-04-14T00:50:32Z

pkg/feature/feature.go

+}
+
+var featureGates = map[featuregate.Feature]featuregate.FeatureSpec{
+	// OperatorLifecycleManagerV2: {Default: false, PreRelease: featuregate.Alpha},


ecordell · 2020-04-14T00:51:12Z

pkg/lib/codec/mapstructure.go

+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func MetaTimeHookFunc() mapstructure.DecodeHookFunc {


I expected a small unit test for this

njhale · 2020-04-15T05:06:40Z

/hold

until API fix is tagged and module version is bumped.

njhale · 2020-04-15T12:43:37Z

/retest

njhale · 2020-04-15T22:34:41Z

/hold cancel

njhale · 2020-04-16T14:20:05Z

/retest

Add support for reconciling the v2alpha1 Operator API: - generate Operator resources when none exist and labeled components found - aggregate component references in the status of their respective Operators - aggregate component status conditions to their respective Operators - enabled by specifying the "OperatorLifecycleManagerV2" feature gate option when invoking the olm command Also: - bump go version to 1.13 to satisfy controller-runtime - swap some API types imports to the operator-framework/api module - regenerate clients and fakes

- Add an e2e test for the v2alpha1 Operator resource - Add some e2e utilities for watching resource changes

- Make kubebuilder assets configurable using the KUBEBUILDER_ASSETS environment variable - Remove automatic kubebuilder installation - Throw make error if any kubebuilder assets are missing

- Add ConfigMaps to the set of resource types that can be Operator components (tracked by the controller) - Remove extra comment - Remove unused dynamic source implementation

For some reason the top-level update call was allowing updates to Operator CR status subresources when its CRD was submitted as v1beta1. Status updates broke as soon as we updated to v1 CRDs and the status specific update became required.

Stop Operator unit test controller manager before stopping testenv to prevent moot error message spam in test output.

Bump api module to v0.3.0 to get v1 CRDs with corrected schemas.

njhale · 2020-04-16T19:05:16Z

/retest

njhale · 2020-04-16T19:11:36Z

/retest

njhale · 2020-04-16T20:39:16Z

/test all

openshift-ci-robot · 2020-04-16T20:40:31Z

@njhale: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp-upgrade	`acd1067`	link	`/test e2e-gcp-upgrade`

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.

njhale · 2020-04-17T01:49:30Z

Superseded by #1453

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 7, 2020

openshift-ci-robot requested review from dinhxuanvu and exdx April 7, 2020 15:01

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 7, 2020

exdx reviewed Apr 7, 2020

View reviewed changes

cmd/olm/manager.go Show resolved Hide resolved

ecordell reviewed Apr 8, 2020

View reviewed changes

njhale force-pushed the op-disc branch 10 times, most recently from 1b5abae to c8744fb Compare April 13, 2020 22:41

njhale changed the title ~~WIP: Enable Operator Discovery~~ Enable Operator Discovery Apr 13, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 13, 2020

ecordell reviewed Apr 14, 2020

View reviewed changes

njhale force-pushed the op-disc branch from 5a252d2 to d86b23e Compare April 14, 2020 16:24

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 15, 2020

njhale force-pushed the op-disc branch from 4940268 to 17bf451 Compare April 15, 2020 05:22

njhale mentioned this pull request Apr 15, 2020

chore(docker): add kubebuilder to base image #1446

Merged

njhale force-pushed the op-disc branch 2 times, most recently from 1a60d47 to fc3af27 Compare April 15, 2020 22:34

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 15, 2020

njhale added 11 commits April 16, 2020 14:10

chore(make): copy crd manifests from apis repo

dfd3bc4

test(operators): add e2e test for the v2alpha1 operator resource

a8216cb

- Add an e2e test for the v2alpha1 Operator resource - Add some e2e utilities for watching resource changes

chore(modules): update vendor dir

3cae891

chore(logs): set manager setup log level to 4

50c0072

chore(make): use existing kubebuilder installation

2ff668b

- Make kubebuilder assets configurable using the KUBEBUILDER_ASSETS environment variable - Remove automatic kubebuilder installation - Throw make error if any kubebuilder assets are missing

test(lib): add unit test for metav1 time conversion hook

f14dc5d

feat(operator): enable configmaps to be operator components

bec6431

- Add ConfigMaps to the set of resource types that can be Operator components (tracked by the controller) - Remove extra comment - Remove unused dynamic source implementation

test(operator): gracefully stop test controller manager

e1abb5b

Stop Operator unit test controller manager before stopping testenv to prevent moot error message spam in test output.

fix(crds): bump api module to v0.3.0

acd1067

Bump api module to v0.3.0 to get v1 CRDs with corrected schemas.

njhale force-pushed the op-disc branch from b078667 to acd1067 Compare April 16, 2020 18:11

ecordell mentioned this pull request Apr 16, 2020

Operator Discovery + Kube 1.18 #1453

Closed

5 tasks

njhale closed this Apr 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Operator Discovery #1431

Enable Operator Discovery #1431

njhale commented Apr 7, 2020 •

edited

openshift-ci-robot commented Apr 7, 2020

exdx commented Apr 7, 2020

ecordell left a comment

ecordell Apr 8, 2020

ecordell Apr 8, 2020

njhale Apr 14, 2020

ecordell Apr 14, 2020

ecordell Apr 8, 2020

njhale Apr 14, 2020

ecordell Apr 8, 2020

njhale Apr 14, 2020

ecordell Apr 8, 2020

njhale Apr 14, 2020 •

edited

ecordell Apr 14, 2020

njhale Apr 14, 2020

ecordell Apr 14, 2020

njhale Apr 14, 2020

njhale Apr 14, 2020

ecordell Apr 14, 2020

ecordell Apr 14, 2020

ecordell Apr 14, 2020

njhale Apr 14, 2020

ecordell Apr 14, 2020

ecordell Apr 14, 2020

njhale Apr 16, 2020

njhale commented Apr 15, 2020

njhale commented Apr 15, 2020

njhale commented Apr 15, 2020

njhale commented Apr 16, 2020

njhale commented Apr 16, 2020

njhale commented Apr 16, 2020

njhale commented Apr 16, 2020

openshift-ci-robot commented Apr 16, 2020 •

edited

njhale commented Apr 17, 2020

Enable Operator Discovery #1431

Enable Operator Discovery #1431

Conversation

njhale commented Apr 7, 2020 • edited

openshift-ci-robot commented Apr 7, 2020

exdx commented Apr 7, 2020

ecordell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njhale Apr 14, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njhale commented Apr 15, 2020

njhale commented Apr 15, 2020

njhale commented Apr 15, 2020

njhale commented Apr 16, 2020

njhale commented Apr 16, 2020

njhale commented Apr 16, 2020

njhale commented Apr 16, 2020

openshift-ci-robot commented Apr 16, 2020 • edited

njhale commented Apr 17, 2020

njhale commented Apr 7, 2020 •

edited

njhale Apr 14, 2020 •

edited

openshift-ci-robot commented Apr 16, 2020 •

edited