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

Remove setters code from cmd/config and kyaml #4045

Closed
phanimarupaka opened this issue Jul 8, 2021 · 19 comments · Fixed by #5291
Closed

Remove setters code from cmd/config and kyaml #4045

phanimarupaka opened this issue Jul 8, 2021 · 19 comments · Fixed by #5291
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@phanimarupaka
Copy link
Contributor

Setters functionality is provided as a KRM function. We should remove code related to setters in cmd/config and kyaml.

@phanimarupaka phanimarupaka added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 8, 2021
@k8s-ci-robot
Copy link
Contributor

@phanimarupaka: This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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 the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jul 8, 2021
@phanimarupaka
Copy link
Contributor Author

phanimarupaka commented Jul 8, 2021

@natasha41575 Let me know if you have cycles to pick this up.

@KnVerey Since cmd/config commands are alpha in kustomize, please let us know the best way to deprecate them and user communication. I am not sure if there are users for those commands apart from kpt.

@natasha41575
Copy link
Contributor

natasha41575 commented Jul 8, 2021

I have cycles to pick this up but also would like clarity on how to deprecate the commands kustomize cfg create-setter and kustomize cfg set and if and how we need to communicate this to users. Since they are alpha, can we just remove them?

Related: #3953

Discussion there agrees with removing these commands, but is under the project Kustomize CLI v5, so it seems like we should only remove them when we are ready to do a major release. @KnVerey could you clarify the timeline?

@KnVerey
Copy link
Contributor

KnVerey commented Jul 8, 2021

Since they are alpha, can we just remove them?

Great question. Any idea if there is precedent to follow here? Perhaps you're right and we can just remove them without warning in 5.0... or maybe even before then? Re: communication, a release note pointing to docs for the KRM function should suffice, I think.

it seems like we should only remove them when we are ready to do a major release. @KnVerey could you clarify the timeline?

The idea is to group together all the breaking changes we've been wanting to make into a single release, to minimize the number of times users have to engage with an update. Partly timeline depends on how we want to handle the Kustomization v1 project. Historically, it seems like breaking changes to Kustomization were made by incrementing the CLI major version instead of the Kustomization version. Since we're necessarily changing the Kustomization version with that project, I think the "right way" (higher effort) would be to have a CLI version that supports both beta and v1, and then eventually remove beta support in another major CLI version. We'll need to check with @monopole on that. The other thing CLI 5.0 timeline depends on is deprecation of flags--because those WILL need a proper deprecation cycle, given that the CLI is already GA.

@natasha41575 natasha41575 added triage/under-consideration and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 8, 2021
@natasha41575 natasha41575 added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed triage/under-consideration labels Jul 16, 2021
@natasha41575
Copy link
Contributor

The commands will be marked deprecated in the next release. We can remove the code as part of the kustomize CLI v5 release.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 20, 2021
@KnVerey
Copy link
Contributor

KnVerey commented Oct 20, 2021

/remove-lifecycle stale

all deprecations are out in the latest release

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 20, 2021
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 18, 2022
@KnVerey
Copy link
Contributor

KnVerey commented Jan 19, 2022

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 19, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 19, 2022
@KnVerey
Copy link
Contributor

KnVerey commented Apr 19, 2022

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 19, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 18, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 17, 2022
@KnVerey KnVerey added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Aug 17, 2022
@natasha41575
Copy link
Contributor

natasha41575 commented Aug 18, 2023

The setters code lives here I think: https://github.com/kubernetes-sigs/kustomize/tree/master/kyaml/setters2 and https://github.com/kubernetes-sigs/kustomize/tree/master/kyaml/fix/fixsetters.

Can someone investigate if we are still using these package anywhere? If not, we can probably just take it out entirely. I don't think we need to continue to support setters-related code.

The only other project that I know has a similar concept of setters is kpt and kpt functions, so it would be helpful if whoever investigates this can also check if they are depending on any of the setters code in kyaml as well.

@khrisrichardson
Copy link
Contributor

I see a couple examples outside of kpt:

fluxcd/image-automation-controller
rancher/fleet

@irvifa
Copy link
Member

irvifa commented Aug 23, 2023

/assign

@irvifa
Copy link
Member

irvifa commented Aug 23, 2023

@natasha41575 I've tried to search of the setters and setters2 usage and most of them related to fork of kpt, however, these:
fluxcd/image-automation-controller with kyml
rancher/fleet with kyaml

Are still using them, They pinned these two into a specific kyaml version. If we decide to go for this removal then we can make a release note that this is actually removed on the next version since we already marked this as deprecated before.

@natasha41575
Copy link
Contributor

Are still using them, They pinned these two into a specific kyaml version. If we decide to go for this removal then we can make a release note that this is actually removed on the next version since we already marked this as deprecated before.

SGTM!

@natasha41575
Copy link
Contributor

Also a note, the issue itself should stay in "in progress" - "Needs LGTM" is just for PRs.

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/frozen Indicates that an issue or PR should not be auto-closed due to staleness. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Development

Successfully merging a pull request may close this issue.

7 participants