CONSOLE-3220: Add console capability to all manifests #665

raspbeep · 2022-07-26T08:47:36Z

https://issues.redhat.com/browse/CONSOLE-3220

Signed-off-by: Pavel Kratochvil [email protected]

openshift-ci · 2022-07-26T08:51:47Z

Hi @raspbeep. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

jhadvig · 2022-07-26T11:06:31Z

/ok-to-test

jhadvig

@raspbeep thanks for the PR! We also need to add the annotation manifests in the bindata/ and quickstarts/ folders.

raspbeep · 2022-07-26T14:22:11Z

/retest

spadgett

It looks like we're leaving the console CRDs (ConsoleLink, ConsolePlugin, etc) even if console is removed. Is that right?

We plan to write a conversion webhook for the ConsolePlugin CRD in the console operator, so we won't be able to have the CRD without the operator. We need to decide if

We want to remove the console CRDs or
We leave the both the console CRDs and operator (for the webhook) even if console is removed or
We create a separate pod just for the webhook.

@jhadvig @bparees Opinion? I'm inclined to say we remove the CRDs, but any operators that relies on them would need to tolerate it when they're missing.

bparees · 2022-07-26T15:39:21Z

@jhadvig @bparees Opinion? I'm inclined to say we remove the CRDs, but any operators that relies on them would need to tolerate it when they're missing.

i'm also inclined to remove them, i'm not sure what background/motivation lead to the decision to always enable them. Presumably it was to avoid breaking consumers of those apis, as @spadgett said, but we should be moving them towards tolerating the absence of these apis, as we make the console in general an optional component.

jhadvig · 2022-07-27T15:38:01Z

It looks like we're leaving the console CRDs (ConsoleLink, ConsolePlugin, etc) even if console is removed. Is that right?

@spadgett @bparees So if the console-operator wont part of the cluster, CVO wont be pulling all it's manifests (together with Console's CRDs) from the image and creating them. For that reason the CRD's should not be created on the cluster if console-operator is disabled. Or is my understanding wrong?

spadgett · 2022-07-27T15:46:20Z

@jhadvig My understanding is the CVO will still install any manifests that don't have the capability annotation. Since we copy the CRD manifests from openshift/api in our Dockerfile, those would still get installed since they don't have the annotation. (Presumably there is a way to add the annotation to the generated CRD from the api repo?)

@bparees Can you confirm?

bparees · 2022-07-27T16:15:00Z

My understanding is the CVO will still install any manifests that don't have the capability annotation. Since we copy the CRD manifests from openshift/api in our Dockerfile, those would still get installed since they don't have the annotation.

correct. What matters is the annotations on the resource yamls that go into the payload.

(Presumably there is a way to add the annotation to the generated CRD from the api repo?)

i think you can just add it manually and generation doesn't touch it.

jhadvig · 2022-07-28T07:45:48Z

@raspbeep could you please update openshift/api#1232 and at the annotation to our CRDs ?

Both:

raspbeep · 2022-07-28T08:46:34Z

Thanks for the reviews. Added the annotations to CRDs in openshift/api#1232.

spadgett · 2022-07-28T15:33:32Z

bindata/configmaps/console-configmap.yaml

    app: "console"
-  annotations: {}
+  annotations:
+    capability.openshift.io/name: console


I don't believe this needs to be on anything the operator itself creates (in other words anything in bindata). Just the YAML files in manifests.

jhadvig · 2022-07-29T07:55:32Z

@spadgett you are right. Since operator is working with bindata we dont need to add the annotation to them. On the other hand we should probably add it to the quickstarts/ CRs.

spadgett · 2022-07-29T11:57:53Z

@spadgett you are right. Since operator is working with bindata we dont need to add the annotation to them. On the other hand we should probably add it to the quickstarts/ CRs.

Right, the quick start CRs will need it.

jhadvig

/lgtm
/approve

jhadvig · 2022-07-29T14:40:02Z

/hold

QE Approver:
 /assign @yanpzhan
Docs Approver:
 /assign @opayne1
PX Approver:
/assign @RickJWagner

