Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Jul 7, 2022

This is basically #646; I just want to see how presubmits work before openshift/api#1212 and later land, while the cluster-version operator doesn't know about the new capability. And #646 is a draft, so no presubmits there yet.

@openshift-ci 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 Jul 7, 2022
@openshift-ci openshift-ci bot requested review from mfojtik and tisnik July 7, 2022 16:48
@openshift-ci
Copy link

openshift-ci bot commented Jul 7, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wking
To complete the pull request process, please assign tisnik after the PR has been reviewed.
You can assign the PR to them by writing /assign @tisnik in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

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

@openshift-ci
Copy link

openshift-ci bot commented Jul 7, 2022

@wking: The following test 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/insights-operator-e2e-tests 9578320 link true /test insights-operator-e2e-tests

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
Copy link
Member Author

wking commented Jul 7, 2022

This pull request is trialing openshift/enhancements#1174. Working through the "In presubmit CI for the pull request" portion:

  1. The capability is being completely removed: checking the e2e run, no insights:

    $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_insights-operator/651/pull-ci-openshift-insights-operator-master-e2e/1545086742334279680/artifacts/e2e/gather-extra/artifacts/namespaces.json | jq -r '.items[].metadata.name' | grep -1 'openshift-i'
    openshift-host-network
    openshift-image-registry
    openshift-infra
    openshift-ingress
    openshift-ingress-canary
    openshift-ingress-operator
    openshift-kni-infra

    And CVO logs:

    $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_insights-operator/651/pull-ci-openshift-insights-operator-master-e2e/1545086742334279680/artifacts/e2e/gather-extra/artifacts/pods/openshift-cluster-version_cluster-version-operator-59877d7558-7wch6_cluster-version-operator.log | grep -o 'excluding.*unrecognized capability names.*' | sort | uniq
    excluding 0000_50_insights-operator_02-namespace.yaml group= kind=Namespace namespace= name=openshift-insights: unrecognized capability names: insights
    excluding 0000_50_insights-operator_03-clusterrole.yaml group=rbac.authorization.k8s.io kind=ClusterRoleBinding namespace= name=insights-operator-auth: unrecognized capability names: insights
    excluding 0000_50_insights-operator_03-clusterrole.yaml group=rbac.authorization.k8s.io kind=ClusterRoleBinding namespace= name=insights-operator-gather-reader: unrecognized capability names: insights
    excluding 0000_50_insights-operator_03-clusterrole.yaml group=rbac.authorization.k8s.io kind=ClusterRoleBinding namespace= name=insights-operator-gather: unrecognized capability names: insights
    ...
    $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_insights-operator/651/pull-ci-openshift-insights-operator-master-e2e/1545086742334279680/artifacts/e2e/gather-extra/artifacts/pods/openshift-cluster-version_cluster-version-operator-59877d7558-7wch6_cluster-version-operator.log | grep insights | grep -v excluding
    ...no hits...

    I dunno what all is involved in the Insights component (CRDs? Other?). Folks who do should audit the list for completeness. But the fact that the install didn't fail on "hey, I can't push $RESOURCE, because the openshift-insights namespace is missing!" or similar is a good sign.

    Also promising is the Insights-specific job failing with:

     E           HTTP response body: b'{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"namespaces \\"openshift-insights\\" not found","reason":"NotFound","details":{"name":"openshift-insights","kind":"namespaces"},"code":404}\n'
    
  2. CI suites which you expected to pass continue to pass: both the e2e run and the update run passed. While it's convenient that we don't have to make test-suite alterations to accommodate the new capability, you probably want to grow some origin smoke-test coverage, so other component teams proposing changes that would cripple Insights have their presubmits fail with "Insights is smoking!" instead of obliviously landing their changes and kicking off a post-merge "what happened to Insights?" fire-drill.

Anyhow, looks good from here to open the API pull request (which in this case is already up, openshift/api#1212), so closing this pull request and returning discussion to #646.

@wking wking closed this Jul 7, 2022
@wking wking deleted the pre-api-capability-annotations branch July 7, 2022 20:27
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 7, 2022
@openshift-ci
Copy link

openshift-ci bot commented Jul 7, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant