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

📖 Add additional context around experiment lifecycle #2974

Merged
merged 4 commits into from
May 8, 2020

Conversation

detiber
Copy link
Member

@detiber detiber commented Apr 28, 2020

What this PR does / why we need it:

  • Add additional context/clarification around lifecycle, admission, and promotion process for experimental types/features

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 28, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: detiber

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 28, 2020
@detiber
Copy link
Member Author

detiber commented Apr 28, 2020

/hold

holding for feedback and larger community consensus.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 28, 2020
@detiber
Copy link
Member Author

detiber commented Apr 28, 2020

This is a followup based on discussion related to #2940

CONTRIBUTING.md Outdated
- Experiments follow a strict lifecycle: Alpha -> Beta -> GA.
- Alpha-stage experiments SHOULD not be enabled by default.
- Inactive or outdated experiments will be deprecated and removed from the codebase, as outlined in Kubernetes deprecation policy
- Experiments follow a strict lifecycle: Early -> Late -> Promoted.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments from the other PR might have get lost, the library we're currently using (component-base) defines these phases as Alpha -> Beta -> GA, that's why that was written that way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I tried to take that into account with the clarifications below around feature gates.

I do think there is value in trying to differentiate our wording here to avoid user/consumer confusion over what support means for Alpha/Beta/GA for experiments compared to non-experimental use.

More than happy to make additional changes here based on feedback and general consensus, though.

Copy link
Member

@fabriziopandini fabriziopandini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@detiber thanks for this PR!
I missed the original comment, but this is definitely a step in the right direction

CONTRIBUTING.md Outdated
- Experiments follow a strict lifecycle: Alpha -> Beta -> GA.
- Alpha-stage experiments SHOULD not be enabled by default.
- Inactive or outdated experiments will be deprecated and removed from the codebase, as outlined in Kubernetes deprecation policy
- Experiments follow a strict lifecycle: Early -> Late -> Promoted.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we should stick to Alfa/Beta/GA and avoid to introduce a new classification

CONTRIBUTING.md Outdated
- Early-stage experiments:
- SHOULD not be enabled by default and any feature gates MUST be marked as 'Alpha'
- MUST be associated with a CAEP that is merged and in at least a provisional state
- MAY be considered inactive and eligible for removal if it does not graduate to Late-stage after 3 minor releases or 1 year, whichever is greater.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that for alpha experiment the deprecation cycle should be shorter (1 minor release)

CONTRIBUTING.md Outdated
- MUST be associated with a CAEP that is at least in the experimental state
- MUST support conversions for any type changes
- MUST remain backwards compatible unless updates are coinciding with a breaking Cluster API release
- MAY be considered inactive and eligible for demotion back to Early-stage if it does not graduate to Promoted after 3 minor releases or 1 year, whichever is greater.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that beta experiment should either be removed (after 2 minors deprecation period) or graduate. Given the two options, I'm not sure we must enforce a max time before graduation.

CONTRIBUTING.md Outdated
- MUST support conversions for any type changes
- MUST remain backwards compatible unless updates are coinciding with a breaking Cluster API release
- MAY be considered inactive and eligible for demotion back to Early-stage if it does not graduate to Promoted after 3 minor releases or 1 year, whichever is greater.
- Promoted experiments:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that an experiment is promoted when it becomes part of the main codebase, so IMO this state should not exist and the underlying list should be transformed into a graduation checklist

@detiber detiber changed the title 📖 Add additional context around experiment lifecycle [WIP] 📖 Add additional context around experiment lifecycle Apr 29, 2020
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 29, 2020
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 29, 2020
@detiber
Copy link
Member Author

detiber commented Apr 29, 2020

@fabriziopandini @vincepri ptal, went ahead and reverted back to Alpha/Beta and tried to be a bit more explicit around deprecation/deletion policies.

Copy link
Member

@fabriziopandini fabriziopandini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@detiber thanks, I really like this new version because it enables quick iterations and it makes clear the needs of active support around experiments
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 2, 2020
@detiber detiber changed the title [WIP] 📖 Add additional context around experiment lifecycle 📖 Add additional context around experiment lifecycle May 4, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 4, 2020
@vincepri
Copy link
Member

vincepri commented May 4, 2020

@michaelgugino did you want to take a look before we merge these guidelines for experiments?

@vincepri
Copy link
Member

vincepri commented May 5, 2020

/milestone v0.3.6

@k8s-ci-robot k8s-ci-robot added this to the v0.3.6 milestone May 5, 2020
@vincepri
Copy link
Member

vincepri commented May 8, 2020

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 8, 2020
@k8s-ci-robot k8s-ci-robot merged commit aa6acaa into kubernetes-sigs:master May 8, 2020
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants