kubectl wait: Introduce --wait-for-creation flag #122994

ardaguclu · 2024-01-26T12:58:41Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces new --wait-for-creation flag to also wait for the creation of resources with
preserving the default behavior.

Which issue(s) this PR fixes:

Fixes kubernetes/kubectl#1516, kubernetes/kubectl#1487

Does this PR introduce a user-facing change?

NONE

(release note cleared since this was rolled back in #125630, original release note below)

`kubectl wait` will now wait for resources to be created by default. Please use the new `--wait-for-creation=false` flag for the previous behavior of immediately failing if the specified resource does not yet exist.

ardaguclu · 2024-02-16T08:51:54Z

/triage accepted
/priority important-long-term

k8s-ci-robot · 2024-02-16T08:51:58Z

@ardaguclu: The label(s) priority/important-long-term cannot be applied, because the repository doesn't have them.

In response to this:

/triage accepted
/priority important-long-term

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.

ardaguclu · 2024-02-16T08:54:08Z

/priority important-longterm

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

kubectl wait command errors out when the waited resource does not exist. But we need to provide a way to the users about intentionally also waiting for the creation of resources. This PR introduces a new flag to cover waiting for the creation of resources with preserving the default behavior.

ardaguclu · 2024-06-03T11:22:09Z

unrelated
/lgtm

k8s-ci-robot · 2024-06-03T11:22:11Z

@ardaguclu: you cannot LGTM your own PR.

In response to this:

unrelated
/lgtm

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-sigs/prow repository.

ardaguclu · 2024-06-03T11:24:37Z

not lgtm :)
/retest

sftim · 2024-06-12T18:29:24Z

It's all green! Thanks @ardaguclu

eddiezane · 2024-06-20T18:20:39Z

/release-note-edit

`kubectl wait` will now wait for resources to be created by default. Please use the new `--wait-for-creation=false` flag for the previous behavior.

eddiezane · 2024-06-20T18:21:50Z

/release-note-edit

`kubectl wait` will now wait for resources to be created by default. Please use the new `--wait-for-creation=false` flag for the previous behavior of immediately failing if the specified resource does not yet exist.

eddiezane

Nice work!

/lgtm

k8s-ci-robot · 2024-06-20T19:25:46Z

LGTM label has been added.

Git tree hash: 8fa76aa7cbf4937ba3ada79a9b65f40bdf7d43c1

k8s-ci-robot · 2024-06-20T19:26:02Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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,eddiezane]
~~test/cmd/OWNERS~~ [ardaguclu,eddiezane]

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

liggitt · 2024-06-21T16:20:29Z

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

@@ -320,6 +328,40 @@ func (o *WaitOptions) RunWait() error {
 	ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), o.Timeout)
 	defer cancel()

