CFE-654: Feature/olm scaffolding #83

thejasn · 2022-11-21T07:43:52Z

Description

Migration of cert manager operator to use operator-sdk for OLM bundle generation and packaging. It changes how bundles are bootstrapped and tweaked.

OLM scaffolding generated using,

$> operator-sdk init --domain openshift.io --project-name cert-manager-operator --repo github.com/openshift/cert-manager-operator
$> operator-sdk create api --group operator.openshift.io --kind CertManager --namespaced=false --version v1alpha1

Notable Changes

Operator namespace changed from openshift-cert-manager-operator -> cert-manager-operator.
Operand namespace changed from openshift-cert-manager -> cert-manager.
config/alphav1 API dropped for the operator.
Bundle version changed to 0.0.1 (default).
New Dockerfile introduced for CI.

Change Log

apis -> api
- Renamed since config/alphav1 API was removed.
bundle : Regenerated using operator-sdk.
- Operator CSV updated
- Operator rbac regenerated using kubebuilder annotations
- Operator install mode changed from AllNamespaces to SingleNamespace and OwnedNamespace.
cmd : Deleted and main.go moved to project root.
config : Bundle template generated using operator-sdk.
hack
- hack/update-cert-manager-manifests.sh : Update manifest destination as config/crd/bases/.
- hack/update-clientgen.sh and hack/update-deepcopy.sh : Update api file path.
jsonnet : Update script with the new namespaces.
pkg/*
- pkg/cmd/operator/cmd.go : Update operator namespace
- pkg/controller/certmanager_controller.go : Introduced placeholder controller along with required kubebuilder annotations.
- pkg/operator : Re-generate operator client and helper functions.

openshift-ci · 2022-11-21T07:43:57Z

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

thejasn · 2022-11-28T07:28:20Z

/test e2e-operator

thejasn · 2022-11-28T07:28:45Z

/test unit

thejasn · 2022-11-28T07:40:31Z

/test e2e-operator

thejasn · 2022-11-28T07:51:13Z

/test e2e-operator

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml

thejasn · 2022-11-29T10:39:43Z

/test e2e-operator

thejasn · 2022-11-29T10:41:30Z

/test images

thejasn · 2022-11-29T11:46:54Z

/retest-required

thejasn · 2022-11-30T08:00:00Z

/test e2e-operator

thejasn · 2022-11-30T10:28:55Z

/test e2e-operator

thejasn · 2022-12-01T10:16:29Z

/test verify

README.md

pkg/controller/deployment/cert_manager_controller_deployment.go

images/ci/Dockerfile

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml

api/v1alpha1/certmanager_types.go

bundle/metadata/annotations.yaml

thejasn · 2022-12-14T12:59:40Z

/retest

thejasn · 2022-12-14T14:19:38Z

/test e2e-operator
Unrelated AWS side error.

openshift-ci · 2022-12-14T15:32:43Z

@thejasn: all tests passed!

Full PR test history. Your PR dashboard.

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.

thejasn · 2022-12-15T04:38:53Z

/hold
For QE approval

swghosh · 2022-12-15T04:41:56Z

/cc @xingxingxia @geliu2016

xingxingxia · 2022-12-15T07:39:16Z

Checked through PR's description for the changes and some files. Adding qe-approved for requirement from openshift/release#34826
/label qe-approved

thejasn · 2022-12-15T10:00:44Z

/hold cancel

thejasn · 2022-12-15T10:02:01Z

/cc @bergerhoffer @davemulford
As far as docs goes, this PR sets the foundation for operator-sdk based packaging only (will be followed up with additional PRs).

bergerhoffer · 2022-12-15T14:29:33Z

@thejasn docs doesn't normally have to approve eng PRs. If there is any user-facing display text that you'd like me to review before it merges in, I'd be happy to do that.

Or feel free to tag me as a heads up as things come in if they are specific new functionality that I might need to know for documentation purposes. Thanks!

thejasn · 2022-12-15T15:22:04Z

/label docs-approved

davemulford · 2022-12-15T22:13:28Z

I saw the epic in Jira and provided comments there.
/label px-approved

geliu2016

/label qe-approved

TrilokGeer · 2022-12-16T07:47:13Z

/lgtm

openshift-ci · 2022-12-16T07:47:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geliu2016, thejasn, TrilokGeer

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~~ [TrilokGeer,thejasn]

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

alebedev87

Sorry for the last review.

As we discussed the main points are:

API doesn't seem to be mature enough:
- it allows too many configuration permutations some of which can be dangerous/not recommended/not tested
- it doesn't have the vision of how CertManager is supposed to be used (not opinionated), the operator is just oc apply, the operational logic is not quite present
Since the namespaces changed: a migration guide needs to be provided in this repository and in the OpenShift docs

The rest is just small remarks after a brief look through.

alebedev87 · 2022-12-20T08:28:09Z

Makefile

+# GO_REQUIRED_MIN_VERSION = 1.17
+# GO_TEST_FLAGS=-v
+# RUNTIME?=docker
+#
+# APP_NAME?=cert-manager-operator
+# IMAGE_REGISTRY?=registry.svc.ci.openshift.org
+# IMAGE_ORG?=openshift-cert-manager
+# IMAGE_TAG?=latest
+# IMAGE_OPERATOR?=$(IMAGE_REGISTRY)/$(IMAGE_ORG)/cert-manager-operator:$(IMAGE_TAG)
+# IMAGE_OPERATOR_BUNDLE?=$(IMAGE_REGISTRY)/$(IMAGE_ORG)/cert-manager-operator-bundle:$(IMAGE_TAG)
+#
+# TEST_OPERATOR_NAMESPACE?=openshift-cert-manager-operator
+#
+# MANIFEST_SOURCE = https://github.com/cert-manager/cert-manager/releases/download/v1.9.1/cert-manager.yaml
+#
+# OPERATOR_SDK_VERSION?=v1.12.0
+# OPERATOR_SDK?=$(PERMANENT_TMP_GOPATH)/bin/operator-sdk-$(OPERATOR_SDK_VERSION)
+# OPERATOR_SDK_DIR=$(dir $(OPERATOR_SDK))
+
+# Include the library makefiles
+#
+#
+# # $1 - target name
+# # $2 - apis
+# # $3 - manifests
+# # $4 - output
+# $(call add-crd-gen,operator-alpha,./apis/operator/v1alpha1,./bundle/manifests,./bundle/manifests)
+# $(call add-crd-gen,config-alpha,./apis/config/v1alpha1,./bundle/manifests,./bundle/manifests)
+#
+#
+# # generate image targets
+# $(call build-image,cert-manager-operator,$(IMAGE_OPERATOR),./images/ci/Dockerfile,.)
+# $(call build-image,cert-manager-operator-bundle,$(IMAGE_OPERATOR_BUNDLE),./bundle/bundle.Dockerfile,./bundle)
+#


The commented code should be removed.

alebedev87 · 2022-12-20T08:36:29Z

Makefile

+
+KUSTOMIZE := go run sigs.k8s.io/kustomize/kustomize/v4
+
+K8S_ENVTEST_VERSION := 1.21.4


Don't see where it's used, ENVTEST_K8S_VERSION seems to be the one for the envtest.

alebedev87 · 2022-12-20T08:51:36Z

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml

+kind: ClusterServiceVersion
+metadata:
+  annotations:
+    alm-examples: '[]'


Weird, the sample didn't end up in CSV. BTW the sample is better to be useful since it'll be used for the scaffolding of the example in UI (you can created CR instances from there).

alebedev87 · 2022-12-20T12:53:34Z

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml

+metadata:
+  annotations:
+    alm-examples: '[]'
+    capabilities: Basic Install


Capabilities used to be Seamless Upgrade, not sure how aligned it was with the reality but that's a drift from what you had.

alebedev87 · 2022-12-20T12:54:49Z

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml

+apiVersion: operators.coreos.com/v1alpha1
+kind: ClusterServiceVersion
+metadata:
+  annotations:


operators.openshift.io/valid-subscription got lost along the way. BTW the subscription is checked in CVP on CPaaS.

alebedev87 · 2022-12-20T12:55:58Z

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml

+      version: v1
+  description: cert-manager-operator
+  displayName: cert-manager-operator
+  icon:


Icon got lost along the way. It's visible in OperatorHub UI.

alebedev87 · 2022-12-20T12:57:32Z

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml

+    operators.operatorframework.io/builder: operator-sdk-v1.25.1
+    operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
+    repository: https://github.com/openshift/cert-manager-operator
+    support: Red Hat, Inc.


There is quite a drift in the annotations compared to the previous version, one of the annotation which is visible to the end user is containerImage ("before install" menu).

alebedev87 · 2022-12-20T13:00:26Z

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml

+          - create
+          - delete
+          - get
+          - list
+          - patch
+          - update
+          - watch


That is more verbs than used to be. Why?

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 Nov 21, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 21, 2022

thejasn changed the title ~~Feature/olm scaffolding~~ [WIP] CFE-654: Feature/olm scaffolding Nov 28, 2022

thejasn force-pushed the feature/olm-skaffolding branch from 9f86b7c to c437022 Compare November 28, 2022 07:50

ayesha54 reviewed Nov 28, 2022

View reviewed changes

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml Outdated Show resolved Hide resolved

thejasn marked this pull request as ready for review November 29, 2022 11:47

openshift-ci bot requested review from s-urbaniak and swghosh November 29, 2022 11:51

thejasn force-pushed the feature/olm-skaffolding branch 3 times, most recently from ce1b844 to bab136b Compare December 5, 2022 13:12

thejasn changed the title ~~[WIP] CFE-654: Feature/olm scaffolding~~ CFE-654: Feature/olm scaffolding Dec 5, 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 Dec 5, 2022

thejasn mentioned this pull request Dec 6, 2022

CFE-676: Feature/decouple cert manager operator as a clusteroperator #84

Merged

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 9, 2022

TrilokGeer reviewed Dec 12, 2022

View reviewed changes

README.md Show resolved Hide resolved

README.md Show resolved Hide resolved

pkg/controller/deployment/cert_manager_controller_deployment.go Outdated Show resolved Hide resolved

TrilokGeer reviewed Dec 13, 2022

View reviewed changes

images/ci/Dockerfile Outdated Show resolved Hide resolved

bundle/manifests/cert-manager-operator.clusterserviceversion.yaml Show resolved Hide resolved

api/v1alpha1/certmanager_types.go Outdated Show resolved Hide resolved

bundle/metadata/annotations.yaml Outdated Show resolved Hide resolved

Thejas N added 2 commits December 14, 2022 17:11

Update gitignore

d4e876b

Update vendor directory

62f28d9

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

openshift-ci bot requested review from geliu2016 and xingxingxia December 15, 2022 04:41

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Dec 15, 2022

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

openshift-ci bot requested review from bergerhoffer and davemulford December 15, 2022 10:02

openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Dec 15, 2022

openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Dec 15, 2022

geliu2016 approved these changes Dec 16, 2022

View reviewed changes

openshift-ci bot assigned TrilokGeer Dec 16, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2022

openshift-merge-robot merged commit ef377e4 into openshift:master Dec 16, 2022

alebedev87 reviewed Dec 20, 2022

View reviewed changes

swghosh mentioned this pull request Dec 30, 2022

Fix namespace in Makefile, change to default namespace in alm-examples #91

Merged

jianzhangbjz mentioned this pull request Mar 29, 2023

OCPQE-14558: replace cert-manage with service mesh operator openshift/release#37789

Merged

joelanford mentioned this pull request Nov 8, 2023

platform-operators: use build_root.from_repository=true openshift/release#44777

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CFE-654: Feature/olm scaffolding #83

CFE-654: Feature/olm scaffolding #83

thejasn commented Nov 21, 2022 •

edited

openshift-ci bot commented Nov 21, 2022

thejasn commented Nov 28, 2022

thejasn commented Nov 28, 2022

thejasn commented Nov 28, 2022

thejasn commented Nov 28, 2022

thejasn commented Nov 29, 2022

thejasn commented Nov 29, 2022

thejasn commented Nov 29, 2022

thejasn commented Nov 30, 2022

thejasn commented Nov 30, 2022

thejasn commented Dec 1, 2022

thejasn commented Dec 14, 2022

thejasn commented Dec 14, 2022

openshift-ci bot commented Dec 14, 2022

thejasn commented Dec 15, 2022

swghosh commented Dec 15, 2022 •

edited

xingxingxia commented Dec 15, 2022

thejasn commented Dec 15, 2022

thejasn commented Dec 15, 2022

bergerhoffer commented Dec 15, 2022

thejasn commented Dec 15, 2022

davemulford commented Dec 15, 2022

geliu2016 left a comment

TrilokGeer commented Dec 16, 2022

openshift-ci bot commented Dec 16, 2022

alebedev87 left a comment

alebedev87 Dec 20, 2022 •

edited

alebedev87 Dec 20, 2022

alebedev87 Dec 20, 2022

alebedev87 Dec 20, 2022

alebedev87 Dec 20, 2022

alebedev87 Dec 20, 2022

alebedev87 Dec 20, 2022 •

edited

alebedev87 Dec 20, 2022


		KUSTOMIZE := go run sigs.k8s.io/kustomize/kustomize/v4

		K8S_ENVTEST_VERSION := 1.21.4

CFE-654: Feature/olm scaffolding #83

CFE-654: Feature/olm scaffolding #83

Conversation

thejasn commented Nov 21, 2022 • edited

Description

Notable Changes

Change Log

openshift-ci bot commented Nov 21, 2022

thejasn commented Nov 28, 2022

thejasn commented Nov 28, 2022

thejasn commented Nov 28, 2022

thejasn commented Nov 28, 2022

thejasn commented Nov 29, 2022

thejasn commented Nov 29, 2022

thejasn commented Nov 29, 2022

thejasn commented Nov 30, 2022

thejasn commented Nov 30, 2022

thejasn commented Dec 1, 2022

thejasn commented Dec 14, 2022

thejasn commented Dec 14, 2022

openshift-ci bot commented Dec 14, 2022

thejasn commented Dec 15, 2022

swghosh commented Dec 15, 2022 • edited

xingxingxia commented Dec 15, 2022

thejasn commented Dec 15, 2022

thejasn commented Dec 15, 2022

bergerhoffer commented Dec 15, 2022

thejasn commented Dec 15, 2022

davemulford commented Dec 15, 2022

geliu2016 left a comment

Choose a reason for hiding this comment

TrilokGeer commented Dec 16, 2022

openshift-ci bot commented Dec 16, 2022

alebedev87 left a comment

Choose a reason for hiding this comment

alebedev87 Dec 20, 2022 • edited

Choose a reason for hiding this comment

alebedev87 Dec 20, 2022

Choose a reason for hiding this comment

alebedev87 Dec 20, 2022

Choose a reason for hiding this comment

alebedev87 Dec 20, 2022

Choose a reason for hiding this comment

alebedev87 Dec 20, 2022

Choose a reason for hiding this comment

alebedev87 Dec 20, 2022

Choose a reason for hiding this comment

alebedev87 Dec 20, 2022 • edited

Choose a reason for hiding this comment

alebedev87 Dec 20, 2022

Choose a reason for hiding this comment

thejasn commented Nov 21, 2022 •

edited

swghosh commented Dec 15, 2022 •

edited

alebedev87 Dec 20, 2022 •

edited

alebedev87 Dec 20, 2022 •

edited