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

Add docs for ApplySet-based pruning in kubectl apply #39818

Merged

k8s-ci-robot merged 8 commits into kubernetes:dev-1.27 from KnVerey:applyset-docs

Apr 4, 2023

Contributor

KnVerey commented Mar 6, 2023 •

edited

Loading

Docs for KEP 3659: https://git.k8s.io/enhancements/keps/sig-cli/3659-kubectl-apply-prune.
KEP issue: kubernetes/enhancements#3659

Screenshots as of Mar 21, commit 07b2089:

k8s-ci-robot added this to the 1.27 milestone

k8s-ci-robot added do-not-merge/work-in-progress cncf-cla: yes labels

KnVerey mentioned this pull request

ApplySet : kubectl apply --prune redesign and graduation strategy kubernetes/enhancements#3659

Open

4 tasks

k8s-ci-robot added the size/S label

k8s-ci-robot requested a review from onlydole

March 6, 2023 19:26

k8s-ci-robot added the area/blog label

k8s-ci-robot requested a review from shannonxtreme

March 6, 2023 19:26

k8s-ci-robot added language/en sig/docs labels

KnVerey force-pushed the applyset-docs branch from 7e43763 to 14228d0 Compare

March 16, 2023 23:24

k8s-ci-robot added size/M and removed size/S labels

KnVerey marked this pull request as ready for review

March 17, 2023 00:12

k8s-ci-robot removed the do-not-merge/work-in-progress label

k8s-ci-robot requested review from nate-double-u and sftim

March 17, 2023 00:12

Contributor Author

KnVerey commented Mar 17, 2023

k8s-ci-robot requested a review from justinsb

March 17, 2023 00:13

KnVerey commented

View reviewed changes

content/en/blog/_posts/2023-04-11-introducing-applyset-pruning.md Outdated


		Authors: Katrina Verey (Shopify) and Justin Santa Barbara (Google)

		PLACEHOLDER: blog post for KEP http://git.k8s.io/enhancements/keps/sig-cli/3659-kubectl-apply-prune

Contributor Author

KnVerey Mar 17, 2023

@justinsb assuming we do want to propose a blog post, should we do it later? If I've understood correctly, the main feature docs are due next Tuesday (Mar 21), but blog posts are due April 4.

Contributor

sftim Mar 17, 2023

Due date for blog placeholders was the 8th of March - see https://github.com/kubernetes/sig-release/tree/master/releases/release-1.27

KnVerey changed the title ~~Placeholder docs for KEP 3659~~ Docs for KEP 3659: ApplySet-based pruning in kubectl apply

Contributor

sftim commented Mar 17, 2023

/retitle Add blog article to announce ApplySet-based pruning in kubectl apply

k8s-ci-robot changed the title ~~Docs for KEP 3659: ApplySet-based pruning in kubectl apply~~ Add blog article to announce ApplySet-based pruning in kubectl apply

sftim reviewed

View reviewed changes

content/en/blog/_posts/2023-04-11-introducing-applyset-pruning.md Outdated Show resolved Hide resolved

sftim changed the title ~~Add blog article to announce ApplySet-based pruning in kubectl apply~~ Add docs and blog article to announce ApplySet-based pruning in kubectl apply

Contributor

sftim commented Mar 17, 2023

/hold

The blog article should be a separate PR so that we can review it in isolation (and can ship v1.27 regardless of progress on post-release blogs).

OK to unhold once the blog part is gone from this PR.

k8s-ci-robot added the do-not-merge/hold label

KnVerey mentioned this pull request

Add blog post to announce ApplySet-based pruning in kubectl #40070

Merged


          Documentation for ApplySet-based pruning (KEP3659)

be6f319

KnVerey force-pushed the applyset-docs branch from 14228d0 to be6f319 Compare

March 17, 2023 03:03

soltysh reviewed

View reviewed changes

Contributor

soltysh left a comment

My biggest concern is wrt to documenting alpha labels, I'd probably pull them together with the rest of the description, so it's clear they are part of alpha or skip it for now.

Alternative, we need an alpha tag next to the labels, so it's clear they should not be relied on, yet, since based on the warning they might change.

content/en/docs/tasks/manage-kubernetes-objects/declarative-config.md

+              {{< feature-state for_k8s_version="v1.27" state="alpha" >}}
+              {{< warning >}}
+              `kubectl apply --prune --applyset` is in alpha, and backwards incompatible

Contributor

soltysh Mar 20, 2023

This warning is why I'd probably not document the labels in the other file, not yet. If we decide to go with a resource, rather than label it'll be confusing and we'll have users relying on alpha feature, which is not plainly obvious in that other file.

Contributor

soltysh Mar 20, 2023

I'm fine documenting it here, under the alpha tag, if there's a desire.

Contributor

sftim Mar 20, 2023

We register labels or annotation as soon as released code might either set them or heed them.

If someone sees a label set on an object and we've not documented that label, it doesn't leave as good an impression.
There's another aspect too: keys are “insta-durable” in as much as the first use we make of them, we're on the hook forever to explain what that label does or did mean. Plenty of the well-known labels are documented as deprecated, but a bit like with stable APIs, once they're registered we never delete the registration.

If it's helpful to, we can:

-### applyset.k8s.io/tooling
+### applyset.k8s.io/tooling (alpha) {#applyset-k8s-io-tooling}

and similar, to mark that the key is alpha. No problem there. Notice how I overrode the fragment identifier so that bookmarks etc still work after graduation to beta or stable.

sftim reviewed

View reviewed changes

content/en/docs/tasks/manage-kubernetes-objects/declarative-config.md Outdated Show resolved Hide resolved

content/en/docs/tasks/manage-kubernetes-objects/declarative-config.md Outdated

               {{< warning >}}
-              You must be careful when using this command, so that you
-              do not delete objects unintentionally.
+              Only use this if you know what you are doing. You must be careful when using this command, so that you do not delete objects unintentionally. For more information on the problems with this command, see the [Current Implementation](git.k8s.io/enhancements/keps/sig-cli/3659-kubectl-apply-prune#current-implementation) section of the ApplySet KEP.

Contributor

sftim Mar 20, 2023

Suggested change

      
            Only use this if you know what you are doing. You must be careful when using this command, so that you do not delete objects unintentionally. For more information on the problems with this command, see the [Current Implementation](git.k8s.io/enhancements/keps/sig-cli/3659-kubectl-apply-prune#current-implementation) section of the ApplySet KEP.
          
            Take care when using `--prune` with `kubectl apply` in allow list mode. The kubectl command must dynamically discover
          
            and infer which objects should be removed as part of the prune operation, and the heuristics for this
          
            can sometimes mean that kubectl deletes objects that did not require pruning.

Contributor Author

KnVerey Mar 20, 2023

The reverse is also true--it can orphan objects that should have been pruned. And user choices regarding the allowlist and labels flags can also cause and/or exacerbate either issue.

content/en/docs/tasks/manage-kubernetes-objects/declarative-config.md Outdated Show resolved Hide resolved

content/en/docs/tasks/manage-kubernetes-objects/declarative-config.md Outdated

               ```shell
-              kubectl apply -f <directory/> --prune -l <labels>
+              kubectl apply -f <directory/> --prune -l <labels> --prune-allowlist=<gvk-list>
               ```
               {{< warning >}}
               Apply with prune should only be run against the root directory
               containing the object configuration files. Running against sub-directories

Contributor

sftim Mar 20, 2023

Optional extra fix:

containing the object manifests. Running against sub-directories

content/en/docs/tasks/manage-kubernetes-objects/declarative-config.md Outdated


		{{< feature-state for_k8s_version="v1.27" state="alpha" >}}

		{{< warning >}}

Contributor

sftim Mar 20, 2023

Use a note or a caution here. Warning is for “this could break your cluster if you don't get it right”; if that's really the casefor apply set pruning, I think we should probably revert it until we're more confident.

Contributor Author

KnVerey Mar 20, 2023

This is just intended as a standard alpha warning, so I'll switch it to caution I guess? This feature is more dangerous than others not because it is incomplete or particularly immature or anything like that; simply because its purpose is to perform destructive actions (deletions), which makes misunderstandings/misuse more consequential. Our unit test coverage is very good (MUCH better than what's there for allowlist-based pruning), and we have a basic multi-step e2e in place as well.

content/en/docs/reference/labels-annotations-taints/_index.md Show resolved Hide resolved

content/en/docs/reference/labels-annotations-taints/_index.md Show resolved Hide resolved

content/en/docs/reference/labels-annotations-taints/_index.md Show resolved Hide resolved

content/en/docs/reference/labels-annotations-taints/_index.md Show resolved Hide resolved

content/en/docs/tasks/manage-kubernetes-objects/declarative-config.md

+              {{< feature-state for_k8s_version="v1.27" state="alpha" >}}
+              {{< warning >}}
+              `kubectl apply --prune --applyset` is in alpha, and backwards incompatible

Contributor

sftim Mar 20, 2023

We register labels or annotation as soon as released code might either set them or heed them.

If someone sees a label set on an object and we've not documented that label, it doesn't leave as good an impression.
There's another aspect too: keys are “insta-durable” in as much as the first use we make of them, we're on the hook forever to explain what that label does or did mean. Plenty of the well-known labels are documented as deprecated, but a bit like with stable APIs, once they're registered we never delete the registration.

If it's helpful to, we can:

-### applyset.k8s.io/tooling
+### applyset.k8s.io/tooling (alpha) {#applyset-k8s-io-tooling}

and similar, to mark that the key is alpha. No problem there. Notice how I overrode the fragment identifier so that bookmarks etc still work after graduation to beta or stable.

KnVerey mentioned this pull request

Use standard *.kubernetes.io instead of *.k8s.io for ApplySet kubernetes/kubernetes#116780

Merged


          Apply suggestions from code review

eb007f2

Co-authored-by: Tim Bannister <[email protected]>

KnVerey force-pushed the applyset-docs branch from 67d5a86 to eb007f2 Compare

March 21, 2023 01:33


          fix label vs annotation copy-paste errors

1b58266

KnVerey requested review from sftim and removed request for justinsb, onlydole, nate-double-u and shannonxtreme

March 21, 2023 01:43


          Update prefix to kubernetes.io

07b2089

Contributor Author

KnVerey commented Mar 21, 2023

Updated with the standard prefix since kubernetes/kubernetes#116780 is fully approved for merge into 1.27.

KnVerey requested review from soltysh and removed request for sftim

March 21, 2023 21:50

tengqm reviewed

View reviewed changes

content/en/docs/reference/labels-annotations-taints/_index.md

+              Used on: Objects being used as ApplySet parents.
+              Use of this annotation is alpha.
+              For Kubernetes version {{< skew currentVersion >}}, you can use this annotation on Secrets, ConfigMaps, or custom resources if the {{< glossary_tooltip term_id="CustomResourceDefinition" text="CustomResourceDefinition" >}} defining them has the `applyset.kubernetes.io/is-parent-type` label.

Contributor

tengqm Mar 21, 2023

Please help wrap the long lines for the benefits of downstream localization teams.

For example:

Suggested change

      
            For Kubernetes version {{< skew currentVersion >}}, you can use this annotation on Secrets, ConfigMaps, or custom resources if the {{< glossary_tooltip term_id="CustomResourceDefinition" text="CustomResourceDefinition" >}} defining them has the `applyset.kubernetes.io/is-parent-type` label.
          
            For Kubernetes version {{< skew currentVersion >}}, you can use this annotation on
          
            Secrets, ConfigMaps, or custom resources if the
          
            {{< glossary_tooltip term_id="CustomResourceDefinition" text="CustomResourceDefinition" >}}
          
            defining them has the `applyset.kubernetes.io/is-parent-type` label.

Contributor Author

KnVerey Mar 23, 2023

Is there an explanation of what the desired wrapping is, or perhaps automation (like a make task) that can take care of the toil if this is required? I'm also curious why that helps localization.

Contributor

sftim Apr 4, 2023

We have a script that identifies lines that have changed. Long lines don't work well with that.

It'd be nice to see this wrapped (eg at around 100 characters). Can happen after this merges.

tengqm reviewed

View reviewed changes

content/en/docs/reference/labels-annotations-taints/_index.md


		Part of the specification used to implement [ApplySet-based pruning in kubectl](/docs/tasks/manage-kubernetes-objects/declarative-config/#alternative-kubectl-apply-f-directory-prune). This annotation is applied to the parent object used to track an ApplySet to extend the scope of the ApplySet beyond the parent object's own namespace (if any). The value is a comma-separated list of the names of namespaces other than the parent's namespace in which objects are found.

		### applyset.kubernetes.io/contains-group-resources (alpha) {#applyset-kubernetes-io-contains-group-resources}

Contributor

tengqm Mar 21, 2023

Maybe a shorter anchor is good enough?

Suggested change

      
            ### applyset.kubernetes.io/contains-group-resources (alpha) {#applyset-kubernetes-io-contains-group-resources}
          
            ### applyset.kubernetes.io/contains-group-resources (alpha) {#applyset-contains-group-resources}

Contributor

sftim Mar 22, 2023

These aren't priced by the byte, you know. The fragment identifies in this page are typically quite long. I think it's OK. The important bit is that they're stable.

Nancy-Chauhan commented Mar 23, 2023

Hello, Comms Shadow for the 1.27 release here. This feature blog is tracked for release, the deadline for submitting the draft is 4th of April- the sooner the better since there's still editing to be done afterwards 😃 Any doubt, the Comms team is here to help. CC @harshitasao

Contributor Author

KnVerey commented Mar 23, 2023

@Nancy-Chauhan I originally included the blog post placeholder on this PR by mistake, but it is now in #40070. This PR is just the docs.

/remove-area blog

k8s-ci-robot removed the area/blog label

LukeMwila commented Apr 4, 2023

/lgtm

k8s-ci-robot assigned LukeMwila

k8s-ci-robot added the lgtm label

Contributor

k8s-ci-robot commented Apr 4, 2023

LGTM label has been added.

Git tree hash: b0f6052c4ff05be02f412272023c98322c3d0edc

Contributor

sftim commented Apr 4, 2023

I previewed this locally. It builds OK.

/approve

Contributor

k8s-ci-robot commented Apr 4, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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:

~~content/en/OWNERS~~ [sftim]

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 added the approved label

k8s-ci-robot merged commit 8c906ea into kubernetes:dev-1.27

KnVerey deleted the applyset-docs branch

April 4, 2023 15:45

DonatoHorn pushed a commit to DonatoHorn/website that referenced this pull request


          Add docs for ApplySet-based pruning in kubectl apply (kubernetes#39818)

35d3f5e

* Documentation for ApplySet-based pruning (KEP3659)

* Apply suggestions from code review

Co-authored-by: Tim Bannister <[email protected]>

* Add ApplySet labels and annotations to well-known list

* Minor fixups

* Address feedback on label/annotation listing

* Apply suggestions from code review

Co-authored-by: Tim Bannister <[email protected]>

* fix label vs annotation copy-paste errors

* Update prefix to kubernetes.io

---------

Co-authored-by: Tim Bannister <[email protected]>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved cncf-cla: yes language/en lgtm sig/docs size/L tide/merge-method-squash