+	isForDelete := strings.ToLower(o.ForCondition) == "delete"
+	if o.WaitForCreation && o.Timeout == 0 {
+		return fmt.Errorf("--wait-for-creation requires a timeout value greater than 0")


This is a regressing change for folks using --timeout=0 as documented:

The length of time to wait before giving up. Zero means check once and don't wait.

Previously, this would check a condition once:

kubectl wait --for=condition=Available deployments --all --timeout=0

Now, it fails with:

error: --wait-for-creation requires a timeout value greater than 0

I'm not sure defaulting WaitForCreation to true is compatible here. Rollback in #125630 with a test added to exercise the --timeout=0 case so this can be brought back in more compatibly.

Does that mean if the timeout=0, we should ignore the waitforcreation step and mention this in the flag description?

Because when timeout=0, ConditionFn will be executed once and returns

yeah, it (L:336) should be something like this;

if o.WaitForCreation && !isForDelete && o.Timeout != 0 {

Please use the new --wait-for-creation=false flag for the previous behavior of immediately failing if the specified resource does not yet exist.

That sounds like this is changing existing behavior, which doesn't really match the description of This PR introduces new --wait-for-creation flag to also wait for the creation of resources with preserving the default behavior.

Would it be simpler and more compatible to default WaitForCreation to false? Then that would be reasonable to forbid WaitForCreation and Timeout of 0, or WaitForCreation and isForDelete.

I agree with you. Besides, we can change the default separately in the future after carefully considering what we should do for such cases.

@eddiezane what do you think about bringing same PR back just the default value is false?

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/feature Categorizes issue or PR as related to a new feature. labels Jan 26, 2024

ardaguclu marked this pull request as draft January 26, 2024 12:58

k8s-ci-robot requested review from mpuckett159 and seans3 January 26, 2024 12:59

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 Jan 26, 2024

ardaguclu force-pushed the wait-non-existed-resources branch from 0594661 to e9a9207 Compare February 16, 2024 08:49

ardaguclu changed the title ~~Add new --wait-non-existence flag in kubectl wait command~~ kubectl wait: Introduce --wait-for-creation flag Feb 16, 2024

ardaguclu marked this pull request as ready for review February 16, 2024 08:51

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 16, 2024

k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Feb 16, 2024

k8s-ci-robot removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Feb 16, 2024

k8s-ci-robot requested a review from brianpursley February 16, 2024 08:52

ardaguclu force-pushed the wait-non-existed-resources branch from e9a9207 to d1408d1 Compare February 16, 2024 08:53

k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Feb 16, 2024

JayKayy reviewed May 31, 2024

View reviewed changes

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

ardaguclu force-pushed the wait-non-existed-resources branch from b474a40 to 17cd4de Compare June 3, 2024 10:28

k8s-ci-robot added area/code-generation sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 3, 2024

ardaguclu force-pushed the wait-non-existed-resources branch from 17cd4de to 7a8faf9 Compare June 3, 2024 10:30

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 3, 2024

ardaguclu force-pushed the wait-non-existed-resources branch from 7a8faf9 to e24b9a0 Compare June 3, 2024 10:31

eddiezane approved these changes Jun 20, 2024

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 20, 2024

k8s-ci-robot merged commit b95fce1 into kubernetes:master Jun 20, 2024
15 checks passed

k8s-ci-robot added this to the v1.31 milestone Jun 20, 2024

ardaguclu deleted the wait-non-existed-resources branch June 21, 2024 04:32

liggitt mentioned this pull request Jun 21, 2024

Revert kubectl wait regression #125630

Merged

liggitt reviewed Jun 21, 2024

View reviewed changes

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jun 21, 2024

ardaguclu mentioned this pull request Jun 21, 2024

kubectl wait: Introduce --wait-for-creation flag #125632

Closed

soltysh mentioned this pull request Jul 3, 2024

Add --for=create option to kubectl wait #125868

Merged

oshoval mentioned this pull request Aug 5, 2024

Add primary UDN net-attach-def kubevirt/cluster-network-addons-operator#1841

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubectl wait: Introduce --wait-for-creation flag #122994

kubectl wait: Introduce --wait-for-creation flag #122994

ardaguclu commented Jan 26, 2024 •

edited by liggitt

Loading

ardaguclu commented Feb 16, 2024

k8s-ci-robot commented Feb 16, 2024

ardaguclu commented Feb 16, 2024

ardaguclu commented Jun 3, 2024

k8s-ci-robot commented Jun 3, 2024

ardaguclu commented Jun 3, 2024

sftim commented Jun 12, 2024

eddiezane commented Jun 20, 2024

eddiezane commented Jun 20, 2024

eddiezane left a comment

k8s-ci-robot commented Jun 20, 2024

k8s-ci-robot commented Jun 20, 2024

liggitt Jun 21, 2024 •

edited

Loading

ardaguclu Jun 21, 2024

ardaguclu Jun 21, 2024

ardaguclu Jun 21, 2024 •

edited

Loading

liggitt Jun 21, 2024 •

edited

Loading

ardaguclu Jun 21, 2024

ardaguclu Jun 21, 2024

kubectl wait: Introduce --wait-for-creation flag #122994

kubectl wait: Introduce --wait-for-creation flag #122994

Conversation

ardaguclu commented Jan 26, 2024 • edited by liggitt Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?

ardaguclu commented Feb 16, 2024

k8s-ci-robot commented Feb 16, 2024

ardaguclu commented Feb 16, 2024

ardaguclu commented Jun 3, 2024

k8s-ci-robot commented Jun 3, 2024

ardaguclu commented Jun 3, 2024

sftim commented Jun 12, 2024

eddiezane commented Jun 20, 2024

eddiezane commented Jun 20, 2024

eddiezane left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 20, 2024

k8s-ci-robot commented Jun 20, 2024

liggitt Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

ardaguclu Jun 21, 2024

Choose a reason for hiding this comment

ardaguclu Jun 21, 2024

Choose a reason for hiding this comment

ardaguclu Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

liggitt Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

ardaguclu Jun 21, 2024

Choose a reason for hiding this comment

ardaguclu Jun 21, 2024

Choose a reason for hiding this comment

ardaguclu commented Jan 26, 2024 •

edited by liggitt

Loading

liggitt Jun 21, 2024 •

edited

Loading

ardaguclu Jun 21, 2024 •

edited

Loading

liggitt Jun 21, 2024 •

edited

Loading