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

Decoupling API versioning and Feature versioning for features turned on by default #6592

Closed
1 of 2 tasks
Tracked by #6966
JeromeJu opened this issue Apr 27, 2023 · 14 comments · Fixed by #6941
Closed
1 of 2 tasks
Tracked by #6966

Decoupling API versioning and Feature versioning for features turned on by default #6592

JeromeJu opened this issue Apr 27, 2023 · 14 comments · Fixed by #6941
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@JeromeJu
Copy link
Member

JeromeJu commented Apr 27, 2023

This issue tracks progress on the API/feature versioning decoupling as the existing v1 migration blocker. It aims to initiate the discussion on the impact of this towards stakeholders and reach consensus on the approach moving forward.

Context

Currently, the behavior of enable-api-fields depends on the CRD API version being used. In v1beta1 CRDs, beta features can be enabled by setting enable-api-fields to beta or to stable, but in v1 CRDs, beta features can only be enabled by setting enable-api-fields to beta. The coupling of API versioning and feature versioning has been a blocker to the v1 migration.

As discussed in the API WG, we would want to decouple API versioning and feature versioning for features
that are turned on by default. If a feature flag was enabled by default but not enabled later on, it would
be considered a breaking change. And it could cause confusion for end users when we are moving to future
stable versions with unstable features. An approach needs to be carried out for avoiding such breaking
changes.

Use case

i.e. resolver is turned on by default in v1beta1 but cannot be migrated to v1 directly since it is not a beta feature in the v1 API, which could potentially cause breaking changes for users.

Proposed Options

There are some ways that we could use for moving forward:

  • Adopting per feature flag
  • Promote features to stable in stable versions also

See decouple API and feature versioning problem statement and adopting per-feature flag proposal for more details.

related: #6579

@JeromeJu JeromeJu added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 27, 2023
@vdemeester
Copy link
Member

I am not sure I follow entirely 🤔 especially as there is not really such thing as feature versionning. The only "version" we have are the API version and the "code/release" version.

Reading the use case, my understanding is that, at least in the example, you can have something that is "usable" in both API versions, but by default on one (v1beta1) but not on the other (v1) ? If that's the case, then, this goes back to the choice of the enable-api-fields values (alpha, beta and stable) and there "enablement" between different API version.

I do still believe, aside from naming, the current behavior is indeed a bit confusing. The feature flag should be independent from the API version used. A feature is experimental or stable (or in between if we consider alpha->beta->stable flow), no matter what the API version is. Enabling by default the resolvers on v1beta1 but not on v1 is indeed a bit confusing. If we enabled it by default on any API version we publish, this means we feel it's stable enough to be enabled by default, imo.

@dibyom
Copy link
Member

dibyom commented Apr 28, 2023

Reading the use case, my understanding is that, at least in the example, you can have something that is "usable" in both API versions, but by default on one (v1beta1) but not on the other (v1) ? If that's the case, then, this goes back to the choice of the enable-api-fields values (alpha, beta and stable) and there "enablement" between different API version.

You are correct!

I do still believe, aside from naming, the current behavior is indeed a bit confusing. The feature flag should be independent from the API version used. A feature is experimental or stable (or in between if we consider alpha->beta->stable flow), no matter what the API version is. Enabling by default the resolvers on v1beta1 but not on v1 is indeed a bit confusing. If we enabled it by default on any API version we publish, this means we feel it's stable enough to be enabled by default, imo.

Agreed. For features that are currently in beta, we have a couple of options when we move to v1 as the storage version

  1. we do nothing - this would mean features that are on by default today are not on by default anymore - this is poor UX and a breaking change for the user

  2. we enable (some or all) beta features on by default - basically we change the first requirement here. Note that the rest of the beta features policy still apply i.e. we can make breaking changes with the proper notice.

  3. we beta features such as resolvers to GA first - IMO, we should not rush pushing features to GA just to be conformant with our current feature gates implementation.

To me, 2. seems like the least bad option right now. Open to other ideas and feedback!

/cc @lbernick

@lbernick
Copy link
Member

I'm confused what it would mean for resolvers to be both on by default (i.e. on even when enable-api-fields is set to stable) but still be a beta feature. Does this mean it would follow the beta stability policy (can be changed w/ 9months warning) even though it is a "stable" api field?

It seems like to satisfy both requirements we'd need to make the default value of enable-api-fields "beta", but that's not backwards compatible.

@dibyom
Copy link
Member

dibyom commented May 1, 2023

You are right, the naming of the flags makes this confusing. The intention would be to decouple the stability level of the feature from whether it is enabled by default in a particular API version or not.

@dibyom
Copy link
Member

dibyom commented May 2, 2023

An example to illustrate this issue:

In #5938 we switch the reconcilers to use v1 types. Now, this should be not be a breaking change for the user at all but what we see is that some existing v1beta1 pipelineruns are now failing e.g. test: TestYamls/yamls/v1beta1/pipelineruns/pipelinerun-with-final-tasks.yaml is failing validation with

            message: 'Pipeline arendelle-xxmg8/clone-cleanup-workspace can''t be Run; it has
              an invalid spec: must not set the field(s): tasks[0].taskref.params, tasks[0].taskref.resolver' 

(see full logs)

@lbernick lbernick added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label May 2, 2023
@lbernick lbernick moved this to Todo in Pipelines V1 May 3, 2023
@lbernick
Copy link
Member

lbernick commented May 3, 2023

I think the issue Dibyo is pointing out here is a separate but related issue; I've created #6616 to track this.

@vdemeester
Copy link
Member

I'm confused what it would mean for resolvers to be both on by default (i.e. on even when enable-api-fields is set to stable) but still be a beta feature. Does this mean it would follow the beta stability policy (can be changed w/ 9months warning) even though it is a "stable" api field?

Let's assume instead of "stable, beta and alpha", the value for enable-api-fields would be "one,two,three". If we consider resolvers to be behind "two", they would be enabled only when enable-api-fields is set to "two", no matter what in what API — aka you could only use resolvers, in v1beta1 or in v1 if the value is "two", or "one".

The "beta" status of the API is different from the "beta" status of a given feature.

