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

Update kubectl documentation for Server Side Apply #39145

Open
sftim opened this issue Jan 29, 2023 · 25 comments
Open

Update kubectl documentation for Server Side Apply #39145

sftim opened this issue Jan 29, 2023 · 25 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. language/en Issues or PRs related to English language lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. wg/api-expression Categorizes an issue or PR as relevant to WG API Expression.

Comments

@sftim
Copy link
Contributor

sftim commented Jan 29, 2023

This is a Feature Request

What would you like to be added
In #39108 (comment) I explained that I think we missed documenting a key change to kubectl that was made when we implemented server-side apply.

…the input to kubectl apply is no longer defined to be a manifest: it's actually an apply configuration, and all valid manifests are also valid apply configurations. However, kubectl apply defaults to validating that the supplied apply configuration is a valid manifest. You can then turn off that validation by using --validate=false.

I think we should clearly document which kubectl subcommands work with manifests and which with apply configurations.

A few subcommands and plugins to consider:

  • kubectl create
  • kubectl replace
  • kubectl diff
  • kubectl convert

Why is this needed

(If I've understood correctly) kubectl apply does accept YAML manifests as its input, but this is actually a side effect of its actual input format being a new thing called an apply configuration, which can be a subset of the fields from a full manifest.

Comments

/wg api-expression
/language en

/lifecycle frozen
We should triage this issue, no matter how long that process takes.

@sftim sftim added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 29, 2023
@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. wg/api-expression Categorizes an issue or PR as relevant to WG API Expression. language/en Issues or PRs related to English language labels Jan 29, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

SIG Docs takes a lead on issue triage for this website, 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 Jan 29, 2023
@sftim sftim mentioned this issue Jan 29, 2023
@lavalamp
Copy link
Member

FWIW this is not actually a change, CSA and SSA both consume the same input. --validate actually doesn't make much sense for either one, except in the case where the object doesn't exist yet. But you can't know that without talking to the server. I think --validate is from an era when the server did less checking and gave less helpful error messages. I'm not sure it's useful at all anymore.

cc @apelisse

@apelisse
Copy link
Member

apelisse commented Jan 30, 2023

I think --validate is from an era when the server did less checking and gave less helpful error messages. I'm not sure it's useful at all anymore.

And it's being replaced by UnknownFieldValidation which should go GA in the next release, and completely deprecates client-side --validate in the next release.

@sftim
Copy link
Contributor Author

sftim commented Jan 30, 2023

I think at some early point in the evolution of Kubernetes we decided that you could specify a partial thing rather than a full, valid manifest, but we never properly documented what these partial things are.

It doesn't really matter exactly when we missed the opportunity; we still need to do it.

@lavalamp
Copy link
Member

I'm not saying we shouldn't fix the documentation, just trying to make it clear that it isn't a CSA/SSA difference and the documentation should be the same for both.

@sftim
Copy link
Contributor Author

sftim commented Jan 30, 2023

Indeed. The input to kubectl apply is a [something] regardless of whether kubectl then does an apply client-side or server-side. And it's the same [something].

@lavalamp
Copy link
Member

I think I called it a "Partially Specified Object" in design docs; "ApplyConfiguration" is language from the fancy go client, I probably wouldn't use that language in user facing docs?

@sftim
Copy link
Contributor Author

sftim commented Jan 30, 2023

If this is a valid gap in docs, let's triage it accepted and work on a sketch for getting some changes in.

@antaloala

This comment was marked as off-topic.

@sftim

This comment was marked as off-topic.

@sftim

This comment was marked as off-topic.

@antaloala

This comment was marked as off-topic.

@apelisse
Copy link
Member

apelisse commented Feb 1, 2023

OK, let me try to clarify the situation.

  • Objects validation is not optional in Kubernetes, the apiserver will ALWAYS refuse invalid objects.
  • Objects that have extraneous (unknown) fields, or redundant fields, will be valid since the extra-fields are automatically (and silently) dropped by the apiserver
  • Kubectl used to validate these extra-fields using the OpenAPI (and the --validate flag)
  • We've moved the validation of extra-fields from kubectl to the apiserver (also controlled with the --validate field, pay attention to the menu because the options have changed). One can ask the apiserver to drop with a warning (new default), drop silently (former default), or reject the request

@apelisse

This comment was marked as off-topic.

@lavalamp

This comment was marked as off-topic.

@antaloala

This comment was marked as off-topic.

@lavalamp

This comment was marked as off-topic.

@antaloala

This comment was marked as off-topic.

@lavalamp
Copy link
Member

lavalamp commented Feb 1, 2023

Yeah, that's basically it. Glad you like it :)

@sftim

This comment was marked as off-topic.

@antaloala
Copy link
Contributor

@sftim I think we do not really need any other new issue for those missing and relevant SSA-related details I mentioned above, they are already described in https://kubernetes.io/docs/reference/using-api/server-side-apply/

If you remove a field from a configuration and apply the configuration, Server-Side Apply checks if there are any other field managers that also own the field. If the field is not owned by any other field managers, it is either deleted from the live object or reset to its default value, if it has one. The same rule applies to associative list or map items.

By the way, It looks current official k8s SSA documentation already provides a name for these partial manifests/intents: "fully specified intent"

A fully specified intent is a partial object that only includes the fields and values for which the user has an opinion. That intent either creates a new object or is combined, by the server, with the existing object.

So no need to invent a new term if we are all happy with the current one (imo the selected term is a little misleading ... but I am pretty sure there are good reasons behind having selected it so not trying/aiming to open a debate on this ;-)

@sftim
Copy link
Contributor Author

sftim commented Feb 2, 2023

  1. There are going to be a lot of people who go and read about kubectl apply but don't go and look into https://kubernetes.io/docs/reference/using-api/server-side-apply/
  2. We're going to struggle to document server-side apply for end users if we don't know what words to use.

I feel that we should agree on the common term to describe what kubectl apply uses. When we introduce manifests we should at least signpost readers to learn about this other concepts, otherwise they won't know to go and learn about it.

Alternatively, if we're redefining manifest to mean the thing that kubectl apply takes as input, we need a new term for the kind of manifest that passes server-side validation.

Leaving things as they are is going to confuse readers. Worse, they won't even know what they've missed out.

@sftim
Copy link
Contributor Author

sftim commented Feb 2, 2023

I'll mark the side conversation as off topic. Thanks @antaloala .

@lavalamp
Copy link
Member

lavalamp commented Feb 2, 2023

"fully specified intent" seems OK to me.

@antaloala
Copy link
Contributor

antaloala commented Feb 2, 2023

@sftim note in my previous comment I am not saying anything against improving current k8s doc and/or kubectl documentation with respect to SSA missing info. When saying "we do not really need any other new issue" I am only referring to that new issue you suggested to file around how SSA implementation behaves when a manager, in a following SSA Apply request, omits fields for which it is the single current owner (current k8s docs on SSA already describe all this).

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. language/en Issues or PRs related to English language lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. wg/api-expression Categorizes an issue or PR as relevant to WG API Expression.
Projects
None yet
Development

No branches or pull requests

5 participants