Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Jul 7, 2022

The outgoing order might result in:

  1. There is an existing cluster capability that folks want to make conditional.
  2. Bump openshift/api to add newCap, and include it in vCurrent.
  3. Bump the CVO vendoring.
  4. New preview release is cut, say 4.12.0-fc.1.
  5. User installs 4.12.0-fc.1 with None.
  6. ClusterVersion claims newCap is disabled, but the cluster still includes all the resources for the associated capability. User is confused.
  7. Manifest annotations pull request opened.
  8. Presubmits pass on the annotation pull request, because we only run vCurrent presubmits.
  9. Manifest annotations land.
  10. no-cap CI starts failing, because we forgot to annotate some manifest (e.g. a custom resource did not get annotated, but the backing CustomResourceDefinition did).
  11. Additional fixup pull requests while folks sort out annotations and test-suite adjustments to get no-cap CI passing again. This could take a while.
  12. The usual post-merge drama like "do we revert or try to roll forward?". Additional pre-releases cut. More confused users asking why None installs are failing, etc.
  13. We get everything straightened out.
  14. Life goes on.

With the new flow, the in-flight testing of annotation changes establishes that:

  • Clusters can still install without those manifests.
  • Test suites covered by presubmits (or cluster-bot testing) are compatible with clusters that lack the new capability.

And all the drama and shifting needed to move from the initial guess at an implementation to an implementation that actually works in those cases is happening pre-merge, where it only impacts folks who are actively working on the annotations and test-suite changes.

CC @bparees

@wking wking force-pushed the pre-api-capabability-annotations branch from 4632352 to 40652c8 Compare July 7, 2022 20:13
@openshift-ci openshift-ci bot requested review from joelanford and russellb July 7, 2022 20:15
@wking
Copy link
Member Author

wking commented Jul 7, 2022

Here is an example of me working the checklist in premerge testing for insights.

…presubmits

The outgoing order might result in:

1. There is an existing cluster capability that folks want to make
  conditional.
2. Bump openshift/api to add newCap, and include it in vCurrent.
3. Bump the CVO vendoring.
4. New preview release is cut, say 4.12.0-fc.1.
5. User installs 4.12.0-fc.1 with None.
6. ClusterVersion claims newCap is disabled, but the cluster still
  includes all the resources for the associated capability.  User is
  confused.
7. Manifest annotations pull request opened.
8. Presubmits pass on the annotation pull request, because we only run
  vCurrent presubmits.
9. Manifest annotations land.
10. no-cap CI starts failing, because we forgot to annotate some
  manifest (e.g. a custom resource did not get annotated, but the
  backing CustomResourceDefinition did).
11. Additional fixup pull requests while folks sort out annotations
  and test-suite adjustments to get no-cap CI passing again.  This
  could take a while.
12. The usual post-merge drama like "do we revert or try to roll
  forward?".  Additional pre-releases cut.  More confused users asking
  why 'None' installs are failing, etc.
13. We get everything straightened out.
14. Life goes on.

With the new flow, the in-flight testing of annotation changes
establishes that:

* Clusters can still install without those manifests.
* Test suites covered by presubmits (or cluster-bot testing) are
  compatible with clusters that lack the new capability.

And all the drama and shifting needed to move from the initial guess
at an implementation to an implementation that actually works in those
cases is happening pre-merge, where it only impacts folks who are
actively working on the annotations and test-suite changes.
@wking wking force-pushed the pre-api-capabability-annotations branch from 40652c8 to 03ed3ad Compare July 7, 2022 20:33
@bparees
Copy link
Contributor

bparees commented Jul 7, 2022

/lgtm

@bparees
Copy link
Contributor

bparees commented Jul 7, 2022

/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 7, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 7, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees

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:

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

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

openshift-ci bot commented Jul 7, 2022

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

@openshift-ci openshift-ci bot merged commit 9be1ebf into openshift:master Jul 7, 2022
@wking wking deleted the pre-api-capabability-annotations branch July 7, 2022 21:35
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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants