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

Kustomize KEP cleanup #2758

Merged
merged 6 commits into from
Jun 4, 2021
Merged

Conversation

KnVerey
Copy link
Contributor

@KnVerey KnVerey commented May 19, 2021

This PR updates the KEP metadata and implementation history for several Kustomize-related KEPs that have already been implemented (alpha or beta).

Where the latest-milestone field was set to zero but the implementation has already merged, I looked up which Kubernetes release cycle it merged in. LMK if that's not the right thing to do.

I used the following heuristic for "stage": If it is behind an alpha flag, it's alpha. If it's a KRM resource or field therein, use the resource's version (i.e. beta for Kustomization and alpha for Component).

I'll make similar updates to the tracking issues. Here's what I think we should do for each after it is updated:

/cc @eddiezane @monopole @natasha41575

@k8s-ci-robot
Copy link
Contributor

@KnVerey: GitHub didn't allow me to request PR reviews from the following users: natasha41575.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

This PR updates the KEP metadata and implementation history for several Kustomize-related KEPs. In most cases they've already been implemented (alpha or beta). For the Composition KEP, it should be implementable.

Where the latest-milestone field was set to zero but the implementation has already merged, I looked up which Kubernetes release cycle it merged in. LMK if that's not the right thing to do.

I used the following heuristic for "stage": If it is behind an alpha flag, it's alpha. If it's a KRM resource or field therein, use the resource's version (i.e. beta for Kustomization and alpha for Component).

I'll make similar updates to the tracking issues. Here's what I think we should do for each after it is updated:

/cc @eddiezane @monopole @natasha41575

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.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/cli Categorizes an issue or PR as relevant to SIG CLI. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 19, 2021
@KnVerey KnVerey force-pushed the kustomize-kep-cleanup branch from dee7b10 to f53995b Compare May 19, 2021 22:18
@@ -404,6 +404,7 @@ NA - Client side only

## Implementation History

- Alpha available in Kustomize (standalone binary, not kubectl kustomize) v2.1.0+ behind the `--enable-alpha-plugins` flag. https://kubectl.docs.kubernetes.io/blog/2019/06/18/v2.1.0/#generator-and-transformer-plugins
Copy link
Contributor Author

@KnVerey KnVerey May 19, 2021

Choose a reason for hiding this comment

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

@monopole can you confirm what the exact state is? I thought we had disabled plugins in kubectl entirely, but that's not the case. It seems like we removed --enable-star and --enable-exec specifically, but not the more general --enable-alpha-plugins gate. I manually checked that container-based transformers work. Do the legacy-style exec transformers work? Was that the intent? I'd like to leave a better note about this state here.

- n/a
superseded-by:
- n/a
status: implemented
Copy link
Contributor

Choose a reason for hiding this comment

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

KEPs stay as implementable until they have graduated to stable. So this should move to implementable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(ref #2758 (comment)) This one is a separate KRM type that you can import from a Kustomization. It has its own API version, which is alpha, but the kustomize CLI doesn't require any flags to use it. The author already closed the tracking issue on this repo.

Choose a reason for hiding this comment

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

Q: how does this move from provisional to implemented without passing through implementable? provisional means that it was not targeting to a release by the sig and that it wasn't officially approved for inclusion in a release.

Copy link
Member

Choose a reason for hiding this comment

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

Q: how does this move from provisional to implemented without passing through implementable? provisional means that it was not targeting to a release by the sig and that it wasn't officially approved for inclusion in a release.

I believe consensus was reached in the sig to move forward with this, the KEP just wasn't updated.

- n/a
superseded-by:
- n/a
status: implemented
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about KEP status, applies to any that are not stable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH I find these values kinda tough to determine for Kustomize. Kustomize the CLI is stable (v4), but its main API, Kustomization, is beta, and almost all features are part of that API. To further complicate things, the API module for using Kustomize in other programs has its own version and is alpha. None of these versions are actually tied to the k/k release cycle... but periodically (very infrequently in the past, but hopefully that's changing) the version of kustomize in kubectl gets bumped. So... it's complicated, and doesn't line up super nicely with the lifecycle Enhancements was designed for. 😅

This particular feature is a field in Kustomization. It has been implemented and documented, and works without any feature gates. I'm sure it will evolve in small ways like any piece of code, but there's no major work remaining afaik. Therefore I don't see much value in holding the KEP open.

Copy link
Contributor

Choose a reason for hiding this comment

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

agree with @KnVerey, the work we intended to do with this KEP is done so I think it can be closed

@kikisdeliveryservice
Copy link
Member

kikisdeliveryservice commented May 25, 2021

Agree with @jeremyrickard 's points above.

It seems that you're leaving things in a perma-alpha/beta state. Is there a reason you won't be moving these things to GA officially?

@pwittrock
Copy link
Member

The PR updates the KEPs to be more accurate than they are tody, so I move to merge it. If it isn't 100% accurate lets follow up with whatever needs to be updated.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 4, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: KnVerey, pwittrock

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 Jun 4, 2021
@k8s-ci-robot k8s-ci-robot merged commit 56f444e into kubernetes:master Jun 4, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.22 milestone Jun 4, 2021
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. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/cli Categorizes an issue or PR as relevant to SIG CLI. 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.

6 participants