Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPVE-673: Bump API to Add OLM Capability #971

Merged
merged 3 commits into from
Oct 14, 2023

Conversation

eggfoobar
Copy link
Contributor

Bump API to add OLM capability

  • bumped to 1.28
  • updated leader lock to use new LeasesResourceLock
  • updated unit tests to account for new capability

/hold

Holding until API change is merged in openshift/api#1589

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 19, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 19, 2023

@eggfoobar: This pull request references OCPVE-673 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

Bump API to add OLM capability

  • bumped to 1.28
  • updated leader lock to use new LeasesResourceLock
  • updated unit tests to account for new capability

/hold

Holding until API change is merged in openshift/api#1589

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 openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 19, 2023
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 3, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 3, 2023

@eggfoobar: This pull request references OCPVE-673 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

Bump API to add OLM capability

  • bumped to 1.28
  • updated leader lock to use new LeasesResourceLock
  • updated unit tests to account for new capability

/hold

Holding until API change is merged in openshift/api#1589

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-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 3, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 3, 2023

@eggfoobar: This pull request references OCPVE-673 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

Bump API to add OLM capability

  • bumped to 1.28
  • updated leader lock to use new LeasesResourceLock
  • updated unit tests to account for new capability

/hold

Holding until API change is merged in openshift/api#1589

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.

@eggfoobar
Copy link
Contributor Author

/unhold

API change was merged in

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 3, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 3, 2023

@eggfoobar: This pull request references OCPVE-673 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

Bump API to add OLM capability

  • bumped to 1.28
  • updated leader lock to use new LeasesResourceLock
  • updated unit tests to account for new capability

/hold

Holding until API change is merged in openshift/api#1589

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-merge-robot
Copy link
Contributor

@eggfoobar: 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-agnostic-ovn 85edb55 link true /test e2e-agnostic-ovn
ci/prow/lint 85edb55 link true /test lint
ci/prow/e2e-hypershift-conformance 85edb55 link true /test e2e-hypershift-conformance
ci/prow/e2e-agnostic-ovn-upgrade-out-of-change 85edb55 link true /test e2e-agnostic-ovn-upgrade-out-of-change
ci/prow/images 85edb55 link true /test images
ci/prow/gofmt 85edb55 link true /test gofmt
ci/prow/e2e-hypershift 85edb55 link true /test e2e-hypershift
ci/prow/e2e-agnostic-operator 85edb55 link true /test e2e-agnostic-operator
ci/prow/e2e-agnostic-ovn-upgrade-into-change 85edb55 link true /test e2e-agnostic-ovn-upgrade-into-change
ci/prow/unit 85edb55 link true /test unit

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.

@eggfoobar eggfoobar changed the title OCPVE-673: Add capability olm OCPVE-673: Bump API to Add OLM Capability Oct 3, 2023
@eggfoobar eggfoobar force-pushed the add-capability-olm branch 2 times, most recently from 06d0a8d to 9e7be2f Compare October 3, 2023 16:48
@eggfoobar
Copy link
Contributor Author

eggfoobar commented Oct 3, 2023

/assign @petr-muller

This should be good now for a review

@eggfoobar
Copy link
Contributor Author

/retest-required

@eggfoobar
Copy link
Contributor Author

/hold for validation

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 4, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 4, 2023

@eggfoobar: This pull request references OCPVE-673 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

Bump API to add OLM capability

  • bumped to 1.28
  • updated leader lock to use new LeasesResourceLock
  • updated unit tests to account for new capability

/hold

Holding until API change is merged in openshift/api#1589

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.

@eggfoobar
Copy link
Contributor Author

/unhold

The addition in CVO was validated to work with ClusterBot, after we had resolved some image-registry errors, the pipeline now looks better. However, because of the addition of Build as optional to the API yesterday openshift/api#1618 some other tests started breaking, but regarding the purpose of this PR the OLM
tests are correctly being skipped or handled.

I'll unblock openshift/origin#28302 since the CI for no-cap runs weekly, that change should get in before the CI runs.

Merging this PR, will unblock:
openshift/operator-framework-olm#565
openshift/cluster-monitoring-operator#2105

Ran: test e2e openshift/operator-framework-olm#565,openshift/cluster-monitoring-operator#2105,openshift/origin#28302,openshift/cluster-version-operator#971 aws,no-capabilities

https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/release-openshift-origin-installer-launch-aws-modern/1712923427997749248

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 13, 2023
Signed-off-by: ehila <[email protected]>
removed deprecated ConfigMapsLeasesResourceLock to new LeasesResourceLock

Signed-off-by: ehila <[email protected]>
@bparees
Copy link
Contributor

bparees commented Oct 13, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2023
@wking
Copy link
Member

wking commented Oct 14, 2023

Rolling back capability addition from the ClusterVersion CRD is known to break the ability of the CVO to write the no-longer-recognized capability string to status.

/override ci/prow/e2e-agnostic-ovn-upgrade-out-of-change

ClusterBot testing looks good and openshift/origin#28302 has merged.

/lgtm
/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 14, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, eggfoobar, wking

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:

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
Contributor

openshift-ci bot commented Oct 14, 2023

@wking: Overrode contexts on behalf of wking: ci/prow/e2e-agnostic-ovn-upgrade-out-of-change

In response to this:

Rolling back capability addition from the ClusterVersion CRD is known to break the ability of the CVO to write the no-longer-recognized capability string to status.

/override ci/prow/e2e-agnostic-ovn-upgrade-out-of-change

ClusterBot testing looks good and openshift/origin#28302 has merged.

/lgtm
/approve

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 14, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 14, 2023

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

@bparees
Copy link
Contributor

bparees commented Oct 14, 2023

Rolling back capability addition from the ClusterVersion CRD is known to break the ability of the CVO to write the no-longer-recognized capability string to status.

@wking the next time(if there is a next time) we run into that i think we will drop the status validation, even if only temporarily, wdyt?

@openshift-ci openshift-ci bot merged commit 80e7bbd into openshift:master Oct 14, 2023
@wking
Copy link
Member

wking commented Oct 14, 2023

what does "if only temporarily" mean? Do we bump the API repo, vendor bump here, and try and remember to vendor-bump again with enum re-tightening once we get past branch-fork? Or can we just drop the status enums altogether?

@bparees
Copy link
Contributor

bparees commented Oct 14, 2023

what does "if only temporarily" mean? Do we bump the API repo, vendor bump here, and try and remember to vendor-bump again with enum re-tightening once we get past branch-fork

we wouldn't need to get past branch fork, just until we have a nightly that doesn't include the reverted cap name so upgrades don't fail, right?

but yes, more or less that's what temporarily would mean. and with "if only temporarily" i was trying to convey "at that point we'd decide if we wanted to only do it temporarily, or convince ourselves to drop them permanently"

@wking
Copy link
Member

wking commented Oct 16, 2023

I've opened openshift/api#1624 floating "drop the enum from status.capabilities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants