mirror of
https://github.com/replicatedhq/troubleshoot.git
synced 2026-08-27 00:37:20 +00:00
refactor: replace SPDY executor with WebSocket-first fallback executor (#2031)
* docs: add design spec for SPDY executor removal Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: add implementation plan for SPDY executor removal Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(k8sutil): add NewFallbackExecutor helper * fix(k8sutil): rename url param to u to avoid shadowing net/url * refactor(collect): use fallback executor in exec collector * refactor(collect): use fallback executor in copy collector * refactor(collect): use fallback executor in copy_from_host collector * refactor(collect): use fallback executor in sonobuoy_results collector * refactor(collect): use fallback executor in etcd collector * refactor(collect): use fallback executor in longhorn collector * refactor(supportbundle): use fallback executor in collect * chore(k8sutil): delete unused PortForward function * fix(k8sutil): use GET for WebSocket executor and broaden fallback predicate WebSocket upgrade requires GET per RFC 6455. Also add IsHTTPSProxyError to the fallback predicate so HTTPS proxy environments fall back to SPDY correctly, matching kubectl's implementation. Remove method param from NewFallbackExecutor since the methods are now transport-specific and not caller-controlled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: remove superpowers docs artifacts * fix(collect): enable stdout capture in exec collector * fix(collect): enable stdout capture in copy collectors PodExecOptions.Stdout: false causes the WebSocket API server to discard stdout entirely, silently breaking tar output for file copy operations. With SPDY this mismatch was harmless; WebSocket strictly respects the field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
2fdd21fca1
commit
48c45d7b0a
+4
-3
@@ -11,6 +11,7 @@ import (
|
||||
|
||||
"github.com/pkg/errors"
|
||||
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/k8sutil"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/client-go/kubernetes"
|
||||
@@ -102,14 +103,14 @@ func copyFilesFromPod(ctx context.Context, dstPath string, clientConfig *restcli
|
||||
Command: command,
|
||||
Container: containerName,
|
||||
Stdin: true,
|
||||
Stdout: false,
|
||||
Stdout: true,
|
||||
Stderr: true,
|
||||
TTY: false,
|
||||
}, parameterCodec)
|
||||
|
||||
exec, err := remotecommand.NewSPDYExecutor(clientConfig, "POST", req.URL())
|
||||
exec, err := k8sutil.NewFallbackExecutor(clientConfig, req.URL())
|
||||
if err != nil {
|
||||
return nil, nil, errors.Wrap(err, "failed to create SPDY executor")
|
||||
return nil, nil, errors.Wrap(err, "failed to create executor")
|
||||
}
|
||||
|
||||
result := NewResult()
|
||||
|
||||
@@ -304,14 +304,14 @@ func copyFilesFromHost(ctx context.Context, dstPath string, clientConfig *restcl
|
||||
Command: command,
|
||||
Container: containerName,
|
||||
Stdin: true,
|
||||
Stdout: false,
|
||||
Stdout: true,
|
||||
Stderr: true,
|
||||
TTY: false,
|
||||
}, parameterCodec)
|
||||
|
||||
exec, err := remotecommand.NewSPDYExecutor(clientConfig, "POST", req.URL())
|
||||
exec, err := k8sutil.NewFallbackExecutor(clientConfig, req.URL())
|
||||
if err != nil {
|
||||
return nil, nil, errors.Wrap(err, "failed to create SPDY executor")
|
||||
return nil, nil, errors.Wrap(err, "failed to create executor")
|
||||
}
|
||||
|
||||
result := NewResult()
|
||||
|
||||
+2
-1
@@ -10,6 +10,7 @@ import (
|
||||
|
||||
"github.com/pkg/errors"
|
||||
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/k8sutil"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/client-go/kubernetes"
|
||||
@@ -325,7 +326,7 @@ func (c *etcdDebug) executeCommand(command string) ([]byte, []byte, error) {
|
||||
TTY: false,
|
||||
}, scheme.ParameterCodec)
|
||||
|
||||
exec, err := remotecommand.NewSPDYExecutor(c.clientConfig, "POST", req.URL())
|
||||
exec, err := k8sutil.NewFallbackExecutor(c.clientConfig, req.URL())
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
+3
-2
@@ -9,6 +9,7 @@ import (
|
||||
|
||||
"github.com/pkg/errors"
|
||||
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/k8sutil"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/client-go/kubernetes"
|
||||
@@ -132,12 +133,12 @@ func getExecOutputs(
|
||||
Command: append(execCollector.Command, execCollector.Args...),
|
||||
Container: container,
|
||||
Stdin: true,
|
||||
Stdout: false,
|
||||
Stdout: true,
|
||||
Stderr: true,
|
||||
TTY: false,
|
||||
}, parameterCodec)
|
||||
|
||||
exec, err := remotecommand.NewSPDYExecutor(clientConfig, "POST", req.URL())
|
||||
exec, err := k8sutil.NewFallbackExecutor(clientConfig, req.URL())
|
||||
if err != nil {
|
||||
return nil, nil, []string{err.Error()}
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
|
||||
"github.com/pkg/errors"
|
||||
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/k8sutil"
|
||||
longhornv1beta1types "github.com/replicatedhq/troubleshoot/pkg/longhorn/apis/longhorn/v1beta1"
|
||||
longhornv1beta1 "github.com/replicatedhq/troubleshoot/pkg/longhorn/client/clientset/versioned/typed/longhorn/v1beta1"
|
||||
longhorntypes "github.com/replicatedhq/troubleshoot/pkg/longhorn/types"
|
||||
@@ -391,7 +392,7 @@ func GetLonghornReplicaChecksum(clientConfig *rest.Config, replica longhornv1bet
|
||||
Param("command", "-c").
|
||||
Param("command", fmt.Sprintf("if [ -d %s ]; then md5sum %s/*; fi", dir, dir))
|
||||
|
||||
executor, err := remotecommand.NewSPDYExecutor(clientConfig, "POST", req.URL())
|
||||
executor, err := k8sutil.NewFallbackExecutor(clientConfig, req.URL())
|
||||
if err != nil {
|
||||
return "", errors.Wrapf(err, "create remote exec")
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"github.com/pkg/errors"
|
||||
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/client/troubleshootclientset/scheme"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/k8sutil"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
kerrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
@@ -147,7 +148,7 @@ func sonobuoyRetrieveResults(
|
||||
Stdout: true,
|
||||
Stderr: false,
|
||||
}, scheme.ParameterCodec)
|
||||
executor, err := remotecommand.NewSPDYExecutor(restConfig, "POST", req.URL())
|
||||
executor, err := k8sutil.NewFallbackExecutor(restConfig, req.URL())
|
||||
if err != nil {
|
||||
return nil, ec, err
|
||||
}
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
package k8sutil
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
|
||||
"k8s.io/apimachinery/pkg/util/httpstream"
|
||||
restclient "k8s.io/client-go/rest"
|
||||
"k8s.io/client-go/tools/remotecommand"
|
||||
)
|
||||
|
||||
// NewFallbackExecutor creates an executor that tries WebSocket first and falls
|
||||
// back to SPDY if the server does not support it. Use this in place of
|
||||
// remotecommand.NewSPDYExecutor everywhere.
|
||||
func NewFallbackExecutor(config *restclient.Config, u *url.URL) (remotecommand.Executor, error) {
|
||||
// WebSocket upgrade requires GET per RFC 6455; SPDY uses POST.
|
||||
wsExec, err := remotecommand.NewWebSocketExecutor(config, "GET", u.String())
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
spdyExec, err := remotecommand.NewSPDYExecutor(config, "POST", u)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
shouldFallback := func(err error) bool {
|
||||
return httpstream.IsUpgradeFailure(err) || httpstream.IsHTTPSProxyError(err)
|
||||
}
|
||||
return remotecommand.NewFallbackExecutor(wsExec, spdyExec, shouldFallback)
|
||||
}
|
||||
@@ -0,0 +1,19 @@
|
||||
package k8sutil
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
restclient "k8s.io/client-go/rest"
|
||||
)
|
||||
|
||||
func TestNewFallbackExecutor(t *testing.T) {
|
||||
config := &restclient.Config{Host: "http://localhost:8080"}
|
||||
u, err := url.Parse("http://localhost:8080/api/v1/namespaces/default/pods/foo/exec")
|
||||
require.NoError(t, err)
|
||||
|
||||
exec, err := NewFallbackExecutor(config, u)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, exec)
|
||||
}
|
||||
@@ -1,72 +0,0 @@
|
||||
package k8sutil
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
restclient "k8s.io/client-go/rest"
|
||||
"k8s.io/client-go/tools/portforward"
|
||||
"k8s.io/client-go/transport/spdy"
|
||||
)
|
||||
|
||||
func PortForward(config *restclient.Config, localPort int, remotePort int, namespace string, podName string) (chan struct{}, error) {
|
||||
roundTripper, upgrader, err := spdy.RoundTripperFor(config)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
path := fmt.Sprintf("/api/v1/namespaces/%s/pods/%s/portforward", namespace, podName)
|
||||
hostIP := strings.TrimLeft(config.Host, "htps:/")
|
||||
serverURL := url.URL{Scheme: "http", Path: path, Host: hostIP}
|
||||
dialer := spdy.NewDialer(upgrader, &http.Client{Transport: roundTripper}, http.MethodPost, &serverURL)
|
||||
|
||||
stopChan, readyChan := make(chan struct{}, 1), make(chan struct{}, 1)
|
||||
out, errOut := new(bytes.Buffer), new(bytes.Buffer)
|
||||
|
||||
forwarder, err := portforward.New(dialer, []string{fmt.Sprintf("%d:%d", localPort, remotePort)}, stopChan, readyChan, out, errOut)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
go func() {
|
||||
for range readyChan { // Kubernetes will close this channel when it has something to tell us.
|
||||
}
|
||||
if errOut.String() != "" {
|
||||
panic(errOut.String())
|
||||
} else if out.String() != "" {
|
||||
// fmt.Println(out.String())
|
||||
}
|
||||
}()
|
||||
|
||||
go func() error {
|
||||
if err = forwarder.ForwardPorts(); err != nil { // Locks until stopChan is closed.
|
||||
panic(err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}()
|
||||
|
||||
// Block until the new service is responding, limited to (math) seconds
|
||||
quickClient := &http.Client{
|
||||
Timeout: time.Millisecond * 200,
|
||||
}
|
||||
|
||||
start := time.Now()
|
||||
for {
|
||||
response, err := quickClient.Get(fmt.Sprintf("http://localhost:%d", localPort))
|
||||
if err == nil && response.StatusCode == http.StatusOK {
|
||||
break
|
||||
}
|
||||
if time.Now().Sub(start) > time.Duration(time.Second*5) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
time.Sleep(time.Millisecond * 100)
|
||||
}
|
||||
|
||||
return stopChan, nil
|
||||
}
|
||||
@@ -19,6 +19,7 @@ import (
|
||||
"github.com/replicatedhq/troubleshoot/pkg/collect"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/constants"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/convert"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/k8sutil"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/redact"
|
||||
"github.com/replicatedhq/troubleshoot/pkg/version"
|
||||
"go.opentelemetry.io/otel"
|
||||
@@ -367,7 +368,7 @@ func getExecOutputs(
|
||||
TTY: false,
|
||||
}, parameterCodec)
|
||||
|
||||
exec, err := remotecommand.NewSPDYExecutor(clientConfig, "POST", req.URL())
|
||||
exec, err := k8sutil.NewFallbackExecutor(clientConfig, req.URL())
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user