Skip to content

Commit b68bf25

Browse files
authored
Merge pull request kubernetes#90448 from soltysh/issue87851
Fix run command when waiting for pods
2 parents 7297fbd + 6f14842 commit b68bf25

File tree

5 files changed

+38
-60
lines changed

5 files changed

+38
-60
lines changed

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

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -355,21 +355,6 @@ func waitForEphemeralContainer(ctx context.Context, podClient corev1client.PodsG
355355
ctx, cancel := watchtools.ContextWithOptionalTimeout(ctx, 0*time.Second)
356356
defer cancel()
357357

358-
preconditionFunc := func(store cache.Store) (bool, error) {
359-
_, exists, err := store.Get(&metav1.ObjectMeta{Namespace: ns, Name: podName})
360-
if err != nil {
361-
return true, err
362-
}
363-
if !exists {
364-
// We need to make sure we see the object in the cache before we start waiting for events
365-
// or we would be waiting for the timeout if such object didn't exist.
366-
// (e.g. it was deleted before we started informers so they wouldn't even see the delete event)
367-
return true, errors.NewNotFound(corev1.Resource("pods"), podName)
368-
}
369-
370-
return false, nil
371-
}
372-
373358
fieldSelector := fields.OneTermEqualSelector("metadata.name", podName).String()
374359
lw := &cache.ListWatch{
375360
ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
@@ -385,7 +370,7 @@ func waitForEphemeralContainer(ctx context.Context, podClient corev1client.PodsG
385370
intr := interrupt.New(nil, cancel)
386371
var result *corev1.Pod
387372
err := intr.Run(func() error {
388-
ev, err := watchtools.UntilWithSync(ctx, lw, &corev1.Pod{}, preconditionFunc, func(ev watch.Event) (bool, error) {
373+
ev, err := watchtools.UntilWithSync(ctx, lw, &corev1.Pod{}, nil, func(ev watch.Event) (bool, error) {
389374
switch ev.Type {
390375
case watch.Deleted:
391376
return false, errors.NewNotFound(schema.GroupResource{Resource: "pods"}, "")

staging/src/k8s.io/kubectl/pkg/cmd/rollout/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ go_library(
2121
"//build/visible_to:pkg_kubectl_cmd_rollout_CONSUMERS",
2222
],
2323
deps = [
24-
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
2524
"//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
2625
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
2726
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",

staging/src/k8s.io/kubectl/pkg/cmd/rollout/rollout_status.go

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323

2424
"github.com/spf13/cobra"
2525

26-
apierrors "k8s.io/apimachinery/pkg/api/errors"
2726
"k8s.io/apimachinery/pkg/api/meta"
2827
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2928
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -200,25 +199,11 @@ func (o *RolloutStatusOptions) Run() error {
200199
},
201200
}
202201

203-
preconditionFunc := func(store cache.Store) (bool, error) {
204-
_, exists, err := store.Get(&metav1.ObjectMeta{Namespace: info.Namespace, Name: info.Name})
205-
if err != nil {
206-
return true, err
207-
}
208-
if !exists {
209-
// We need to make sure we see the object in the cache before we start waiting for events
210-
// or we would be waiting for the timeout if such object didn't exist.
211-
return true, apierrors.NewNotFound(mapping.Resource.GroupResource(), info.Name)
212-
}
213-
214-
return false, nil
215-
}
216-
217202
// if the rollout isn't done yet, keep watching deployment status
218203
ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), o.Timeout)
219204
intr := interrupt.New(nil, cancel)
220205
return intr.Run(func() error {
221-
_, err = watchtools.UntilWithSync(ctx, lw, &unstructured.Unstructured{}, preconditionFunc, func(e watch.Event) (bool, error) {
206+
_, err = watchtools.UntilWithSync(ctx, lw, &unstructured.Unstructured{}, nil, func(e watch.Event) (bool, error) {
222207
switch t := e.Type; t {
223208
case watch.Added, watch.Modified:
224209
status, done, err := statusViewer.Status(e.Object.(runtime.Unstructured), o.Revision)

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

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -381,17 +381,21 @@ func (o *RunOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e
381381
}
382382

383383
var pod *corev1.Pod
384-
leaveStdinOpen := o.LeaveStdinOpen
385-
waitForExitCode := !leaveStdinOpen && restartPolicy == corev1.RestartPolicyNever
384+
waitForExitCode := !o.LeaveStdinOpen && (restartPolicy == corev1.RestartPolicyNever || restartPolicy == corev1.RestartPolicyOnFailure)
386385
if waitForExitCode {
387-
pod, err = waitForPod(clientset.CoreV1(), attachablePod.Namespace, attachablePod.Name, opts.GetPodTimeout, podCompleted)
386+
// we need different exit condition depending on restart policy
387+
// for Never it can either fail or succeed, for OnFailure only
388+
// success matters
389+
exitCondition := podCompleted
390+
if restartPolicy == corev1.RestartPolicyOnFailure {
391+
exitCondition = podSucceeded
392+
}
393+
pod, err = waitForPod(clientset.CoreV1(), attachablePod.Namespace, attachablePod.Name, opts.GetPodTimeout, exitCondition)
388394
if err != nil {
389395
return err
390396
}
391-
}
392-
393-
// after removal is done, return successfully if we are not interested in the exit code
394-
if !waitForExitCode {
397+
} else {
398+
// after removal is done, return successfully if we are not interested in the exit code
395399
return nil
396400
}
397401

@@ -457,21 +461,6 @@ func waitForPod(podClient corev1client.PodsGetter, ns, name string, timeout time
457461
ctx, cancel := watchtools.ContextWithOptionalTimeout(context.Background(), timeout)
458462
defer cancel()
459463

460-
preconditionFunc := func(store cache.Store) (bool, error) {
461-
_, exists, err := store.Get(&metav1.ObjectMeta{Namespace: ns, Name: name})
462-
if err != nil {
463-
return true, err
464-
}
465-
if !exists {
466-
// We need to make sure we see the object in the cache before we start waiting for events
467-
// or we would be waiting for the timeout if such object didn't exist.
468-
// (e.g. it was deleted before we started informers so they wouldn't even see the delete event)
469-
return true, errors.NewNotFound(corev1.Resource("pods"), name)
470-
}
471-
472-
return false, nil
473-
}
474-
475464
fieldSelector := fields.OneTermEqualSelector("metadata.name", name).String()
476465
lw := &cache.ListWatch{
477466
ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
@@ -487,9 +476,7 @@ func waitForPod(podClient corev1client.PodsGetter, ns, name string, timeout time
487476
intr := interrupt.New(nil, cancel)
488477
var result *corev1.Pod
489478
err := intr.Run(func() error {
490-
ev, err := watchtools.UntilWithSync(ctx, lw, &corev1.Pod{}, preconditionFunc, func(ev watch.Event) (bool, error) {
491-
return exitCondition(ev)
492-
})
479+
ev, err := watchtools.UntilWithSync(ctx, lw, &corev1.Pod{}, nil, exitCondition)
493480
if ev != nil {
494481
result = ev.Object.(*corev1.Pod)
495482
}
@@ -708,6 +695,20 @@ func podCompleted(event watch.Event) (bool, error) {
708695
return false, nil
709696
}
710697

698+
// podSucceeded returns true if the pod has run to completion, false if the pod has not yet
699+
// reached running state, or an error in any other case.
700+
func podSucceeded(event watch.Event) (bool, error) {
701+
switch event.Type {
702+
case watch.Deleted:
703+
return false, errors.NewNotFound(schema.GroupResource{Resource: "pods"}, "")
704+
}
705+
switch t := event.Object.(type) {
706+
case *corev1.Pod:
707+
return t.Status.Phase == corev1.PodSucceeded, nil
708+
}
709+
return false, nil
710+
}
711+
711712
// podRunningAndReady returns true if the pod is running and ready, false if the pod has not
712713
// yet reached those states, returns ErrPodCompleted if the pod has run to completion, or
713714
// an error in any other case.

test/e2e/kubectl/kubectl.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -522,13 +522,21 @@ var _ = SIGDescribe("Kubectl client", func() {
522522
_, err = framework.NewKubectlCommand(ns, nsFlag, "run", "-i", "--image="+busyboxImage, "--restart=OnFailure", "failure-2", "--", "/bin/sh", "-c", "cat && exit 42").
523523
WithStdinData("abcd1234").
524524
Exec()
525-
framework.ExpectNoError(err)
525+
ee, ok = err.(uexec.ExitError)
526+
framework.ExpectEqual(ok, true)
527+
if !strings.Contains(ee.String(), "timed out") {
528+
framework.Failf("Missing expected 'timed out' error, got: %#v", ee)
529+
}
526530

527531
ginkgo.By("running a failing command without --restart=Never, but with --rm")
528532
_, err = framework.NewKubectlCommand(ns, nsFlag, "run", "-i", "--image="+busyboxImage, "--restart=OnFailure", "--rm", "failure-3", "--", "/bin/sh", "-c", "cat && exit 42").
529533
WithStdinData("abcd1234").
530534
Exec()
531-
framework.ExpectNoError(err)
535+
ee, ok = err.(uexec.ExitError)
536+
framework.ExpectEqual(ok, true)
537+
if !strings.Contains(ee.String(), "timed out") {
538+
framework.Failf("Missing expected 'timed out' error, got: %#v", ee)
539+
}
532540
e2epod.WaitForPodToDisappear(f.ClientSet, ns, "failure-3", labels.Everything(), 2*time.Second, wait.ForeverTestTimeout)
533541

534542
ginkgo.By("running a failing command with --leave-stdin-open")

0 commit comments

Comments
 (0)