apis/v1: Add policyEngineURI #66

wking · 2020-10-05T20:19:15Z

So users don't have to figure out the ${NAME}-policy-engine-route route naming on their own.

openshift-ci-robot · 2020-10-05T20:19:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wking

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

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

openshift-ci-robot · 2020-10-05T22:36:46Z

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

Test name	Commit	Details	Rerun command
ci/prow/cincinnati-operator-e2e-aws	`b60214c`	link	`/test cincinnati-operator-e2e-aws`

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.

wking · 2020-10-05T22:53:48Z

e2e:

 Retrieving ginkgo and gomega build dependencies
go: github.com/openshift/cluster-image-registry-operator@v0.0.0-20200415091009-99c06ee64540 requires
	github.com/openshift/[email protected] requires
	bitbucket.org/ww/[email protected]: reading https://api.bitbucket.org/2.0/repositories/ww/goautoneg?fields=scm: 404 Not Found
go: github.com/openshift/cluster-image-registry-operator@v0.0.0-20200415091009-99c06ee64540 requires
	github.com/openshift/[email protected] requires
	bitbucket.org/ww/[email protected]: reading https://api.bitbucket.org/2.0/repositories/ww/goautoneg?fields=scm: 404 Not Found
hack/functest.sh: line 17: /go/bin/ginkgo: No such file or directory
hack/functest.sh: line 19: /go/bin/ginkgo: No such file or directory

Dunno what's up with that. I'm pretty sure we don't actually need goautoneg, but ideally we're building from vendor and it won't matter if the build location download fresh copies or not.

Use Go's built-in testing package instead. This avoids issues like [1]: Retrieving ginkgo and gomega build dependencies go: github.com/openshift/cluster-image-registry-operator@v0.0.0-20200415091009-99c06ee64540 requires github.com/openshift/[email protected] requires bitbucket.org/ww/[email protected]: reading https://api.bitbucket.org/2.0/repositories/ww/goautoneg?fields=scm: 404 Not Found go: github.com/openshift/cluster-image-registry-operator@v0.0.0-20200415091009-99c06ee64540 requires github.com/openshift/[email protected] requires bitbucket.org/ww/[email protected]: reading https://api.bitbucket.org/2.0/repositories/ww/goautoneg?fields=scm: 404 Not Found hack/functest.sh: line 17: /go/bin/ginkgo: No such file or directory hack/functest.sh: line 19: /go/bin/ginkgo: No such file or directory The tests are simple enough that Ginkgo structuring was overkill anyway. Generated by manually editing files in functests, and then running: $ go mod tidy $ go mod vendor $ git add -A go.* vendor using: $ go version go version go1.15.2 linux/amd64 [1]: openshift#66 (comment)

wking · 2020-10-27T20:40:53Z

Rebased around #67 with b586385 -> 7f99c51, as part of sliding this change from one side of #65 to the other.

functests/cincinnati_creation_test.go

jottofar · 2020-10-28T13:10:11Z

/hold
Can we not merge any further code changes until #65 is merged. I have to rebase that PR every time we do.

LalatenduMohanty

Commit 9767242 has a WIP string.

jottofar · 2020-10-30T17:29:59Z

/retest

wking · 2020-11-13T03:47:08Z

Rebased on top of #83 with the kubebuilder:rbac fixups.