jhadvig · 2022-08-23T09:15:58Z

/retest

manifests/0000_90_console-operator_01_prometheusrbac.yaml

wking · 2022-08-25T14:58:18Z

Hmm, I was checking in back here to report openshift/installer#5336 landing to unblock your CI. But weirdly, this run from the 23rd has:

$ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_console-operator/665/pull-ci-openshift-console-operator-master-e2e-gcp/1562005782285783040/artifacts/e2e-gcp/ipi-install-install/artifacts/.openshift_install.log | grep -A5 'openshift-console route'
time="2022-08-23T10:11:59Z" level=info msg="Waiting up to 10m0s (until 10:21AM) for the openshift-console route to be created..."
time="2022-08-23T10:11:59Z" level=debug msg="Route found in openshift-console namespace: console"
time="2022-08-23T10:11:59Z" level=debug msg="OpenShift console route is admitted"
time="2022-08-23T10:11:59Z" level=info msg="Install complete!"
time="2022-08-23T10:11:59Z" level=info msg="To access the cluster as the system:admin user when using 'oc', run\n    export KUBECONFIG=/tmp/installer/auth/kubeconfig"
time="2022-08-23T10:11:59Z" level=info msg="Access the OpenShift web-console here: https://console-openshift-console.apps.ci-op-zph4n5kx-2f88d.XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"

Is the plan to leave that route in place, even when the console is removed? Ah, it's because openshift/cluster-version-operator#801 landed:

$ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_console-operator/665/pull-ci-openshift-console-operator-master-e2e-gcp/1562005782285783040/artifacts/e2e-gcp/gather-extra/artifacts/clusterversion.json | jq -c '.items[].status.capabilities.enabledCapabilities'
["Console","Insights","Storage","baremetal","marketplace","openshift-samples"]

and that took CI on this PR from "will things work with our removal annotations?" to "meaningless" 😂. I guess we'll see post-merge ;)

jcaianirh · 2022-08-25T15:51:18Z

/test e2e-aws-console

jhadvig · 2022-08-30T07:49:44Z

/retest

ERRO[2022-08-29T23:04:00Z] Some steps failed:                           
ERRO[2022-08-29T23:04:00Z] 
  * could not run steps: step e2e-aws-console failed: "e2e-aws-console" test steps failed: "e2e-aws-console" pod "e2e-aws-console-test" failed: the pod ci-op-h8hckbiq/e2e-aws-console-test failed after 2h1m8s (failed containers: test): ContainerFailed one or more containers exited
Container test exited with code 1, reason Error
---
ng Container image with extrenal registry on Add page: KN-05-TC05 (example #1) (376617ms)
    ✓ knative service menu options: KN-02-TC01 (9737ms)
    ✓ side bar details of knative Service: KN-06-TC01 (11087ms)
    ✓ Edit labels modal details: KN-02-TC02 (10570ms)
    ✓ Edit Annotation modal details: KN-02-TC17 (9932ms)
      Verifying service kn-service is present in application openshift-app
    ✓ Update the service to new application group: KN-02-TC08 (18182ms)
    ✓ Context menu for knative Revision: KN-01-TC01 (8584ms)
    ✓ side bar details of knative Revision: KN-06-TC02 (10495ms)
    ✓ Delete Revision not possible for the service which contains one revision: KN-01-TC12 (10485ms)
      user is able to see revisions in knative service : kn-service of topology side pane
    ✓ Create Revision for the existing knative Service (17777ms)
{"component":"entrypoint","file":"k8s.io/test-infra/prow/entrypoint/run.go:165","func":"k8s.io/test-infra/prow/entrypoint.Options.ExecuteProcess","level":"error","msg":"Process did not finish before 2h0m0s timeout","severity":"error","time":"2022-08-29T22:50:01Z"}
      Displaying following error: "admission webhook "validation.webhook.serving.knative.dev" denied the request: validation failed: Traffic targets sum to 150, want 100: spec.traffic", so closing this form or modal
    ✓ Set traffic distribution greater than 100% for the Revisions of the knative Service: KN-02-TC10 (12036ms)
{"component":"entrypoint","file":"k8s.io/test-infra/prow/entrypoint/run.go:255","func":"k8s.io/test-infra/prow/entrypoint.gracefullyTerminate","level":"error","msg":"Process did not exit before 15s grace period","severity":"error","time":"2022-08-29T22:50:16Z"}
{"component":"entrypoint","error":"process timed out","file":"k8s.io/test-infra/prow/entrypoint/run.go:80","func":"k8s.io/test-infra/prow/entrypoint.Options.Run","level":"error","msg":"Error executing test process","severity":"error","time":"2022-08-29T22:50:16Z"}
error: failed to execute wrapped command: exit status 127

jhadvig · 2022-08-31T10:05:21Z

/test e2e-aws-console

jhadvig · 2022-08-31T20:14:51Z

/retest

invincibleJai · 2022-09-07T14:25:00Z

/retest

raspbeep · 2022-09-08T04:46:02Z

Thank you, @invincibleJai 🙏

jhadvig

/lgtm
/approve

openshift-ci · 2022-09-08T08:02:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhadvig, raspbeep

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

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 · 2022-09-08T12:35:33Z

/retest-required

Remaining retests: 0 against base HEAD 50c9852 and 2 for PR HEAD 0f860bb in total

jhadvig · 2022-09-08T15:06:33Z

/retest

openshift-ci-robot · 2022-09-08T18:58:19Z

/retest-required

Remaining retests: 0 against base HEAD fefaa89 and 1 for PR HEAD 0f860bb in total

jhadvig · 2022-09-09T08:15:43Z

/retest

    unmanaged_test.go:12: waiting for setup to reach settled state...
    unmanaged_test.go:13: changing console operator state to 'Unmanaged'...
    util.go:28: patching Data on the console ConfigMap
    util.go:35: polling for patched Data on the console ConfigMap
    unmanaged_test.go:41: error: timed out waiting for the condition
    unmanaged_test.go:18: waiting for cleanup to reach settled state...
--- FAIL: TestEditUnmanagedConfigMap (15.90s)

looks like a flake

raspbeep · 2022-09-09T12:28:31Z

/retest

openshift-ci · 2022-09-09T15:13:50Z

@raspbeep: all tests passed!

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.

…e.url when Console is not enabled Avoid noise like [1,2]: Error from server (NotFound): namespaces "openshift-console" not found when the Console cluster capability is not enabled [3]. With this commit, the output will be: Console is not a ClusterVersion enabled capability: [] [1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/periodic-ci-openshift-release-master-ci-4.12-e2e-aws-sdn-no-capabilities/1568745812194758656 [2]: https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/logs/periodic-ci-openshift-release-master-ci-4.12-e2e-aws-sdn-no-capabilities/1568745812194758656/artifacts/e2e-aws-sdn-no-capabilities/ipi-install-install/build-log.txt [3]: openshift/console-operator#665

Since these manifests became deletion manifests in 6c6c5ce (Add release.openshift.io/delete annotation to consoleLink CRDs, 2021-07-15, openshift#565, 4.9 [2]), the only change has been 0f860bb (Add console capability to all manifests in manifests/ and quickstarts/, 2022-07-26, openshift#665). We can remove these deletion references now, because a 4.8 cluster that might have included these resources should have completed an update to 4.9 which would have removed them. And if that failed, they should have completed an update to one of the later 4.y and removed the resources. By removing the resource, we save the cluster-version operator some time checking to ensure deletion, and only expose ourselves to leaking the resources on clusters that updated from 4.8 through to 4.14 without ever having completed an update before reaching 4.14. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1980531#c8

Since these manifests became deletion manifests in 6c6c5ce (Add release.openshift.io/delete annotation to consoleLink CRDs, 2021-07-15, openshift#565, 4.9 [1]), the only change has been 0f860bb (Add console capability to all manifests in manifests/ and quickstarts/, 2022-07-26, openshift#665). We can remove these deletion references now, because a 4.8 cluster that might have included these resources should have completed an update to 4.9 which would have removed them. And if that failed, they should have completed an update to one of the later 4.y and removed the resources. By removing the resource, we save the cluster-version operator some time checking to ensure deletion, and only expose ourselves to leaking the resources on clusters that updated from 4.8 through to 4.14 without ever having completed an update before reaching 4.14. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1980531#c8

raspbeep mentioned this pull request Jul 26, 2022

CONSOLE-3221: Add console capability openshift/api#1232

Merged

openshift-ci bot requested review from jhadvig and spadgett July 26, 2022 08:51

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 26, 2022

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 26, 2022

jhadvig changed the title ~~Add console capability to all manifests~~ CONSOLE-3220: Add console capability to all manifests Jul 26, 2022

jhadvig reviewed Jul 26, 2022

View reviewed changes

spadgett reviewed Jul 26, 2022

View reviewed changes

spadgett reviewed Jul 28, 2022

View reviewed changes

raspbeep force-pushed the console-capability-manifests branch 3 times, most recently from 98593d5 to f9c0546 Compare July 29, 2022 14:13

jhadvig approved these changes Jul 29, 2022

View reviewed changes

openshift-ci bot assigned jhadvig Jul 29, 2022

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

openshift-ci bot assigned RickJWagner Jul 29, 2022

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

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2022

spadgett reviewed Aug 24, 2022

View reviewed changes

manifests/0000_90_console-operator_01_prometheusrbac.yaml Show resolved Hide resolved

jhadvig approved these changes Sep 8, 2022

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 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 Sep 8, 2022

openshift-merge-robot merged commit 22c5707 into openshift:master Sep 9, 2022

wking mentioned this pull request Sep 13, 2022

ci-operator/step-registry/ipi/install/install: Gracefully skip console.url when Console is not enabled openshift/release#32262

Closed

wking mentioned this pull request Aug 3, 2023

chore: manifests/09-console-link-*: Drop obsolete deletion manifests #784

Merged

CONSOLE-3220: Add console capability to all manifests #665

CONSOLE-3220: Add console capability to all manifests #665

Uh oh!

Conversation

raspbeep commented Jul 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jul 26, 2022

Uh oh!

jhadvig commented Jul 26, 2022

Uh oh!

jhadvig left a comment

Choose a reason for hiding this comment

Uh oh!

raspbeep commented Jul 26, 2022

Uh oh!

spadgett left a comment

Choose a reason for hiding this comment

Uh oh!

bparees commented Jul 26, 2022

Uh oh!

jhadvig commented Jul 27, 2022

Uh oh!

spadgett commented Jul 27, 2022

Uh oh!

bparees commented Jul 27, 2022

Uh oh!

jhadvig commented Jul 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

raspbeep commented Jul 28, 2022

Uh oh!

spadgett Jul 28, 2022

Choose a reason for hiding this comment

Uh oh!

jhadvig commented Jul 29, 2022

Uh oh!

spadgett commented Jul 29, 2022

Uh oh!

jhadvig left a comment

Choose a reason for hiding this comment

Uh oh!

jhadvig commented Jul 29, 2022

Uh oh!

jhadvig commented Aug 23, 2022

Uh oh!

Uh oh!

wking commented Aug 25, 2022

Uh oh!

jcaianirh commented Aug 25, 2022

Uh oh!

jhadvig commented Aug 30, 2022

Uh oh!

jhadvig commented Aug 31, 2022

Uh oh!

jhadvig commented Aug 31, 2022

Uh oh!

invincibleJai commented Sep 7, 2022

Uh oh!

raspbeep commented Sep 8, 2022

Uh oh!

jhadvig left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Sep 8, 2022

Uh oh!

openshift-ci-robot commented Sep 8, 2022

Uh oh!

jhadvig commented Sep 8, 2022

Uh oh!

openshift-ci-robot commented Sep 8, 2022

Uh oh!

jhadvig commented Sep 9, 2022

Uh oh!

raspbeep commented Sep 9, 2022

Uh oh!

openshift-ci bot commented Sep 9, 2022

Uh oh!

Reviewers

Assignees

raspbeep commented Jul 26, 2022 •

edited

Loading

jhadvig commented Jul 28, 2022 •

edited

Loading