A very good example of how confusing this behavior is :

  • Today, I am using v1beta1, and I am using resolvers (let's say the git and bundle resolvers).
  • v1 is available, so I want to migrate to v1 (because why not 👼🏼)
  • I migrate all my object to v1, still using resolvers.
  • I try to apply them, and they get rejected because well, it's not allowed (as resolvers are not enabled by default on v1)

This is so confusing and counter-intuitive for users.

It seems like to satisfy both requirements we'd need to make the default value of enable-api-fields "beta", but that's not backwards compatible.

What wouldn't be backward compatible ? We are in a situation where the only "way forward to be backward compatible" (what a weird sentence 😂) is to mark resolvers as stable, as well as everything that is behind enable-api-fields: beta today before changing the meaning of enable-api-fields...
And I really think this is something we need to do 😅

I also think we need to rethink the enable-api-fields flag and figure out if it's not bringing more issues than having per-feature feature-flags. It doesn't matter if it's more work for us (developer/maintainer of tektoncd/pipeline), it has to be useful, consistent and easy for the end-users.

@dibyom
Copy link
Member

dibyom commented May 3, 2023

I also think we need to rethink the enable-api-fields flag and figure out if it's not bringing more issues than having per-feature feature-flags. It doesn't matter if it's more work for us (developer/maintainer of tektoncd/pipeline), it has to be useful, consistent and easy for the end-users.

+1. @jerop and I were just discussing the Kuberentes style feature flag support where each feature has a flag, and the stability level is tracked in code in a struct (vs via docs like we have today)

@vdemeester making sure I understand what you are saying - are you saying the only way forward is moving resolvers and current beta features to "stable" or can we set enable-api-fields to "beta" once we move the reconciler and storage versions to v1 (to match the current reconciler behavior)?

@vdemeester
Copy link
Member

@vdemeester making sure I understand what you are saying - are you saying the only way forward is moving resolvers and current beta features to "stable" or can we set enable-api-fields to "beta" once we move the reconciler and storage versions to v1 (to match the current reconciler behavior)?

Yes, that's what I am saying from "on top of my head" 😝

@lbernick
Copy link
Member

lbernick commented May 4, 2023

@vdemeester and I chatted on slack, we think:

@tektoncd/core-maintainers I'd like to get consensus on a short-term solution that will help us move forward with moving to a v1 api. Please comment here re: whether changing "enable-api-fields" to "beta" seems like an OK short term strategy, or whether you have concerns or alternative suggestions. (Feel free to also message me directly.)

@pierretasci
Copy link

+1 to everything @lbernick said. My 2 cents, as it pertains to resolvers is that avoiding breakages should be the short-term priority because it is unpleasant to users who may have specific expectations based on the current versioning constructs. The longer term fix for "beta" features and how they are exposed can be had separately.

Long-term, I do think that other than just not releasing any features until they are fully stable and then having to update the Tekton API versions very often (ie V2Beta56 😨 )... decoupling the API stability from the feature stability on a per feature basis is the most sustainable option. Ultimately though, the onus doesn't have to be exclusively on Tekton. An operator or end-user who installs Tekton no matter what needs to understand the implications of the feature, default-on or not and make a determination for themselves if it is worthwhile for their use case.

So, what if, long-term, every per-feature feature-flag is always default off? We don't ship a version of Tekton's config with anything on. A user always has to make the determination for themselves what to turn on. With each release, we can indicate the "feature stability" (resolvers is now beta meaning we have confidence in it but may still change the contract) without having to update the config. Perhaps at some point, a feature can become "absorbed" and it is now permanent functionality that is absorbed and thus is a breaking change. Just some thoughts 😄

@lbernick
Copy link
Member

lbernick commented May 8, 2023

Chatted with @skaegi, he is also fine with changing the default value to "beta" and moving to per-feature flags.

@dibyom
Copy link
Member

dibyom commented May 26, 2023

Chatted with @lbernick and @JeromeJu about this this week. My summary is:

  1. We need to default enable-api-fields to beta once we move the reconciler to use v1 types in order to make this a non-breaking change for users - we can consider setting the default to a different value (i.e. just stable as @afrittoli suggested in Versioned validation of referenced Pipelines/Tasks #6616 (comment))

  2. If an user is not using the default value of enable-api-fields and manually overriding them to stable, we will still break them (existing v1beta1 resources created with beta fields enabled will now fail validation). @lbernick has a solution for this in Validate beta features only when v1 Tasks and Pipelines are defined #6701 - the adding an annotation alternative (Use original version of enable-api-fields for validating beta features #6716 ) does not work for existing v1beta1 resources. One alternative could be to not do this and simply mention this as a breaking change to users since we expect the number of impacted by this to be small.

  3. Finally, we need to decouple our API version from feature version (as mentioned here by updating TEP-033, using per feature flags or another solution.

JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Aug 28, 2023
This commit adds the feature graduation process documentation to the
API compatibility policy. It aims to mitigate the confusions of API
versioning that was coupled with feature versioning as in tektoncd#6592.

related: TEP0138
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Aug 29, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they are validated only in v1 api behind beta
enable-api-fields but not in v1beta1. This PR removes the gap
between the validations of the stable features in v1 and v1beta1.

It also updates the api compatibility policy to clarify that feature
stability levels are independent on CRD apiVersions.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Aug 29, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they are validated only in v1 api behind beta
enable-api-fields but not in v1beta1. This PR removes the gap
between the validations of the stable features in v1 and v1beta1.

It also updates the api compatibility policy to clarify that feature
stability levels are independent on CRD apiVersions.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Aug 29, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they are validated only in v1 api behind beta
enable-api-fields but not in v1beta1. This PR removes the gap
between the validations of the stable features in v1 and v1beta1.

It also updates the api compatibility policy to clarify that feature
stability levels are independent on CRD apiVersions.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Aug 30, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Aug 30, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Aug 30, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Aug 30, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Aug 30, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Aug 31, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 25, 2023
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 5, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 5, 2023
This commit adds the feature graduation process documentation to the
API compatibility policy. It aims to mitigate the confusions of API
versioning that was coupled with feature versioning as in tektoncd#6592.

part of: tektoncd#7177
related: TEP0138

/kind documentation
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 5, 2023
This commit adds the feature graduation process documentation to the
API compatibility policy. It aims to mitigate the confusions of API
versioning that was coupled with feature versioning as in tektoncd#6592.
The graduation process clarifies the independence of feature versions
from API versions.

part of: tektoncd#7177
related: TEP0138

/kind documentation
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 5, 2023
This commit adds the feature graduation process documentation to the
API compatibility policy. It aims to mitigate the confusions of API
versioning that was coupled with feature versioning as in tektoncd#6592.
The graduation process clarifies the independence of feature versions
from API versions.

part of: tektoncd#7177
related: TEP0138

/kind documentation
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 5, 2023
This commit adds the feature graduation process documentation to the
API compatibility policy. It aims to mitigate the confusions of API
versioning that was coupled with feature versioning as in tektoncd#6592.
The graduation process clarifies the independence of feature versions
from API versions.

part of: tektoncd#7177
related: TEP0138

/kind documentation
tekton-robot pushed a commit that referenced this issue Oct 6, 2023
This commit adds the feature graduation process documentation to the
API compatibility policy. It aims to mitigate the confusions of API
versioning that was coupled with feature versioning as in #6592.
The graduation process clarifies the independence of feature versions
from API versions.

part of: #7177
related: TEP0138

/kind documentation
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 10, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 12, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 17, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 19, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 20, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 23, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 24, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
JeromeJu added a commit to JeromeJu/pipeline that referenced this issue Oct 24, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: tektoncd#6592
tekton-robot pushed a commit that referenced this issue Oct 24, 2023
This commit decouples the existing beta feature validation in v1beta1.
Prior to this change, beta features are regarded as stable in v1beta1
apiVersion and they only require enable-api-fields to be set to beta in
v1 apiVersion but not in v1beta1. This PR removes the gap between the
validations of the stable features in v1 and v1beta1 by syncing up the
validations in v1beta1.

Fixes: #6592
@github-project-automation github-project-automation bot moved this from Todo to Done in Pipelines V1 Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants