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

Declare kubectl wait flag in a way consistent with other deletion flags #64375

Merged
merged 1 commit into from May 29, 2018

Conversation

@nilebox
Copy link
Member

nilebox commented May 27, 2018

What this PR does / why we need it:
A follow up PR for #64034 and #63979 that makes declaring wait flag consistent with the other flags.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #64401

Special notes for your reviewer:

Release note:


@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented May 27, 2018

@nilebox: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

@neolit123

This comment has been minimized.

Copy link
Member

neolit123 commented May 27, 2018

// To preserve backwards compatibility, but prevent accidental data loss, we convert --grace-period=0
// into --grace-period=1 and wait until the object is successfully deleted. Users may provide --force
// to bypass this wait.
o.WaitForDeletion = true

This comment has been minimized.

@neolit123

neolit123 May 27, 2018

Member

comment above needs adjustment.

if f.Output != nil {
cmd.Flags().StringVarP(f.Output, "output", "o", *f.Output, "Output mode. Use \"-o name\" for shorter output (resource/name).")
}

This comment has been minimized.

@neolit123

neolit123 May 27, 2018

Member

no need for this whitespace change.

This comment has been minimized.

@nilebox

nilebox May 27, 2018

Author Member

sure, but I don't see any value in keeping it.

@@ -211,5 +219,6 @@ func NewDeleteFlags(usage string) *DeleteFlags {
// add non-defaults
Force: &force,
Timeout: &timeout,
Wait: &wait,

This comment has been minimized.

@neolit123

neolit123 May 27, 2018

Member

please, align it to the above param values.

@nilebox nilebox force-pushed the nilebox:delete-wait-cleanup branch 2 times, most recently from 4a1d473 to d4fdae8 May 27, 2018

@ash2k

This comment has been minimized.

Copy link
Member

ash2k commented May 28, 2018

/ok-to-test

@nilebox nilebox changed the title [WIP] Declare wait flag in a way consistent with other deletion flags Declare wait flag in a way consistent with other deletion flags May 28, 2018

@nilebox

This comment has been minimized.

Copy link
Member Author

nilebox commented May 28, 2018

/sig CLI

@nilebox

This comment has been minimized.

Copy link
Member Author

nilebox commented May 28, 2018

/test pull-kubernetes-kubemark-e2e-gce

@nilebox nilebox changed the title Declare wait flag in a way consistent with other deletion flags Declare kubectl wait flag in a way consistent with other deletion flags May 28, 2018

@soltysh soltysh added this to the v1.11 milestone May 28, 2018

@soltysh

This comment has been minimized.

Copy link
Contributor

soltysh commented May 28, 2018

This lgtm, please squash your changes into a single commit and I'll merge it.

@nilebox nilebox force-pushed the nilebox:delete-wait-cleanup branch from fe6c4e1 to acc5dd4 May 28, 2018

@nilebox nilebox force-pushed the nilebox:delete-wait-cleanup branch from acc5dd4 to 6f1b178 May 28, 2018

@dixudx

This comment has been minimized.

Copy link
Member

dixudx commented May 29, 2018

/lgtm

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented May 29, 2018

@nilebox: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

@soltysh
Copy link
Contributor

soltysh left a comment

/lgtm
/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented May 29, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dixudx, nilebox, 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:

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

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

k8s-github-robot commented May 29, 2018

[MILESTONENOTIFIER] Milestone Pull Request Labels Incomplete

@dixudx @nilebox @soltysh

Action required: This pull request requires label changes. If the required changes are not made within 3 days, the pull request will be moved out of the v1.11 milestone.

kind: Must specify exactly one of kind/bug, kind/cleanup or kind/feature.
priority: Must specify exactly one of priority/critical-urgent, priority/important-longterm or priority/important-soon.

Help
@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

k8s-github-robot commented May 29, 2018

Automatic merge from submit-queue (batch tested with PRs 64300, 64375). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 07e6410 into kubernetes:master May 29, 2018

18 checks passed

Submit Queue Queued to run github e2e tests a second time.
Details
cla/linuxfoundation nilebox authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

@nilebox nilebox deleted the nilebox:delete-wait-cleanup branch May 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.