manifests/00-ingress-credentials-request: Fill in cluster-profile annotations #692

wking · 2021-12-21T22:58:44Z

All of these platforms require manual credential creation (e.g. via ccoctl), so it doesn't really matter if the CredentialRequests get pushed into the cluster or not. But having them in the cluster will allow the Kube API-server and possibly the cloud-credential operator to complain about anything surprising they see ("that's not a valid value for that property", "I'm watching over your manual-mode shoulder, and you missed a requested secret", etc.). Regardless of whether we have that coverage today, getting these into the cluster is consistent with our other, existing CredentialsRequests.

Also tweak the namespace of the Alibaba request to match our usual openshift-cloud-credential-operator. Neither the in-cluster cloud-credential operator nor ccoctl care about cred-request namespaces; they both happily process requests in any namespace. But all of our other, existing CredentialsRequests use the cloud-cred namespace, so stick with that for consistency too.

…otations All of these platforms require manual credential creation (e.g. via ccoctl), so it doesn't really matter if the CredentialRequests get pushed into the cluster or not. But having them in the cluster will allow the Kube API-server and possibly the cloud-credential operator to complain about anything surprising they see ("that's not a valid value for that property", "I'm watching over your manual-mode shoulder, and you missed a requested secret", etc.). Regardless of whether we have that coverage today, getting these into the cluster is consistent with our other, existing CredentialsRequests. Also tweak the namespace of the Alibaba request to match our usual openshift-cloud-credential-operator. Neither the in-cluster cloud-credential operator nor ccoctl care about cred-request namespaces; they both happily process requests in any namespace. But all of our other, existing CredentialsRequests use the cloud-cred namespace, so stick with that for consistency too.

Miciah · 2021-12-21T23:03:45Z

/lgtm
/hold
@kwoodson, @csrwng, would you mind double-checking that these changes are appropriate for Alibaba and IBM Cloud?

openshift-ci · 2021-12-21T23:04:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Miciah, 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~~ [Miciah]

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

csrwng · 2021-12-21T23:07:37Z

hypershift/roks doesn't use the cloud credential operator, so the ibm-cloud-managed annotation can be left out. However, it doesn't do any harm since there's no operator to do anything with them.

wking · 2021-12-21T23:21:37Z

Waiting while we decide whether the cloud-credential operator should be installed for more profiles than just self-managed-high-availability.

manifests/00-ingress-credentials-request.yaml

$ sed -i '/ibm-cloud-managed/d' manifests/00-ingress-credentials-request.yaml From Cesar [1]: hypershift/roks doesn't use the cloud credential operator, so the ibm-cloud-managed annotation can be left out. However, it doesn't do any harm since there's no operator to do anything with them. Dropping the cluster-profile from these resources should reduce confusion by clearing unused cruft out of the IBM clusters. [1]: openshift#692 (comment)

openshift-ci · 2021-12-22T07:08:56Z

New changes are detected. LGTM label has been removed.

wking · 2021-12-22T07:10:48Z

ok, fbcf019 -> ef47357 drops ibm-cloud-managed from the CredentialsRequests (following the above comments), and also drops single-node-developer across the board (it isn't used, see openshift/cluster-version-operator#685).

There's an enhancement proposal for this profile [1], and the Code Ready Containers folks took a run at using it in [2] before backing off in [3]. I don't have any problems with having a specific CRC profile, but until we end up going that way, save ourselves the mental overhead of trying to guess whether it will want each of our manifest resources by dropping the annotation across the board. Effectively reverts abd8a20 (Annotate manifests for single-node-developer cluster profile, 2020-11-27, openshift#498). Generated with: $ sed -i '/single-node-developer/d' manifests/*.yaml $ git checkout HEAD -- manifests/00-custom-resource-definition* where I'm leaving the CRDs alone to avoid things like [4]: hack/verify-generated-crd.sh --- vendor/github.com/openshift/api/operator/v1/0000_50_ingress-operator_00-ingresscontroller.crd.yaml 2021-12-22 07:10:24.000000000 +0000 +++ manifests/00-custom-resource-definition.yaml 2021-12-22 07:10:25.000000000 +0000 @@ -5,7 +5,6 @@ metadata: api-approved.openshift.io: openshift/api#616 include.release.openshift.io/ibm-cloud-managed: "true" include.release.openshift.io/self-managed-high-availability: "true" - include.release.openshift.io/single-node-developer: "true" name: ingresscontrollers.operator.openshift.io spec: group: operator.openshift.io invalid CRD: vendor/github.com/openshift/api/operator/v1/0000_50_ingress-operator_00-ingresscontroller.crd.yaml => manifests/00-custom-resource-definition.yaml [1]: https://github.com/openshift/enhancements/blob/2911c46bf7d2f22eb1ab81739b4f9c2603fd0c07/enhancements/single-node/developer-cluster-profile.md [2]: crc-org/snc#338 [3]: crc-org/snc#373 (comment) [4]: https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_cluster-ingress-operator/692/pull-ci-openshift-cluster-ingress-operator-master-verify/1473551168843026432#1:build-log.txt%3A14

Generated with: $ update-generated-bindata.sh

openshift-ci · 2021-12-22T09:56:02Z

@wking: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-single-node	`7e66b3b`	link	false	`/test e2e-aws-single-node`
ci/prow/e2e-aws-operator	`7e66b3b`	link	true	`/test e2e-aws-operator`
ci/prow/e2e-gcp-serial	`7e66b3b`	link	true	`/test e2e-gcp-serial`

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 · 2021-12-22T18:09:51Z

manifests/00-ingress-credentials-request.yaml

+  namespace: openshift-cloud-credential-operator
+  annotations:
+    include.release.openshift.io/ibm-cloud-managed: "true"
+    include.release.openshift.io/self-managed-high-availability: "true"


Talking this over with @joelddiaz , we probably want to keep these out of this profile, to make it more clear to cluster admins that the cloud-cred operator will completely ignore CredentialsRequests on these platforms, regardless of whether it's configured for manual or mint or whatever mode.

this does imply we should do this for ALL CredentialsRequests for platforms that are only supported in "Manual" mode...

openshift-bot · 2022-03-22T23:29:37Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-ci · 2022-03-22T23:29:45Z

@wking: PR needs rebase.

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.

openshift-bot · 2022-04-21T23:58:14Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

openshift-bot · 2022-05-22T00:21:44Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2022-05-22T00:22:13Z

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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.

openshift-ci bot requested review from miheer and rfredette December 21, 2021 22:59

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 21, 2021

openshift-ci bot assigned Miciah Dec 21, 2021

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

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

kwoodson reviewed Dec 22, 2021

View reviewed changes

manifests/00-ingress-credentials-request.yaml Outdated Show resolved Hide resolved

wking mentioned this pull request Dec 22, 2021

pkg/payload: Log manifest exclusion openshift/cluster-version-operator#712

Merged

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 22, 2021

wking force-pushed the manifest-cluster-profile-annotations branch from ef47357 to 3155096 Compare December 22, 2021 07:19

wking force-pushed the manifest-cluster-profile-annotations branch from 3155096 to 0dd22b3 Compare December 22, 2021 07:43

pkg/manifests: Update bindata

7e66b3b

Generated with: $ update-generated-bindata.sh

wking commented Dec 22, 2021

View reviewed changes

openshift-ci bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 22, 2022

openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 21, 2022

openshift-ci bot closed this May 22, 2022

manifests/00-ingress-credentials-request: Fill in cluster-profile annotations #692

manifests/00-ingress-credentials-request: Fill in cluster-profile annotations #692

Uh oh!

Conversation

wking commented Dec 21, 2021

Uh oh!

Miciah commented Dec 21, 2021

Uh oh!

openshift-ci bot commented Dec 21, 2021

Uh oh!

csrwng commented Dec 21, 2021

Uh oh!

wking commented Dec 21, 2021

Uh oh!

Uh oh!

openshift-ci bot commented Dec 22, 2021

Uh oh!

wking commented Dec 22, 2021

Uh oh!

openshift-ci bot commented Dec 22, 2021

Uh oh!

wking Dec 22, 2021

Choose a reason for hiding this comment

Uh oh!

joelddiaz Dec 22, 2021

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Mar 22, 2022

Uh oh!

openshift-ci bot commented Mar 22, 2022

Uh oh!

openshift-bot commented Apr 21, 2022

Uh oh!

openshift-bot commented May 22, 2022

Uh oh!

openshift-ci bot commented May 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants