From d981b19ad30c3396e8a4d197bedc346e68e0270b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Tue, 13 Aug 2024 11:02:04 +0300 Subject: [PATCH 1/3] Add timeout cancellation to kubectl cp destination path check --- staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go index 5f07d51df3d..b4d3748c387 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go @@ -19,11 +19,13 @@ package cp import ( "archive/tar" "bytes" + "context" "errors" "fmt" "io" "os" "strings" + "time" "github.com/spf13/cobra" @@ -279,7 +281,21 @@ func (o *CopyOptions) checkDestinationIsDir(dest fileSpec) error { Executor: &exec.DefaultRemoteExecutor{}, } - return o.execute(options) + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + + done := make(chan error) + + go func() { + done <- o.execute(options) + }() + + select { + case <-ctx.Done(): + return fmt.Errorf("timeout exceeded while checking the destination") + case err := <-done: + return err + } } func (o *CopyOptions) copyToPod(src, dest fileSpec, options *exec.ExecOptions) error { From 78ae67a90042aa6ed5e91fc5730c10f26ba8d7f6 Mon Sep 17 00:00:00 2001 From: Benjamin Elder Date: Wed, 14 Aug 2024 16:58:02 -0700 Subject: [PATCH 2/3] kuebctl cp: discard output from test command we only care about the exit code see https://github.com/kubernetes/kubernetes/issues/126669 --- staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go index b4d3748c387..9702d8501ed 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go @@ -18,7 +18,6 @@ package cp import ( "archive/tar" - "bytes" "context" "errors" "fmt" @@ -269,8 +268,8 @@ func (o *CopyOptions) checkDestinationIsDir(dest fileSpec) error { options := &exec.ExecOptions{ StreamOptions: exec.StreamOptions{ IOStreams: genericiooptions.IOStreams{ - Out: bytes.NewBuffer([]byte{}), - ErrOut: bytes.NewBuffer([]byte{}), + Out: io.Discard, + ErrOut: io.Discard, }, Namespace: dest.PodNamespace, From ca2c9c64b489d14352e9b6f4c827506a5653a569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Fri, 16 Aug 2024 16:13:01 +0300 Subject: [PATCH 3/3] Increate timeout to 10sec and shortcut when ctx deadline is exceeded --- staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go index 9702d8501ed..68b55bf202e 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go @@ -280,7 +280,7 @@ func (o *CopyOptions) checkDestinationIsDir(dest fileSpec) error { Executor: &exec.DefaultRemoteExecutor{}, } - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() done := make(chan error) @@ -291,7 +291,7 @@ func (o *CopyOptions) checkDestinationIsDir(dest fileSpec) error { select { case <-ctx.Done(): - return fmt.Errorf("timeout exceeded while checking the destination") + return ctx.Err() case err := <-done: return err } @@ -310,6 +310,10 @@ func (o *CopyOptions) copyToPod(src, dest fileSpec, options *exec.ExecOptions) e // If no error, dest.File was found to be a directory. // Copy specified src into it destFile = destFile.Join(srcFile.Base()) + } else if errors.Is(err, context.DeadlineExceeded) { + // we haven't decided destination is directory or not because context timeout is exceeded. + // That's why, we should shortcut the process in here. + return err } go func(src localPath, dest remotePath, writer io.WriteCloser) {