Kubebuilder config is from d9f361a (Migrate operator from v0.18.2 to v0.19.3, 2020-09-30, openshift#65) and ce9a447 (Rename operator Update Service and change version to v1, 2020-10-04, openshift#65): $ git blame origin/master controllers/updateservice_controller.go | grep kubebuilder ce9a447 controllers/updateservice_controller.go (Jack Ottofaro 2020-10-04 11:10:37 -0400 125) // +kubebuilder:rbac:groups="",namespace="updateservice-operator",resources=pods;services;services/finalizers;endpoints;persistentvolumeclaims;events;configmaps;secrets,verbs=create;delete;get;list;patch;update;watch ce9a447 controllers/updateservice_controller.go (Jack Ottofaro 2020-10-04 11:10:37 -0400 126) // +kubebuilder:rbac:groups="apps",namespace="updateservice-operator",resources=deployments;daemonsets;replicasets;statefulsets,verbs=create;delete;get;list;patch;update;watch ce9a447 controllers/updateservice_controller.go (Jack Ottofaro 2020-10-04 11:10:37 -0400 127) // +kubebuilder:rbac:groups="monitoring.coreos.com",namespace="updateservice-operator",resources=servicemonitors,verbs=create;get ce9a447 controllers/updateservice_controller.go (Jack Ottofaro 2020-10-04 11:10:37 -0400 128) // +kubebuilder:rbac:groups="apps",namespace="updateservice-operator",resourceNames=updateservice-operator,resources=deployments/finalizers,verbs=update ce9a447 controllers/updateservice_controller.go (Jack Ottofaro 2020-10-04 11:10:37 -0400 129) // +kubebuilder:rbac:groups="",namespace="updateservice-operator",resources=pods,verbs=get ce9a447 controllers/updateservice_controller.go (Jack Ottofaro 2020-10-04 11:10:37 -0400 130) // +kubebuilder:rbac:groups="apps",namespace="updateservice-operator",resources=replicasets;deployments,verbs=get ce9a447 controllers/updateservice_controller.go (Jack Ottofaro 2020-10-04 11:10:37 -0400 131) // +kubebuilder:rbac:groups="policy",namespace="updateservice-operator",resources=poddisruptionbudgets,verbs=create;delete;get;list;patch;update;watch ce9a447 controllers/updateservice_controller.go (Jack Ottofaro 2020-10-04 11:10:37 -0400 132) // +kubebuilder:rbac:groups=updateservice.operator.openshift.io,namespace="updateservice-operator",resources=*,verbs=create;delete;get;list;patch;update;watch d9f361a controllers/cincinnati_controller.go (Jack Ottofaro 2020-09-30 16:08:24 -0400 133) // +kubebuilder:rbac:groups=config.openshift.io,resources=images,verbs=get;list;watch d9f361a controllers/cincinnati_controller.go (Jack Ottofaro 2020-09-30 16:08:24 -0400 134) // +kubebuilder:rbac:groups=route.openshift.io,resources=routes,verbs=create;get;list;patch;update;watch d9f361a controllers/cincinnati_controller.go (Jack Ottofaro 2020-09-30 16:08:24 -0400 135) // +kubebuilder:rbac:groups="",resources=pods;services;services/finalizers;endpoints;persistentvolumeclaims;events;configmaps;secrets,verbs=create;delete;get;list;patch;update;watch d9f361a controllers/cincinnati_controller.go (Jack Ottofaro 2020-09-30 16:08:24 -0400 136) // +kubebuilder:rbac:groups="apps",resources=deployments;daemonsets;replicasets;statefulsets,verbs=create;delete;get;list;patch;update;watch d9f361a controllers/cincinnati_controller.go (Jack Ottofaro 2020-09-30 16:08:24 -0400 137) // +kubebuilder:rbac:groups="apps",resources=replicasets;deployments,verbs=get d9f361a controllers/cincinnati_controller.go (Jack Ottofaro 2020-09-30 16:08:24 -0400 138) // +kubebuilder:rbac:groups="",resources=pods,verbs=get d9f361a controllers/cincinnati_controller.go (Jack Ottofaro 2020-09-30 16:08:24 -0400 139) // +kubebuilder:rbac:groups="monitoring.coreos.com",resources=servicemonitors,verbs=create;get ce9a447 controllers/updateservice_controller.go (Jack Ottofaro 2020-10-04 11:10:37 -0400 140) // +kubebuilder:rbac:groups="apps",resourceNames=updateservice-operator,resources=deployments/finalizers,verbs=update d9f361a controllers/cincinnati_controller.go (Jack Ottofaro 2020-09-30 16:08:24 -0400 141) // +kubebuilder:rbac:groups="policy",resources=poddisruptionbudgets,verbs=create;delete;get;list;patch;update;watch ce9a447 controllers/updateservice_controller.go (Jack Ottofaro 2020-10-04 11:10:37 -0400 142) // +kubebuilder:rbac:groups=updateservice.operator.openshift.io,resources=*,verbs=create;delete;get;list;patch;update;watch But a bare SDK 1.0.1 scaffold contains no namespace properties in those kubebuilder fields: $ operator-sdk-v1.0.1 init --domain openshift.io --repo github.com/openshift/cincinnati-operator $ operator-sdk-v1.0.1 create api --group updateservice.operator --version v1 --kind UpdateService --resource --controller $ grep -r kubebuilder:rbac controllers controllers/updateservice_controller.go:// +kubebuilder:rbac:groups=updateservice.operator.openshift.io,resources=updateservices,verbs=get;list;watch;create;update;patch;delete controllers/updateservice_controller.go:// +kubebuilder:rbac:groups=updateservice.operator.openshift.io,resources=updateservices/status,verbs=get;update;patch The SDK scaffolding also tells us how to consume the kubebuilder declarations: $ grep CRD_OPTIONS Makefile CRD_OPTIONS ?= "crd:trivialVersions=true" $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases This commit consolidates around that pattern with: $ emacs controllers/updateservice_controller.go # kubebuilder:rbac, drop namepace, sort alphabetically, remove duplicate lines $ controller-gen crd:trivialVersions=true rbac:roleName=updateservice-operator webhook paths="./..." output:crd:artifacts:config=config/crd/bases $ git rm config/crd/bases/updateservice.operator.openshift.io_updateservices_crd.yaml # remove the old name; I'm not sure how to generate with this name $ sed -i 's/updateservice.operator.openshift.io_updateservices_crd.yaml/updateservice.operator.openshift.io_updateservices.yaml/' $(git grep -l updateservice.operator.openshift.io_updateservices_crd.yaml) $ git add config controllers hack using: $ controller-gen --version Version: v0.3.0 The rbac:roleName=updateservice-operator argument passed to controller-gen needs to match: $ git --no-pager blame config/manager/manager.yaml | grep serviceAccountName ce9a447 config/manager/manager.yaml (Jack Ottofaro 2020-10-04 11:10:37 -0400 15) serviceAccountName: updateservice-operator to avoid: Failed to get Pod{...} is forbidden: User "system:serviceaccount:openshift-updateservice:updateservice-operator" cannot get resource "pods" in API group "" in the namespace "openshift-updateservice": RBAC: [clusterrole.rbac.authorization.k8s.io "updateservice-operator" not found, role.rbac.authorization.k8s.io "updateservice-operator" not found

So users don't have to figure out the "${NAME}-policy-engine-route" route naming on their own. Generated with: $ emacs api controllers docs functests # manual changes $ go get github.com/openshift/library-go@093ad3cf66000cb994f8c8010da43a71ba147671 go: github.com/openshift/library-go 093ad3cf66000cb994f8c8010da43a71ba147671 => v0.0.0-20201109112824-093ad3cf6600 $ go mod tidy $ go mod vendor $ controller-gen crd:trivialVersions=true rbac:roleName=updateservice-operator webhook paths="./..." output:crd:artifacts:config=config/crd/bases $ git add -A api config controllers docs functests go.* vendor using: $ go version go version go1.15.2 linux/amd64 $ controller-gen --version Version: v0.3.0 The controller-gen command from the previous commit.

jottofar · 2020-11-16T21:07:30Z

/lgtm

wking · 2020-11-16T22:12:24Z

/hold cancel

wking · 2020-11-16T22:14:04Z

/hold

I'll let #85 pull this in, to minimize rebasing/retesting.

Mixing in some precedent from logging/, the Update Service blog post [1], the GitHub docs [2], and some more recent operator CRD changes like [3,4]. Kathryn didn't want us asking the user to poll [5], so I'm using POSIX-shell 'while' loops to poll on the user's behalf. [1]: https://www.openshift.com/blog/openshift-update-service-update-manager-for-your-cluster [2]: https://github.com/openshift/cincinnati-operator/blob/2df239a8486d2ba3aa0d9925e5d505105ab36afe/docs/disconnected-cincinnati-operator.md [3]: openshift/cincinnati-operator#66 [4]: openshift/cincinnati-operator#85 [5]: openshift#26219 (comment)

channels/stable-4.2: 4.2.19 to stable

openshift-ci-robot requested review from djzager and jottofar October 5, 2020 20:19

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

wking force-pushed the status-route-uri branch 3 times, most recently from 9580a57 to 77e3548 Compare October 5, 2020 20:23

wking mentioned this pull request Oct 5, 2020

pkg/route: Add a package with route helpers openshift/library-go#911

Merged

wking force-pushed the status-route-uri branch 2 times, most recently from 16b111a to b60214c Compare October 5, 2020 21:49

wking mentioned this pull request Oct 19, 2020

functests: Drop Ginkgo dependency #69

Merged

wking force-pushed the status-route-uri branch 2 times, most recently from b586385 to 7f99c51 Compare October 27, 2020 20:40

wking force-pushed the status-route-uri branch from f4874c8 to 9767242 Compare October 28, 2020 05:59

LalatenduMohanty reviewed Oct 28, 2020

View reviewed changes

functests/cincinnati_creation_test.go Show resolved Hide resolved

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

LalatenduMohanty suggested changes Oct 29, 2020

View reviewed changes

openshift-ci-robot assigned LalatenduMohanty Oct 29, 2020

wking force-pushed the status-route-uri branch from 9767242 to 80000a1 Compare November 13, 2020 00:28

wking changed the title ~~pkg/apis/cincinnati/v1beta1: Add policyEngineURI~~ apis/v1: Add policyEngineURI Nov 13, 2020

wking force-pushed the status-route-uri branch from 80000a1 to 519d02e Compare November 13, 2020 03:46

wking force-pushed the status-route-uri branch from 519d02e to b1a5801 Compare November 13, 2020 05:34

wking force-pushed the status-route-uri branch from b1a5801 to bc12702 Compare November 13, 2020 05:39

openshift-ci-robot assigned jottofar Nov 16, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2020

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

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

openshift-merge-robot merged commit bc12702 into openshift:master Nov 16, 2020

wking deleted the status-route-uri branch November 16, 2020 22:28

wking mentioned this pull request Nov 16, 2020

*: Remove executable bit from non-executable files #84

Merged

PratikMahajan pushed a commit to PratikMahajan/cincinnati-operator that referenced this pull request Mar 17, 2021

Merge pull request openshift#66 from thiagoalessio/stable-4.2.19

a0c2dce

channels/stable-4.2: 4.2.19 to stable

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

apis/v1: Add policyEngineURI #66

apis/v1: Add policyEngineURI #66

Uh oh!

wking commented Oct 5, 2020

Uh oh!

openshift-ci-robot commented Oct 5, 2020

Uh oh!

openshift-ci-robot commented Oct 5, 2020

Uh oh!

wking commented Oct 5, 2020

Uh oh!

wking commented Oct 27, 2020

Uh oh!

Uh oh!

jottofar commented Oct 28, 2020

Uh oh!

LalatenduMohanty left a comment

Uh oh!

jottofar commented Oct 30, 2020

Uh oh!

wking commented Nov 13, 2020

Uh oh!

jottofar commented Nov 16, 2020

Uh oh!

wking commented Nov 16, 2020

Uh oh!

wking commented Nov 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

apis/v1: Add policyEngineURI #66

apis/v1: Add policyEngineURI #66

Uh oh!

Conversation

wking commented Oct 5, 2020

Uh oh!

openshift-ci-robot commented Oct 5, 2020

Uh oh!

openshift-ci-robot commented Oct 5, 2020

Uh oh!

wking commented Oct 5, 2020

Uh oh!

wking commented Oct 27, 2020

Uh oh!

Uh oh!

jottofar commented Oct 28, 2020

Uh oh!

LalatenduMohanty left a comment

Choose a reason for hiding this comment

Uh oh!

jottofar commented Oct 30, 2020

Uh oh!

wking commented Nov 13, 2020

Uh oh!

jottofar commented Nov 16, 2020

Uh oh!

wking commented Nov 16, 2020

Uh oh!

wking commented Nov 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants