kubectl delete: Introduce new interactive flag for interactive deletion #114530

ardaguclu · 2022-12-16T07:32:12Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces new interactive flag, namely interactive, into delete command. When this flag is set, users have to confirm resource deletion per resource. As alpha stage, this flag will be hidden behind KUBECTL_INTERACTIVE_DELETE environment variable and can only be used after KUBECTL_INTERACTIVE_DELETE=true.

This PR is continuation of @eddiezane's previous work eddiezane@e970248

Which issue(s) this PR fixes:

Fixes kubernetes/kubectl#1330

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Added a new command line argument `--interactive` to kubectl. The new command line argument lets a user confirm deletion requests per resource interactively.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Alpha implementation of KEP-3895: kubernetes/enhancements#3895

ardaguclu · 2022-12-16T07:33:52Z

/triage accepted
/priority backlog

/hold
until review

ardaguclu · 2022-12-16T07:35:06Z

/cc @eddiezane @KnVerey @brianpursley @soltysh

sftim · 2022-12-17T15:27:39Z

Hi. I think this release note would work better

/release-note-edit Added a new command line argument --confirm to kubectl. The new command line argument lets a user confirm deletion requests per resource interactively.

In Markdown:

Added a new command line argument `--confirm` to kubectl. The new command line argument lets a user confirm deletion requests per resource interactively.

Not sure from https://www.k8s.dev/docs/guide/release-notes/ if this is the right command; 🤞 hoping it works.

staging/src/k8s.io/kubectl/pkg/cmd/delete/delete.go

brianpursley

I left a few minor comments, but the code looks good.

But I do have a bigger question about the feature overall:

Have we considered what the user experience will be like if you want to delete a large set of resources?

Would it make sense to summarize what is about to occur and then ask the user for a single confirmation?

For example, with individual confirmation:

$ kubectl delete pod -l group=abc --confirm
Are you sure you want to delete pod/bar? (y/n): y
pod "bar" deleted
Are you sure you want to delete pod/baz? (y/n): y
pod "baz" deleted
Are you sure you want to delete pod/foo? (y/n): y
pod "foo" deleted

Versus a single confirmation:

$ kubectl delete pod -l group=abc --confirm
You are about to delete the following 3 resources:
pod/bar
pod/baz
pod/foo
Do you want to continue? (y/n): y
pod "bar" deleted
pod "baz" deleted
pod "foo" deleted

I'm not trying to redesign the feature, but want to bring up this topic for consideration and make sure it will work well in all situations.

staging/src/k8s.io/kubectl/pkg/cmd/delete/delete.go

staging/src/k8s.io/kubectl/pkg/cmd/delete/delete_test.go

staging/src/k8s.io/kubectl/pkg/cmd/delete/delete.go

ardaguclu · 2023-02-27T09:29:37Z

Failure is unrelated;

/test pull-kubernetes-node-e2e-containerd

staging/src/k8s.io/kubectl/pkg/cmd/delete/delete.go

soltysh · 2023-02-28T12:25:25Z

staging/src/k8s.io/kubectl/pkg/cmd/delete/delete_flags.go

