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.

Sign up for GitHub

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

Jump to bottom

Idea: EOL "apply" in favor of more descriptive verbs #118775

Open
thockin opened this issue Jun 20, 2023 · 7 comments
Open

Idea: EOL "apply" in favor of more descriptive verbs #118775

thockin opened this issue Jun 20, 2023 · 7 comments
Labels
sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/cli Categorizes an issue or PR as relevant to SIG CLI. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@thockin
Copy link
Member

thockin commented Jun 20, 2023 •
edited
Loading

#118725 illustrates some surprising behavior with kubectl apply. I have found, repeatedly, that humans are bad at predicting what apply will do (made worse by client-side and server-side being different).

From talking to a lot of people, the apply action feels like it means create-or-replace, but it's really not. It's a much more sophisticated resource co-ownership system. If you don't keep that in mind, it will almost certainly NOT do what you expect it to do, which has led to many bugs over the years (as @apelisse can attest).

In hindsight, I wonder if "apply" was the right name for this. Perhaps we really want 2 distinct operations - something like "kubectl set" (simple create-or-replace) and "kubectl merge" (what we call "apply" today). I suspect that vast majority of issues reported with "apply" would be avoided because humans would mostly use "set", and "merge" would be much more clearly a variant of patch, left for "advanced" cases.

Saying kubectl set -f deploy.yaml intuitively means one thing, while kubectl merge -f deploy.yaml clearly means something else. I don't know if we want to extend that to edit, too.

I wonder if it is possible to steer towards a future where this is clearer. @apelisse @soltysh @KnVerey @eddiezane @jpbetz
@thockin thockin added sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. labels Jun 20, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jun 20, 2023
@apelisse
Copy link
Member

apelisse commented Jun 20, 2023 •
edited
Loading

I totally agree with a lot of the sentiment here. I have regularly people asking me why kubectl apply doesn't reset some of the fields that they haven't specified when they apply. "I don't have field XYZ in my manifest, and when I apply it, it doesn't remove field XYZ, why?" Well, because we're merging, not "replacing". But then, I ask, do you want to remove the status? probably not, do you want to remove clusterIP? well that doesn't work. So I don't know if people want "merge" or "replace", I think people just don't know what they want? I'm still not sure I know what they want either.

@thockin
Copy link
Member Author

thockin commented Jun 20, 2023

I think spec vs status is OK. But it gets way more subtle, like the service-ports merging. It's a correct result for "merge" but very surprising for "apply" and would be obviously wrong for "set".

@apelisse
Copy link
Member

apelisse commented Jun 20, 2023 •
edited
Loading

Hard for me to understand what you mean by that since I can use "apply" and "merge" almost indistinctly.
Apply has always meant "merge" (see #1702), and as an implementation, has always used strategic-merge-patch, and/or json-merge-patch, both merging mechanisms. The term "apply", in my understanding, means "apply these changes on top of the existing resource", with the extra-feature of removing things that I used to have but don't have anymore. People can already replace objects using PUT, but I have a feeling that won't solve their problem either.

@thockin
Copy link
Member Author

thockin commented Jun 20, 2023

I mean: for many cases, "apply" and "replace" are functionally the same (or close enough). Thus, many people think of "apply" as "fancy replace", when it's really "fancy patch". That leads to surprises and frustration, and bug reports that are actually WAI.

@thockin
Copy link
Member Author

thockin commented Jun 23, 2023

Also "terraform apply" is closer to "fancy replace" than "fancy patch", so using the same word causes friction.

@shawkins shawkins mentioned this issue Jul 18, 2023
Open
@mpuckett159
Copy link
Contributor

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 19, 2023
@helayoty helayoty added this to SIG CLI Oct 2, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage in SIG CLI Oct 2, 2023
@helayoty helayoty moved this from Needs Triage to Backlog in SIG CLI Oct 2, 2023
@k8s-triage-robot
Copy link

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Jul 18, 2024
@thockin thockin added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/cli Categorizes an issue or PR as relevant to SIG CLI. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG CLI
Status: Backlog
Milestone
No milestone
Development

No branches or pull requests
5 participants
@apelisse @thockin @mpuckett159 @k8s-ci-robot @k8s-triage-robot