Revert "feat(troubleshoot): add filesystem latency check" (#1024)

Revert "feat(troubleshoot): add filesystem latency check (#1018)"

This reverts commit 7f7de2b.

Author: sgalsaleh

Makefile

@@ -36,7 +36,6 @@ LD_FLAGS = \
 	-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) \
 	-X github.com/replicatedhq/embedded-cluster/pkg/addons/embeddedclusteroperator.EmbeddedOperatorImageOverride=$(EMBEDDED_OPERATOR_IMAGE_OVERRIDE)
-DISABLE_FIO_BUILD ?= 0
 
 export PATH := $(shell pwd)/bin:$(PATH)
 
@@ -77,16 +76,6 @@ 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
@@ -121,7 +110,6 @@ 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

cmd/embedded-cluster/install.go

@@ -119,14 +119,11 @@ func runHostPreflights(c *cli.Context, hpf *v1beta2.HostPreflightSpec, proxy *ec
 		return nil
 	}
 	pb.Infof("Running host preflights")
-	output, stderr, err := preflights.Run(c.Context, hpf, proxy)
+	output, 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 {

e2e/materialize_test.go

@@ -21,12 +21,10 @@ 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)

fio/Dockerfile

@@ -3,11 +3,6 @@
 // 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
@@ -16,88 +11,97 @@ var (
 // Holds the default no proxy values.
 var DefaultNoProxy = []string{"localhost", "127.0.0.1", ".default", ".local", ".svc", "kubernetes", "kotsadm-rqlite"}
 
-const ProxyRegistryAddress = "proxy.replicated.com"
+var 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 DefaultProvider.BinaryName()
+	return def().BinaryName()
 }
 
 // EmbeddedClusterBinsSubDir calls EmbeddedClusterBinsSubDir on the default provider.
 func EmbeddedClusterBinsSubDir() string {
-	return DefaultProvider.EmbeddedClusterBinsSubDir()
+	return def().EmbeddedClusterBinsSubDir()
 }
 
 // EmbeddedClusterChartsSubDir calls EmbeddedClusterChartsSubDir on the default provider.
 func EmbeddedClusterChartsSubDir() string {
-	return DefaultProvider.EmbeddedClusterChartsSubDir()
+	return def().EmbeddedClusterChartsSubDir()
 }
 
 // EmbeddedClusterImagesSubDir calls EmbeddedClusterImagesSubDir on the default provider.
 func EmbeddedClusterImagesSubDir() string {
-	return DefaultProvider.EmbeddedClusterImagesSubDir()
+	return def().EmbeddedClusterImagesSubDir()
 }
 
 // EmbeddedClusterLogsSubDir calls EmbeddedClusterLogsSubDir on the default provider.
 func EmbeddedClusterLogsSubDir() string {
-	return DefaultProvider.EmbeddedClusterLogsSubDir()
+	return def().EmbeddedClusterLogsSubDir()
 }
 
 // K0sBinaryPath calls K0sBinaryPath on the default provider.
 func K0sBinaryPath() string {
-	return DefaultProvider.K0sBinaryPath()
+	return def().K0sBinaryPath()
 }
 
 // PathToEmbeddedClusterBinary calls PathToEmbeddedClusterBinary on the default provider.
 func PathToEmbeddedClusterBinary(name string) string {
-	return DefaultProvider.PathToEmbeddedClusterBinary(name)
+	return def().PathToEmbeddedClusterBinary(name)
 }
 
 // PathToLog calls PathToLog on the default provider.
 func PathToLog(name string) string {
-	return DefaultProvider.PathToLog(name)
+	return def().PathToLog(name)
 }
 
 // PathToKubeConfig calls PathToKubeConfig on the default provider.
 func PathToKubeConfig() string {
-	return DefaultProvider.PathToKubeConfig()
+	return def().PathToKubeConfig()
 }
 
 // PreferredNodeIPAddress calls PreferredNodeIPAddress on the default provider.
 func PreferredNodeIPAddress() (string, error) {
-	return DefaultProvider.PreferredNodeIPAddress()
+	return def().PreferredNodeIPAddress()
 }
 
 // TryDiscoverPublicIP calls TryDiscoverPublicIP on the default provider.
 func TryDiscoverPublicIP() string {
-	return DefaultProvider.TryDiscoverPublicIP()
+	return def().TryDiscoverPublicIP()
 }
 
 // PathToK0sConfig calls PathToK0sConfig on the default provider.
 func PathToK0sConfig() string {
-	return DefaultProvider.PathToK0sConfig()
+	return def().PathToK0sConfig()
 }
 
 // PathToK0sStatusSocket calls PathToK0sStatusSocket on the default provider.
 func PathToK0sStatusSocket() string {
-	return DefaultProvider.PathToK0sStatusSocket()
+	return def().PathToK0sStatusSocket()
 }
 
 func PathToK0sContainerdConfig() string {
-	return DefaultProvider.PathToK0sContainerdConfig()
+	return def().PathToK0sContainerdConfig()
 }
 
 // EmbeddedClusterHomeDirectory calls EmbeddedClusterHomeDirectory on the default provider.
 func EmbeddedClusterHomeDirectory() string {
-	return DefaultProvider.EmbeddedClusterHomeDirectory()
+	return def().EmbeddedClusterHomeDirectory()
 }
 
 // PathToEmbeddedClusterSupportFile calls PathToEmbeddedClusterSupportFile on the default provider.
 func PathToEmbeddedClusterSupportFile(name string) string {
-	return DefaultProvider.PathToEmbeddedClusterSupportFile(name)
+	return def().PathToEmbeddedClusterSupportFile(name)
 }

pkg/defaults/defaults.go

@@ -63,19 +63,6 @@ spec:
         collectorName: resolv.conf
         command: 'sh'
         args: ['-c', 'cat /etc/resolv.conf']
-    - filesystemPerformance:
-        collectorName: filesystem-write-latency-etcd
-        timeout: 2m
-        directory: /var/lib/k0s/etcd
-        fileSize: 22Mi
-        operationSizeBytes: 2300
-        datasync: true
-        enableBackgroundIOPS: true
-        backgroundIOPSWarmupSeconds: 10
-        backgroundWriteIOPS: 300
-        backgroundWriteIOPSJobs: 6
-        backgroundReadIOPS: 50
-        backgroundReadIOPSJobs: 1
   analyzers:
     - cpu:
         checkName: CPU
@@ -380,12 +367,3 @@ spec:
           - pass:
               when: "false"
               message: "resolv.conf does not reference '127.0.0.1'"
-    - filesystemPerformance:
-        checkName: Filesystem Write Latency
-        collectorName: filesystem-write-latency-etcd
-        outcomes:
-          - pass:
-              when: "p99 < 10ms"
-              message: 'Write latency is ok (p99 target < 10ms, actual: {{ "{{" }} .P99 {{ "}}" }})'
-          - fail:
-              message: 'Write latency is high (p99 target < 10ms, actual: {{ "{{" }} .String {{ "}}" }})'

pkg/preflights/host-preflight.yaml

@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"strings"
@@ -31,34 +32,29 @@ 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, string, error) {
+func Run(ctx context.Context, spec *troubleshootv1beta2.HostPreflightSpec, proxy *ecv1beta1.ProxySpec) (*Output, 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 = os.Environ()
-	cmd.Env = proxyEnv(cmd.Env, proxy)
-	cmd.Env = pathEnv(cmd.Env)
-	cmd.Stdout, cmd.Stderr = stdout, stderr
+	cmd.Env = proxyEnv(proxy)
+	cmd.Stdout, cmd.Stderr = stdout, io.Discard
 	if err = cmd.Run(); err == nil {
-		out, err := OutputFromReader(stdout)
-		return out, stderr.String(), err
+		return OutputFromReader(stdout)
 	}
 	var exit *exec.ExitError
 	if !errors.As(err, &exit) || exit.ExitCode() < 2 {
-		return nil, stderr.String(), fmt.Errorf("error running host preflight: %w, stderr=%q", err, stderr.String())
+		return nil, fmt.Errorf("unknown error running host preflight: %w", err)
 	}
-	out, err := OutputFromReader(stdout)
-	return out, stderr.String(), err
+	return OutputFromReader(stdout)
 }
 
 // saveHostPreflightFile saves the provided spec to a temporary file and returns
@@ -101,40 +97,20 @@ func dedup[T any](objs []T) []T {
 	return out
 }
 
-func proxyEnv(env []string, proxy *ecv1beta1.ProxySpec) []string {
-	next := []string{}
-	for _, e := range env {
+func proxyEnv(proxy *ecv1beta1.ProxySpec) []string {
+	env := []string{}
+	for _, e := range os.Environ() {
 		switch strings.SplitN(e, "=", 2)[0] {
 		// Unset proxy environment variables
 		case "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy":
-		default:
-			next = append(next, e)
+			continue
 		}
+		env = append(env, e)
 	}
 	if proxy != nil {
-		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()))
+		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))
 	}
-	return next
+	return env
 }