@@ -155,6 +159,9 @@ func (f *DeleteFlags) AddFlags(cmd *cobra.Command) {
 	if f.Raw != nil {
 		cmd.Flags().StringVar(f.Raw, "raw", *f.Raw, "Raw URI to DELETE to the server.  Uses the transport specified by the kubeconfig file.")
 	}
+	if f.Interactive != nil {
+		cmd.Flags().BoolVarP(f.Interactive, "interactive", "i", *f.Interactive, "If true, delete resource only when user confirms. This flag is in Alpha.")


@ardaguclu I had a lengthy conversation with @KnVerey over the past few days and we concluded that it's best to write a KEP laying out all the possible approaches we've discussed during sig-cli call last week, as well as taking into account information from past efforts (kubernetes/enhancements#2775 and kubernetes/enhancements#2777), and the lengthy discussion that happened on k-dev . Especially, that this will be the first installment of interactivity mechanism in kubectl, so this will lay the ground for eventual future work around the topic.

Lastly, given all the various discussions, I'd suggest writing the KEP asap and sharing it with k-dev and sig-cli mailing list, to gather more feedback about its functionality. This will also allow us more time for experiments, when we hide the flag behind feature gate (aka env var).

/hold
for KEP, and 1.28

k8s-triage-robot · 2023-05-29T12:42:23Z

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

This bot triages 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 PR is closed

You can:

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

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

/lifecycle stale

ardaguclu · 2023-05-29T12:46:28Z

/remove-lifecycle stale

dipankardas011 · 2023-07-03T10:13:46Z

Hi everyone, seen a lot of conversation regarding this issue
I have a thought that what if we don't validate or get if the resource is available or not rather than we can directly use the users input and prompt the above (sorry if this is want being worked on) also this will be much easier to add as it will lead to lower number of changes other than cli

dipankardas011 · 2023-07-03T10:14:49Z

also another query what if admin wants this interactive things as always on what are we planning for that?

ardaguclu · 2023-07-03T10:19:55Z

also another query what if admin wants this interactive things as always on what are we planning for that?

Thanks for dropping your comments @dipankardas011. This will be possible, when kuberc feature is landed(see kubernetes/enhancements#3104)

ardaguclu · 2023-07-04T11:57:40Z

@soltysh could you PTAL?. It is changed to align with the KEP.

soltysh

/lgtm

soltysh · 2023-07-11T11:38:36Z

staging/src/k8s.io/kubectl/pkg/cmd/delete/delete.go

+		// preview result will be used to list resources for confirmation prior to actual delete.
+		// We can not use r as result object because it can only be used once. But we need to traverse
+		// twice. Parameters in preview result must be equal to genuine result.
+		previewr := f.NewBuilder().


Nit 😉

Suggested change

previewr := f.NewBuilder().

previewer := f.NewBuilder().

soltysh · 2023-07-11T12:03:37Z

staging/src/k8s.io/kubectl/pkg/cmd/delete/delete.go

+	}
+	fmt.Fprintf(o.Out, i18n.T("Do you want to continue?")+" (y/n): ")
+	var input string
+	_, err := fmt.Fscan(o.In, &input)


It's probably better to use fmt.Fscanln or r.ReadString('\n') to ensure that it doesn't hang if we don't specify anything. Currently pressing enter will be stuck until you push any button.

Can be addressed in followup.

k8s-ci-robot · 2023-07-11T12:03:54Z

LGTM label has been added.

Git tree hash: 2ba5de33583bfd8a95f56658cbafc769ccb42274

k8s-ci-robot · 2023-07-11T12:04:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ardaguclu, soltysh

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:

~~staging/src/k8s.io/kubectl/OWNERS~~ [ardaguclu,soltysh]
~~test/cmd/OWNERS~~ [ardaguclu,soltysh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

soltysh · 2023-07-11T12:04:12Z

/hold cancel

k8s-ci-robot · 2023-07-11T12:58:09Z

@ardaguclu: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubernetes-e2e-kind-ipv6	`98a6fbd`	link	unknown	`/test pull-kubernetes-e2e-kind-ipv6`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

ardaguclu · 2023-07-11T12:59:43Z

/test pull-kubernetes-e2e-kind-ipv6

k8s-ci-robot requested review from mpuckett159 and seans3 December 16, 2022 07:33

k8s-ci-robot added area/kubectl area/test sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 16, 2022

k8s-ci-robot requested review from brianpursley, eddiezane, KnVerey and soltysh December 16, 2022 07:35

ardaguclu force-pushed the delete-confirm-flag branch from 43461cb to 8f698eb Compare December 16, 2022 08:24

sftim reviewed Dec 17, 2022

View reviewed changes

staging/src/k8s.io/kubectl/pkg/cmd/delete/delete.go Outdated Show resolved Hide resolved

brianpursley suggested changes Dec 23, 2022

View reviewed changes

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Feb 27, 2023

soltysh reviewed Feb 28, 2023

View reviewed changes

This was referenced Mar 1, 2023

kubectl delete: Add interactive(-i) flag kubernetes/enhancements#3895

Closed

KEP-3895: kubectl delete: Add interactive(-i) flag kubernetes/enhancements#3896

Merged

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 29, 2023

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 29, 2023

ardaguclu force-pushed the delete-confirm-flag branch from 178f792 to fd27dd9 Compare July 4, 2023 10:23

kubectl delete: Introduce new interactive flag for interactive deletion

98a6fbd

ardaguclu force-pushed the delete-confirm-flag branch from fd27dd9 to 98a6fbd Compare July 4, 2023 10:29

soltysh approved these changes Jul 11, 2023

View reviewed changes

k8s-ci-robot assigned soltysh Jul 11, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 11, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 11, 2023

k8s-ci-robot merged commit 3267dd9 into kubernetes:master Jul 11, 2023

k8s-ci-robot added this to the v1.28 milestone Jul 11, 2023

ardaguclu deleted the delete-confirm-flag branch July 11, 2023 13:08

ardaguclu mentioned this pull request Jul 21, 2023

Add a warning when a user attempt to delete namespce #119497

Closed

jaskaransarkaria mentioned this pull request Jun 5, 2024

Planning upgrade to EKS 1.28 ministryofjustice/cloud-platform#5570

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubectl delete: Introduce new interactive flag for interactive deletion #114530

kubectl delete: Introduce new interactive flag for interactive deletion #114530

ardaguclu commented Dec 16, 2022 •

edited

Loading

ardaguclu commented Dec 16, 2022

ardaguclu commented Dec 16, 2022

sftim commented Dec 17, 2022 •

edited

Loading

brianpursley left a comment

ardaguclu commented Feb 27, 2023

soltysh Feb 28, 2023

k8s-triage-robot commented May 29, 2023

ardaguclu commented May 29, 2023

dipankardas011 commented Jul 3, 2023

dipankardas011 commented Jul 3, 2023

ardaguclu commented Jul 3, 2023

ardaguclu commented Jul 4, 2023

soltysh left a comment

soltysh Jul 11, 2023

soltysh Jul 11, 2023

k8s-ci-robot commented Jul 11, 2023

k8s-ci-robot commented Jul 11, 2023

soltysh commented Jul 11, 2023

k8s-ci-robot commented Jul 11, 2023

ardaguclu commented Jul 11, 2023

kubectl delete: Introduce new interactive flag for interactive deletion #114530

kubectl delete: Introduce new interactive flag for interactive deletion #114530

Conversation

ardaguclu commented Dec 16, 2022 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

ardaguclu commented Dec 16, 2022

ardaguclu commented Dec 16, 2022

sftim commented Dec 17, 2022 • edited Loading

brianpursley left a comment

Choose a reason for hiding this comment

ardaguclu commented Feb 27, 2023

soltysh Feb 28, 2023

Choose a reason for hiding this comment

k8s-triage-robot commented May 29, 2023

ardaguclu commented May 29, 2023

dipankardas011 commented Jul 3, 2023

dipankardas011 commented Jul 3, 2023

ardaguclu commented Jul 3, 2023

ardaguclu commented Jul 4, 2023

soltysh left a comment

Choose a reason for hiding this comment

soltysh Jul 11, 2023

Choose a reason for hiding this comment

soltysh Jul 11, 2023

Choose a reason for hiding this comment

k8s-ci-robot commented Jul 11, 2023

k8s-ci-robot commented Jul 11, 2023

soltysh commented Jul 11, 2023

k8s-ci-robot commented Jul 11, 2023

ardaguclu commented Jul 11, 2023

ardaguclu commented Dec 16, 2022 •

edited

Loading

sftim commented Dec 17, 2022 •

edited

Loading