diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 498dbd9ae..23645e42e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -409,6 +409,37 @@ jobs: with: message-path: download-link.txt + # e2e-docker runs the e2e tests inside a docker container rather than a full VM + e2e-docker: + runs-on: ubuntu-latest + needs: + - build + strategy: + fail-fast: false + matrix: + test: + - TestPreflights + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Download binary + uses: actions/download-artifact@v4 + with: + name: embedded-release + path: output/bin + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: Write license file + run: | + echo "${{ secrets.STAGING_EMBEDDED_CLUSTER_LICENSE }}" | base64 --decode > e2e/license.yaml + - name: Run test + env: + LICENSE_PATH: license.yaml + run: | + make e2e-test TEST_NAME=${{ matrix.test }} + e2e: runs-on: ${{ matrix.runner || 'ubuntu-latest' }} needs: @@ -499,6 +530,7 @@ jobs: runs-on: ubuntu-20.04 needs: - e2e + - e2e-docker - sanitize - tests - check-images diff --git a/Makefile b/Makefile index 619b979d6..7db684807 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ K0S_GO_VERSION = v1.29.7+k0s.0 PREVIOUS_K0S_VERSION ?= v1.28.10+k0s.0 K0S_BINARY_SOURCE_OVERRIDE = PREVIOUS_K0S_BINARY_SOURCE_OVERRIDE = -TROUBLESHOOT_VERSION = v0.97.0 +TROUBLESHOOT_VERSION = v0.100.0 KOTS_VERSION = v$(shell awk '/^version/{print $$2}' pkg/addons/adminconsole/static/metadata.yaml | sed 's/\([0-9]\+\.[0-9]\+\.[0-9]\+\).*/\1/') KOTS_BINARY_URL_OVERRIDE = # TODO: move this to a manifest file @@ -33,6 +33,7 @@ LD_FLAGS = \ -X github.com/replicatedhq/embedded-cluster/pkg/addons/adminconsole.AdminConsoleImageOverride=$(ADMIN_CONSOLE_IMAGE_OVERRIDE) \ -X github.com/replicatedhq/embedded-cluster/pkg/addons/adminconsole.AdminConsoleMigrationsImageOverride=$(ADMIN_CONSOLE_MIGRATIONS_IMAGE_OVERRIDE) \ -X github.com/replicatedhq/embedded-cluster/pkg/addons/adminconsole.AdminConsoleKurlProxyImageOverride=$(ADMIN_CONSOLE_KURL_PROXY_IMAGE_OVERRIDE) +DISABLE_FIO_BUILD ?= 0 export PATH := $(shell pwd)/bin:$(PATH) @@ -73,6 +74,16 @@ pkg/goods/bins/local-artifact-mirror: Makefile $(MAKE) -C local-artifact-mirror build GOOS=linux GOARCH=amd64 cp local-artifact-mirror/bin/local-artifact-mirror-$(GOOS)-$(GOARCH) pkg/goods/bins/local-artifact-mirror +pkg/goods/bins/fio: PLATFORM = linux/amd64 +pkg/goods/bins/fio: Makefile +ifneq ($(DISABLE_FIO_BUILD),1) + mkdir -p pkg/goods/bins + docker build -t fio --build-arg PLATFORM=$(PLATFORM) fio + docker rm -f fio && docker run --name fio fio + docker cp fio:/output/fio pkg/goods/bins/fio + touch pkg/goods/bins/fio +endif + pkg/goods/internal/bins/kubectl-kots: Makefile mkdir -p pkg/goods/internal/bins mkdir -p output/tmp/kots @@ -107,6 +118,7 @@ static: pkg/goods/bins/k0s \ pkg/goods/bins/kubectl-preflight \ pkg/goods/bins/kubectl-support_bundle \ pkg/goods/bins/local-artifact-mirror \ + pkg/goods/bins/fio \ pkg/goods/internal/bins/kubectl-kots .PHONY: embedded-cluster-linux-amd64 diff --git a/cmd/embedded-cluster/install.go b/cmd/embedded-cluster/install.go index a1864e047..bec7feee6 100644 --- a/cmd/embedded-cluster/install.go +++ b/cmd/embedded-cluster/install.go @@ -119,11 +119,14 @@ func runHostPreflights(c *cli.Context, hpf *v1beta2.HostPreflightSpec, proxy *ec return nil } pb.Infof("Running host preflights") - output, err := preflights.Run(c.Context, hpf, proxy) + output, stderr, err := preflights.Run(c.Context, hpf, proxy) if err != nil { pb.CloseWithError() return fmt.Errorf("host preflights failed to run: %w", err) } + if stderr != "" { + logrus.Debugf("preflight stderr: %s", stderr) + } err = output.SaveToDisk() if err != nil { diff --git a/e2e/docker/docker.go b/e2e/docker/docker.go new file mode 100644 index 000000000..a7fc57be1 --- /dev/null +++ b/e2e/docker/docker.go @@ -0,0 +1,124 @@ +package docker + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +type Docker struct { + client string + t *testing.T +} + +func NewCLI(t *testing.T) *Docker { + client, err := exec.LookPath("docker") + if err != nil { + t.Fatalf("failed to find docker in path: %v", err) + } + return &Docker{ + client: client, + t: t, + } +} + +type Container struct { + Image string + Volumes []string + + id string + t *testing.T +} + +func NewContainer(t *testing.T) *Container { + return &Container{ + id: generateID(), + t: t, + } +} + +func (c *Container) WithImage(image string) *Container { + c.Image = image + return c +} + +func (c *Container) WithECBinary() *Container { + path, err := filepath.Abs("../output/bin/embedded-cluster") + if err != nil { + c.t.Fatalf("failed to get absolute path to embedded-cluster binary: %v", err) + } + _, err = os.Stat(path) + if err != nil { + c.t.Fatalf("failed to find embedded-cluster binary: %v", err) + } + err = os.Chmod(path, 0755) + if err != nil { + c.t.Fatalf("failed to chmod embedded-cluster binary: %v", err) + } + return c.WithVolume(fmt.Sprintf("%s:%s", path, c.GetECBinaryPath())) +} + +func (c *Container) GetECBinaryPath() string { + return "/ec/bin/embedded-cluster" +} + +func (c *Container) WithLicense(path string) *Container { + path, err := filepath.Abs(path) + if err != nil { + c.t.Fatalf("failed to get absolute path to license file: %v", err) + } + _, err = os.Stat(path) + if err != nil { + c.t.Fatalf("failed to find embedded-cluster binary: %v", err) + } + return c.WithVolume(fmt.Sprintf("%s:%s", path, c.GetLicensePath())) +} + +func (c *Container) GetLicensePath() string { + return "/ec/license.yaml" +} + +func (c *Container) WithVolume(volume string) *Container { + c.Volumes = append(c.Volumes, volume) + return c +} + +func (c *Container) Start(cli *Docker) { + execCmd := exec.Command( + cli.client, "run", "--rm", "-d", "-w", "/ec", "--platform=linux/amd64", "--privileged", + "--name", c.id, + ) + for _, volume := range c.Volumes { + execCmd.Args = append(execCmd.Args, "-v", volume) + } + execCmd.Args = append(execCmd.Args, c.Image) + execCmd.Args = append(execCmd.Args, "sh", "-c", "while true; do sleep 1; done") + c.t.Logf("starting container: docker %s", strings.Join(execCmd.Args, " ")) + err := execCmd.Run() + if err != nil { + c.t.Fatalf("failed to start container: %v", err) + } +} + +func (c *Container) Destroy(cli *Docker) { + execCmd := exec.Command(cli.client, "rm", "-f", c.id) + err := execCmd.Run() + if err != nil { + c.t.Fatalf("failed to destroy container: %v", err) + } +} + +func (c *Container) Exec(cli *Docker, cmd string) (string, string, error) { + args := []string{"exec", c.id, "sh", "-c", cmd} + execCmd := exec.Command(cli.client, args...) + c.t.Logf("executing command: docker %s", strings.Join(execCmd.Args, " ")) + var stdout, stderr bytes.Buffer + execCmd.Stdout = &stdout + execCmd.Stderr = &stderr + err := execCmd.Run() + return stdout.String(), stderr.String(), err +} diff --git a/e2e/docker/id.go b/e2e/docker/id.go new file mode 100644 index 000000000..63528309c --- /dev/null +++ b/e2e/docker/id.go @@ -0,0 +1,13 @@ +package docker + +import "math/rand" + +var alphabet = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789") + +func generateID() string { + b := make([]rune, 32) + for i := range b { + b[i] = alphabet[rand.Intn(len(alphabet))] + } + return "ece2e-" + string(b) +} diff --git a/e2e/materialize_test.go b/e2e/materialize_test.go index d4b57f615..a84e88f07 100644 --- a/e2e/materialize_test.go +++ b/e2e/materialize_test.go @@ -21,10 +21,12 @@ func TestMaterialize(t *testing.T) { {"rm", "-rf", "/var/lib/embedded-cluster/bin/kubectl"}, {"rm", "-rf", "/var/lib/embedded-cluster/bin/kubectl-preflight"}, {"rm", "-rf", "/var/lib/embedded-cluster/bin/kubectl-support_bundle"}, + {"rm", "-rf", "/var/lib/embedded-cluster/bin/fio"}, {"embedded-cluster", "materialize"}, {"ls", "-la", "/var/lib/embedded-cluster/bin/kubectl"}, {"ls", "-la", "/var/lib/embedded-cluster/bin/kubectl-preflight"}, {"ls", "-la", "/var/lib/embedded-cluster/bin/kubectl-support_bundle"}, + {"ls", "-la", "/var/lib/embedded-cluster/bin/fio"}, } if err := RunCommandsOnNode(t, tc, 0, commands); err != nil { t.Fatalf("fail testing materialize assets: %v", err) diff --git a/e2e/preflights_test.go b/e2e/preflights_test.go new file mode 100644 index 000000000..ce1f8c5aa --- /dev/null +++ b/e2e/preflights_test.go @@ -0,0 +1,124 @@ +package e2e + +import ( + "fmt" + "os" + "strings" + "testing" + + "github.com/replicatedhq/embedded-cluster/e2e/docker" + "github.com/replicatedhq/embedded-cluster/pkg/preflights" +) + +func TestPreflights(t *testing.T) { + t.Parallel() + + cli := docker.NewCLI(t) + + container := docker.NewContainer(t). + WithImage("debian:bookworm-slim"). + WithECBinary() + if licensePath := os.Getenv("LICENSE_PATH"); licensePath != "" { + t.Logf("using license %s", licensePath) + container = container.WithLicense(licensePath) + } + container.Start(cli) + + t.Cleanup(func() { + container.Destroy(cli) + }) + + _, stderr, err := container.Exec(cli, + "apt-get update && apt-get install -y apt-utils kmod", + ) + if err != nil { + t.Fatalf("failed to install deps: err=%v, stderr=%s", err, stderr) + } + + runCmd := fmt.Sprintf("%s install run-preflights --no-prompt", container.GetECBinaryPath()) + if os.Getenv("LICENSE_PATH") != "" { + runCmd = fmt.Sprintf("%s --license %s", runCmd, container.GetLicensePath()) + } + + // we are more interested in the results + runStdout, runStderr, runErr := container.Exec(cli, runCmd) + + stdout, stderr, err := container.Exec(cli, + "cat /var/lib/embedded-cluster/support/host-preflight-results.json", + ) + if err != nil { + t.Logf("run-preflights: err=%v, stdout=%s, stderr=%s", runErr, runStdout, runStderr) + t.Fatalf("failed to get preflight results: err=%v, stderr=%s", err, stderr) + } + + results, err := preflights.OutputFromReader(strings.NewReader(stdout)) + if err != nil { + t.Fatalf("failed to parse preflight results: %v", err) + } + + tests := []struct { + name string + assert func(t *testing.T, results *preflights.Output) + }{ + { + name: "Should contain fio results", + assert: func(t *testing.T, results *preflights.Output) { + for _, res := range results.Pass { + if res.Title == "Filesystem Write Latency" { + t.Logf("fio test passed: %s", res.Message) + return + } + } + for _, res := range results.Fail { + if !strings.Contains(res.Message, "Write latency is high") { + t.Errorf("fio test failed: %s", res.Message) + } + // as long as fio ran successfully, we're good + t.Logf("fio test failed: %s", res.Message) + } + + t.Errorf("fio test not found") + }, + }, + { + name: "Should not contain unexpected failures", + assert: func(t *testing.T, results *preflights.Output) { + expected := map[string]bool{ + // TODO: work to remove these + "System Clock": true, + "'devices' Cgroup Controller": true, + "Default Route": true, + "API Access": true, + "Proxy Registry Access": true, + // as long as fio ran successfully, we're good + "Filesystem Write Latency": true, + } + for _, res := range results.Fail { + if !expected[res.Title] { + t.Errorf("unexpected failure: %q, %q", res.Title, res.Message) + } else { + t.Logf("found expected failure: %q, %q", res.Title, res.Message) + } + } + }, + }, + { + name: "Should not contain unexpected warnings", + assert: func(t *testing.T, results *preflights.Output) { + expected := map[string]bool{} + for _, res := range results.Warn { + if !expected[res.Title] { + t.Errorf("unexpected warning: %q, %q", res.Title, res.Message) + } else { + t.Logf("found expected warning: %q, %q", res.Title, res.Message) + } + } + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.assert(t, results) + }) + } +} diff --git a/fio/Dockerfile b/fio/Dockerfile new file mode 100644 index 000000000..e03b4387a --- /dev/null +++ b/fio/Dockerfile @@ -0,0 +1,29 @@ +ARG PLATFORM=linux/amd64 + +FROM --platform=$PLATFORM ubuntu:22.04 AS build + +ARG DEBIAN_FRONTEND=noninteractive +ARG TZ=Etc/UTC + +RUN apt-get update \ + && apt-get install -y \ + build-essential cmake libstdc++6 pkg-config unzip wget + +RUN mkdir -p /fio +WORKDIR /fio + +RUN wget -O- https://api.github.com/repos/axboe/fio/releases/latest \ + | grep "tarball_url" \ + | cut -d : -f 2,3 \ + | tr -d '", ' \ + | xargs -n1 wget -O fio.tar.gz -q +RUN tar -xzf fio.tar.gz --strip-components=1 + +RUN ./configure --build-static +RUN make -j$(nproc) + +FROM ubuntu:22.04 +COPY --from=build /fio/fio /output/fio +# ensure that the binary is statically linked +RUN ldd /output/fio 2>&1 | grep 'not a dynamic executable' +CMD [ "echo", "Done" ] diff --git a/go.mod b/go.mod index db91ca7cc..79bbed5f3 100644 --- a/go.mod +++ b/go.mod @@ -32,7 +32,7 @@ require ( github.com/replicatedhq/embedded-cluster-operator v0.42.1 github.com/replicatedhq/embedded-cluster-utils v1.0.0 github.com/replicatedhq/kotskinds v0.0.0-20240621115447-55148ce032e4 - github.com/replicatedhq/troubleshoot v0.99.0 + github.com/replicatedhq/troubleshoot v0.100.0 github.com/sirupsen/logrus v1.9.3 github.com/spf13/cobra v1.8.1 github.com/spf13/viper v1.19.0 diff --git a/go.sum b/go.sum index c9f3ed89b..aa069683f 100644 --- a/go.sum +++ b/go.sum @@ -516,8 +516,8 @@ github.com/replicatedhq/embedded-cluster-utils v1.0.0 h1:Axdni1nYfl5zeOP9g5U79yv github.com/replicatedhq/embedded-cluster-utils v1.0.0/go.mod h1:4JmMC2CwMCLxq05GEW3XSPPVotqyamAF/omrbB3pH+c= github.com/replicatedhq/kotskinds v0.0.0-20240621115447-55148ce032e4 h1:nsNSod6wpFpaMUq1IqnU4y2XWQd2FEKtdr02UB2udkk= github.com/replicatedhq/kotskinds v0.0.0-20240621115447-55148ce032e4/go.mod h1:QjhIUu3+OmHZ09u09j3FCoTt8F3BYtQglS+OLmftu9I= -github.com/replicatedhq/troubleshoot v0.99.0 h1:KtsCe/8EL1VPQrokZw3bcKo8HcCTRUMEtUb2+SJ5l1k= -github.com/replicatedhq/troubleshoot v0.99.0/go.mod h1:5rRx3kCUCX9Adl3ST1mzo57FICjIJMaIrkj3rTrzvv4= +github.com/replicatedhq/troubleshoot v0.100.0 h1:efRc3M91Dnnvv66oSX5vs+GF2MjHS6O1OriBikox15Y= +github.com/replicatedhq/troubleshoot v0.100.0/go.mod h1:5rRx3kCUCX9Adl3ST1mzo57FICjIJMaIrkj3rTrzvv4= github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= diff --git a/pkg/defaults/defaults.go b/pkg/defaults/defaults.go index 3d2e97843..9aee6b550 100644 --- a/pkg/defaults/defaults.go +++ b/pkg/defaults/defaults.go @@ -3,6 +3,11 @@ // these should not happen in the first place. package defaults +var ( + // DefaultProvider holds the default provider and is used by the exported functions. + DefaultProvider = NewProvider("") +) + var ( // provider holds a global reference to the default provider. provider *Provider @@ -11,97 +16,88 @@ var ( // Holds the default no proxy values. var DefaultNoProxy = []string{"localhost", "127.0.0.1", ".default", ".local", ".svc", "kubernetes", "kotsadm-rqlite"} -var ProxyRegistryAddress = "proxy.replicated.com" +const ProxyRegistryAddress = "proxy.replicated.com" const KotsadmNamespace = "kotsadm" const SeaweedFSNamespace = "seaweedfs" const RegistryNamespace = "registry" const VeleroNamespace = "velero" -// def returns a global reference to the default provider. creates one if not -// already created. -func def() *Provider { - if provider == nil { - provider = NewProvider("") - } - return provider -} - // BinaryName calls BinaryName on the default provider. func BinaryName() string { - return def().BinaryName() + return DefaultProvider.BinaryName() } // EmbeddedClusterBinsSubDir calls EmbeddedClusterBinsSubDir on the default provider. func EmbeddedClusterBinsSubDir() string { - return def().EmbeddedClusterBinsSubDir() + return DefaultProvider.EmbeddedClusterBinsSubDir() } // EmbeddedClusterChartsSubDir calls EmbeddedClusterChartsSubDir on the default provider. func EmbeddedClusterChartsSubDir() string { - return def().EmbeddedClusterChartsSubDir() + return DefaultProvider.EmbeddedClusterChartsSubDir() } // EmbeddedClusterImagesSubDir calls EmbeddedClusterImagesSubDir on the default provider. func EmbeddedClusterImagesSubDir() string { - return def().EmbeddedClusterImagesSubDir() + return DefaultProvider.EmbeddedClusterImagesSubDir() } // EmbeddedClusterLogsSubDir calls EmbeddedClusterLogsSubDir on the default provider. func EmbeddedClusterLogsSubDir() string { - return def().EmbeddedClusterLogsSubDir() + return DefaultProvider.EmbeddedClusterLogsSubDir() } // K0sBinaryPath calls K0sBinaryPath on the default provider. func K0sBinaryPath() string { - return def().K0sBinaryPath() + return DefaultProvider.K0sBinaryPath() } // PathToEmbeddedClusterBinary calls PathToEmbeddedClusterBinary on the default provider. func PathToEmbeddedClusterBinary(name string) string { - return def().PathToEmbeddedClusterBinary(name) + return DefaultProvider.PathToEmbeddedClusterBinary(name) } // PathToLog calls PathToLog on the default provider. func PathToLog(name string) string { - return def().PathToLog(name) + return DefaultProvider.PathToLog(name) } // PathToKubeConfig calls PathToKubeConfig on the default provider. func PathToKubeConfig() string { - return def().PathToKubeConfig() + return DefaultProvider.PathToKubeConfig() } // PreferredNodeIPAddress calls PreferredNodeIPAddress on the default provider. func PreferredNodeIPAddress() (string, error) { - return def().PreferredNodeIPAddress() + return DefaultProvider.PreferredNodeIPAddress() } // TryDiscoverPublicIP calls TryDiscoverPublicIP on the default provider. func TryDiscoverPublicIP() string { - return def().TryDiscoverPublicIP() + return DefaultProvider.TryDiscoverPublicIP() } // PathToK0sConfig calls PathToK0sConfig on the default provider. func PathToK0sConfig() string { - return def().PathToK0sConfig() + return DefaultProvider.PathToK0sConfig() } // PathToK0sStatusSocket calls PathToK0sStatusSocket on the default provider. func PathToK0sStatusSocket() string { - return def().PathToK0sStatusSocket() + return DefaultProvider.PathToK0sStatusSocket() } func PathToK0sContainerdConfig() string { - return def().PathToK0sContainerdConfig() + return DefaultProvider.PathToK0sContainerdConfig() } // EmbeddedClusterHomeDirectory calls EmbeddedClusterHomeDirectory on the default provider. func EmbeddedClusterHomeDirectory() string { - return def().EmbeddedClusterHomeDirectory() + return DefaultProvider.EmbeddedClusterHomeDirectory() } // PathToEmbeddedClusterSupportFile calls PathToEmbeddedClusterSupportFile on the default provider. func PathToEmbeddedClusterSupportFile(name string) string { - return def().PathToEmbeddedClusterSupportFile(name) + return DefaultProvider.PathToEmbeddedClusterSupportFile(name) } diff --git a/pkg/preflights/host-preflight.yaml b/pkg/preflights/host-preflight.yaml index bb8d2cd63..1741e7bcc 100644 --- a/pkg/preflights/host-preflight.yaml +++ b/pkg/preflights/host-preflight.yaml @@ -63,6 +63,14 @@ spec: collectorName: resolv.conf command: 'sh' args: ['-c', 'cat /etc/resolv.conf'] + - filesystemPerformance: + collectorName: filesystem-write-latency-etcd + timeout: 5m + directory: /var/lib/k0s/etcd + fileSize: 22Mi + operationSize: 2300 + datasync: true + runTime: "0" # let it run to completion analyzers: - cpu: checkName: CPU @@ -355,4 +363,13 @@ spec: message: "Neither 'nameserver localhost' nor 'nameserver 127.0.0.1' can be present in resolv.conf. Remove them to continue." - pass: when: "false" - message: "Neither 'nameserver localhost' nor 'nameserver 127.0.01' is present in resolv.conf" \ No newline at end of file + message: "Neither 'nameserver localhost' nor 'nameserver 127.0.01' is present in resolv.conf" + - filesystemPerformance: + checkName: Filesystem Write Latency + collectorName: filesystem-write-latency-etcd + outcomes: + - pass: + when: "p99 < 10ms" + message: 'P99 write latency for the disk at /var/lib/k0s/etcd is {{ "{{" }} .P99 {{ "}}" }}, which is better than the 10 ms requirement.' + - fail: + message: 'P99 write latency for the disk at /var/lib/k0s/etcd is {{ "{{" }} .P99 {{ "}}" }}, but it must be less than 10 ms. A higher-performance disk is required.' diff --git a/pkg/preflights/preflights.go b/pkg/preflights/preflights.go index f220584d0..748abd73e 100644 --- a/pkg/preflights/preflights.go +++ b/pkg/preflights/preflights.go @@ -7,7 +7,6 @@ import ( "encoding/json" "errors" "fmt" - "io" "os" "os/exec" "strings" @@ -32,29 +31,34 @@ func SerializeSpec(spec *troubleshootv1beta2.HostPreflightSpec) ([]byte, error) // Run runs the provided host preflight spec locally. This function is meant to be // used when upgrading a local node. -func Run(ctx context.Context, spec *troubleshootv1beta2.HostPreflightSpec, proxy *ecv1beta1.ProxySpec) (*Output, error) { +func Run(ctx context.Context, spec *troubleshootv1beta2.HostPreflightSpec, proxy *ecv1beta1.ProxySpec) (*Output, string, error) { // Deduplicate collectors and analyzers before running preflights spec.Collectors = dedup(spec.Collectors) spec.Analyzers = dedup(spec.Analyzers) fpath, err := saveHostPreflightFile(spec) if err != nil { - return nil, fmt.Errorf("unable to save preflight locally: %w", err) + return nil, "", fmt.Errorf("unable to save preflight locally: %w", err) } defer os.Remove(fpath) binpath := defaults.PathToEmbeddedClusterBinary("kubectl-preflight") stdout := bytes.NewBuffer(nil) + stderr := bytes.NewBuffer(nil) cmd := exec.Command(binpath, "--interactive=false", "--format=json", fpath) - cmd.Env = proxyEnv(proxy) - cmd.Stdout, cmd.Stderr = stdout, io.Discard + cmd.Env = os.Environ() + cmd.Env = proxyEnv(cmd.Env, proxy) + cmd.Env = pathEnv(cmd.Env) + cmd.Stdout, cmd.Stderr = stdout, stderr if err = cmd.Run(); err == nil { - return OutputFromReader(stdout) + out, err := OutputFromReader(stdout) + return out, stderr.String(), err } var exit *exec.ExitError if !errors.As(err, &exit) || exit.ExitCode() < 2 { - return nil, fmt.Errorf("unknown error running host preflight: %w", err) + return nil, stderr.String(), fmt.Errorf("error running host preflight: %w, stderr=%q", err, stderr.String()) } - return OutputFromReader(stdout) + out, err := OutputFromReader(stdout) + return out, stderr.String(), err } // saveHostPreflightFile saves the provided spec to a temporary file and returns @@ -97,20 +101,40 @@ func dedup[T any](objs []T) []T { return out } -func proxyEnv(proxy *ecv1beta1.ProxySpec) []string { - env := []string{} - for _, e := range os.Environ() { +func proxyEnv(env []string, proxy *ecv1beta1.ProxySpec) []string { + next := []string{} + for _, e := range env { switch strings.SplitN(e, "=", 2)[0] { // Unset proxy environment variables case "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy": - continue + default: + next = append(next, e) } - env = append(env, e) } if proxy != nil { - env = append(env, fmt.Sprintf("HTTP_PROXY=%s", proxy.HTTPProxy)) - env = append(env, fmt.Sprintf("HTTPS_PROXY=%s", proxy.HTTPSProxy)) - env = append(env, fmt.Sprintf("NO_PROXY=%s", proxy.NoProxy)) + next = append(next, fmt.Sprintf("HTTP_PROXY=%s", proxy.HTTPProxy)) + next = append(next, fmt.Sprintf("HTTPS_PROXY=%s", proxy.HTTPSProxy)) + next = append(next, fmt.Sprintf("NO_PROXY=%s", proxy.NoProxy)) + } + return next +} + +func pathEnv(env []string) []string { + path := "" + next := []string{} + for _, e := range env { + switch strings.SplitN(e, "=", 2)[0] { + // Unset PATH environment variable + case "PATH": + path = strings.SplitN(e, "=", 2)[1] + default: + next = append(next, e) + } + } + if path != "" { + next = append(next, fmt.Sprintf("PATH=%s:%s", path, defaults.EmbeddedClusterBinsSubDir())) + } else { + next = append(next, fmt.Sprintf("PATH=%s", defaults.EmbeddedClusterBinsSubDir())) } - return env + return next } diff --git a/pkg/preflights/preflights_test.go b/pkg/preflights/preflights_test.go index f7fcfc2d0..8b6441d30 100644 --- a/pkg/preflights/preflights_test.go +++ b/pkg/preflights/preflights_test.go @@ -2,116 +2,188 @@ package preflights import ( + "os" "strings" "testing" ecv1beta1 "github.com/replicatedhq/embedded-cluster-kinds/apis/v1beta1" + "github.com/replicatedhq/embedded-cluster/pkg/defaults" "github.com/stretchr/testify/assert" ) func Test_proxyEnv(t *testing.T) { type args struct { + env []string proxy *ecv1beta1.ProxySpec } tests := []struct { - name string - env map[string]string - args args - wantHTTPProxy string - wantHTTPSProxy string - wantNoProxy string + name string + args args + want []string }{ { name: "no proxy nil", args: args{ + env: []string{ + "TEST=test", + }, proxy: nil, }, - wantHTTPProxy: "", - wantHTTPSProxy: "", - wantNoProxy: "", + want: []string{ + "TEST=test", + }, }, { - name: "no proxy nil", + name: "no proxy empty", args: args{ + env: []string{ + "TEST=test", + }, proxy: &ecv1beta1.ProxySpec{}, }, - wantHTTPProxy: "", - wantHTTPSProxy: "", - wantNoProxy: "", + want: []string{ + "TEST=test", + "HTTP_PROXY=", + "HTTPS_PROXY=", + "NO_PROXY=", + }, }, { name: "no proxy unset env", - env: map[string]string{ - "HTTP_PROXY": "http://proxy:8080", - "HTTPS_PROXY": "https://proxy:8080", - "NO_PROXY": "localhost", - "http_proxy": "http://proxy:8080", - "https_proxy": "https://proxy:8080", - "no_proxy": "localhost", - }, args: args{ + env: []string{ + "TEST=test", + "HTTP_PROXY=http://proxy:8080", + "HTTPS_PROXY=https://proxy:8080", + "NO_PROXY=localhost", + "http_proxy=http://proxy:8080", + "https_proxy=https://proxy:8080", + "no_proxy=localhost", + }, proxy: &ecv1beta1.ProxySpec{}, }, - wantHTTPProxy: "", - wantHTTPSProxy: "", - wantNoProxy: "", + want: []string{ + "TEST=test", + "HTTP_PROXY=", + "HTTPS_PROXY=", + "NO_PROXY=", + }, }, { name: "no proxy set env", args: args{ + env: []string{ + "TEST=test", + }, proxy: &ecv1beta1.ProxySpec{ HTTPProxy: "http://proxy:8080", HTTPSProxy: "https://proxy:8080", NoProxy: "localhost", }, }, - wantHTTPProxy: "http://proxy:8080", - wantHTTPSProxy: "https://proxy:8080", - wantNoProxy: "localhost", + want: []string{ + "TEST=test", + "HTTP_PROXY=http://proxy:8080", + "HTTPS_PROXY=https://proxy:8080", + "NO_PROXY=localhost", + }, }, { name: "proxy override env", - env: map[string]string{ - "HTTP_PROXY": "http://proxy:8080", - "HTTPS_PROXY": "https://proxy:8080", - "NO_PROXY": "localhost", - "http_proxy": "http://proxy:8080", - "https_proxy": "https://proxy:8080", - "no_proxy": "localhost", - }, args: args{ + env: []string{ + "TEST=test", + "HTTP_PROXY=http://proxy:8080", + "HTTPS_PROXY=https://proxy:8080", + "NO_PROXY=localhost", + "http_proxy=http://proxy:8080", + "https_proxy=https://proxy:8080", + "no_proxy=localhost", + }, proxy: &ecv1beta1.ProxySpec{ HTTPProxy: "http://proxy2:8080", HTTPSProxy: "https://proxy2:8080", NoProxy: "localhost2", }, }, - wantHTTPProxy: "http://proxy2:8080", - wantHTTPSProxy: "https://proxy2:8080", - wantNoProxy: "localhost2", + want: []string{ + "TEST=test", + "HTTP_PROXY=http://proxy2:8080", + "HTTPS_PROXY=https://proxy2:8080", + "NO_PROXY=localhost2", + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Unset proxy environment variables - for _, k := range []string{"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy"} { - t.Setenv(k, "") - } - for k, v := range tt.env { - t.Setenv(k, v) + got := proxyEnv(tt.args.env, tt.args.proxy) + gotMap := make(map[string]string) + for _, e := range got { + parts := strings.SplitN(e, "=", 2) + gotMap[parts[0]] = parts[1] } - got := proxyEnv(tt.args.proxy) + assert.Equal(t, tt.want, got) + }) + } +} + +func Test_pathEnv(t *testing.T) { + dir, err := os.MkdirTemp("", "embedded-cluster") + if err != nil { + t.Fatal(err) + } + + oDefaultProvider := defaults.DefaultProvider + defaults.DefaultProvider = defaults.NewProvider(dir) + t.Cleanup(func() { + defaults.DefaultProvider = oDefaultProvider + }) + + binDir := defaults.DefaultProvider.EmbeddedClusterBinsSubDir() + + type args struct { + env []string + } + tests := []struct { + name string + args args + want []string + }{ + { + name: "path empty", + args: args{ + env: []string{ + "TEST=test", + }, + }, + want: []string{ + "TEST=test", + "PATH=" + binDir, + }, + }, + { + name: "path override", + args: args{ + env: []string{ + "TEST=test", + "PATH=/usr/bin:/usr/local/bin", + }, + }, + want: []string{ + "TEST=test", + "PATH=/usr/bin:/usr/local/bin:" + binDir, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := pathEnv(tt.args.env) gotMap := make(map[string]string) for _, e := range got { parts := strings.SplitN(e, "=", 2) gotMap[parts[0]] = parts[1] } - assert.Equal(t, tt.wantHTTPProxy, gotMap["HTTP_PROXY"]) - assert.Equal(t, tt.wantHTTPSProxy, gotMap["HTTPS_PROXY"]) - assert.Equal(t, tt.wantNoProxy, gotMap["NO_PROXY"]) - assert.Empty(t, gotMap["http_proxy"]) - assert.Empty(t, gotMap["https_proxy"]) - assert.Empty(t, gotMap["no_proxy"]) + assert.Equal(t, tt.want, got) }) } }