diff --git a/connectivity/check/action.go b/connectivity/check/action.go index 0f55e27abb..fb5de411d4 100644 --- a/connectivity/check/action.go +++ b/connectivity/check/action.go @@ -188,7 +188,8 @@ func (a *Action) ExecInPod(ctx context.Context, cmd []string) { a.Debug("Executing command", cmd) - stdout, stderr, err := pod.K8sClient.ExecInPodWithStderr(ctx, + // Warning: ExecInPod* does not use ctx, command cannot be cancelled. + stdout, stderr, err := pod.K8sClient.ExecInPodWithStderr(context.TODO(), pod.Pod.Namespace, pod.Pod.Name, pod.Pod.Labels["name"], cmd) cmdName := cmd[0] diff --git a/connectivity/check/deployment.go b/connectivity/check/deployment.go index 0a7caa97f6..5f602b96bc 100644 --- a/connectivity/check/deployment.go +++ b/connectivity/check/deployment.go @@ -501,7 +501,9 @@ func (ct *ConnectivityTest) waitForDNS(ctx context.Context, pod Pod) error { r := time.After(time.Second) target := "kube-dns.kube-system.svc.cluster.local" - stdout, _, err := pod.K8sClient.ExecInPodWithStderr(ctx, pod.Pod.Namespace, pod.Pod.Name, + // Warning: ExecInPod ignores ctx. Don't pass it here so we don't + // falsely expect the function to be able to be cancelled. + stdout, _, err := pod.K8sClient.ExecInPodWithStderr(context.TODO(), pod.Pod.Namespace, pod.Pod.Name, "", []string{"nslookup", target}) if err == nil { return nil @@ -527,7 +529,9 @@ func (ct *ConnectivityTest) waitForIPCache(ctx context.Context, pod Pod) error { // Don't retry lookups more often than once per second. r := time.After(time.Second) - stdout, err := pod.K8sClient.ExecInPod(ctx, pod.Pod.Namespace, pod.Pod.Name, + // Warning: ExecInPod ignores ctx. Don't pass it here so we don't + // falsely expect the function to be able to be cancelled. + stdout, err := pod.K8sClient.ExecInPod(context.TODO(), pod.Pod.Namespace, pod.Pod.Name, "cilium-agent", []string{"cilium", "bpf", "ipcache", "list", "-o", "json"}) if err == nil { var ic ipCache @@ -607,7 +611,9 @@ func (ct *ConnectivityTest) waitForService(ctx context.Context, service Service) // Don't retry lookups more often than once per second. r := time.After(time.Second) - _, e, err := ct.client.ExecInPodWithStderr(ctx, + // Warning: ExecInPodWithStderr ignores ctx. Don't pass it here so we don't + // falsely expect the function to be able to be cancelled. + _, e, err := ct.client.ExecInPodWithStderr(context.TODO(), pod.Pod.Namespace, pod.Pod.Name, pod.Pod.Labels["name"], []string{"nslookup", service.Service.Name}) // BusyBox nslookup doesn't support any arguments. diff --git a/connectivity/check/policy.go b/connectivity/check/policy.go index 89bb401302..a569c39e10 100644 --- a/connectivity/check/policy.go +++ b/connectivity/check/policy.go @@ -71,7 +71,7 @@ func getCiliumPolicyRevision(ctx context.Context, pod Pod) (int, error) { } revision, err := strconv.Atoi(strings.Trim(stdout.String(), "'\n")) if err != nil { - return 0, fmt.Errorf("revision %q is not valid: %w", stdout.String(), err) + return 0, fmt.Errorf("revision '%s' is not valid: %w", stdout.String(), err) } return revision, nil } diff --git a/internal/k8s/exec.go b/internal/k8s/exec.go index 93afcbdcfa..ff66c0e153 100644 --- a/internal/k8s/exec.go +++ b/internal/k8s/exec.go @@ -19,7 +19,6 @@ import ( "context" "fmt" - "github.com/cilium/cilium-cli/internal/utils" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/remotecommand" @@ -50,10 +49,10 @@ func (c *Client) execInPod(ctx context.Context, p ExecParameters) (*ExecResult, req.VersionedParams(&corev1.PodExecOptions{ Command: p.Command, Container: p.Container, - Stdin: true, + Stdin: false, Stdout: true, Stderr: true, - TTY: true, + TTY: false, }, parameterCodec) exec, err := remotecommand.NewSPDYExecutor(c.Config, "POST", req.URL()) @@ -62,23 +61,11 @@ func (c *Client) execInPod(ctx context.Context, p ExecParameters) (*ExecResult, } result := &ExecResult{} - // CtrlCReader sends Ctrl-C/D sequence if context is cancelled - stdin := utils.NewCtrlCReader(ctx) - // Graceful close of stdin once we are done, no Ctrl-C is sent - // if execution finishes before the context expires. - defer stdin.Close() - err = exec.Stream(remotecommand.StreamOptions{ - Stdin: stdin, + Stdin: nil, Stdout: &result.Stdout, Stderr: &result.Stderr, - Tty: true, + Tty: false, }) - - // Replace "\r\n" sequences in stdout with "\n" - if bytes.Contains(result.Stdout.Bytes(), []byte("\r\n")) { - result.Stdout = *bytes.NewBuffer(bytes.ReplaceAll(result.Stdout.Bytes(), []byte("\r\n"), []byte("\n"))) - } - return result, err } diff --git a/internal/utils/ctrlcreader.go b/internal/utils/ctrlcreader.go deleted file mode 100644 index 3a0b1af650..0000000000 --- a/internal/utils/ctrlcreader.go +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright 2020-2021 Authors of Cilium -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package utils - -import ( - "context" - "io" - "sync" -) - -// CtrlCReader implements a simple Reader/Closer that returns Ctrl-C and EOF -// on Read() after it has been closed, and nothing before it. -type CtrlCReader struct { - ctx context.Context - closeOnce sync.Once - closed chan struct{} -} - -// NewCtrlCReader returns a new CtrlCReader instance -func NewCtrlCReader(ctx context.Context) *CtrlCReader { - return &CtrlCReader{ - ctx: ctx, - closed: make(chan struct{}), - } -} - -// Read implements io.Reader. -// Blocks until we are done. -func (cc *CtrlCReader) Read(p []byte) (n int, err error) { - if len(p) == 0 { - return 0, nil - } - select { - case <-cc.closed: - // Graceful close, EOF without any data - return 0, io.EOF - case <-cc.ctx.Done(): - // Context cancelled, send Ctrl-C/Ctrl-D - p[0] = byte(3) // Ctrl-C - if len(p) > 1 { - // Add Ctrl-D for the case Ctrl-C alone is ineffective. - // We skip this in the odd case where the buffer is too small. - p[1] = byte(4) // Ctrl-D - return 2, io.EOF - } - return 1, io.EOF - } -} - -// Close implements io.Closer. Note that we do not return an error on -// second close, not do we wait for the close to have any effect. -func (cc *CtrlCReader) Close() error { - cc.closeOnce.Do(func() { - close(cc.closed) - }) - return nil -}