fix TestLocalArtifactMirror and TestCommandsRequireSudo

cmd/installer/cli/install2.go

@@ -157,10 +157,9 @@ func addInstallAdminConsoleFlags(cmd *cobra.Command, flags *Install2CmdFlags) er
 	cmd.Flags().StringVar(&flags.adminConsolePassword, "admin-console-password", "", "Password for the Admin Console")
 	cmd.Flags().IntVar(&flags.adminConsolePort, "admin-console-port", ecv1beta1.DefaultAdminConsolePort, "Port on which the Admin Console will be served")
 	cmd.Flags().StringVarP(&flags.licenseFile, "license", "l", "", "Path to the license file")
-	// TODO: uncomment this when we have tests passing
-	// if err := cmd.MarkFlagRequired("license"); err != nil {
-	// 	panic(err)
-	// }
+	if err := cmd.MarkFlagRequired("license"); err != nil {
+		panic(err)
+	}
 	cmd.Flags().StringVar(&flags.configValues, "config-values", "", "Path to the config values to use when installing")
 
 	return nil

cmd/installer/cli/metrics.go

@@ -36,7 +36,7 @@ func (r *InstallReporter) ReportInstallationFailed(ctx context.Context, err erro
 }
 
 func (r *InstallReporter) ReportPreflightsFailed(ctx context.Context, output preflightstypes.Output, bypassed bool) {
-	metrics.ReportPreflightsFailed(ctx, r.license.Spec.Endpoint, r.clusterID, output, bypassed, r.cmd)
+	metrics.ReportPreflightsFailed(ctx, metrics.BaseURL(r.license), r.clusterID, output, bypassed, r.cmd)
 }
 
 type JoinReporter struct {

cmd/installer/cli/node.go

@@ -13,10 +13,11 @@ func NodeCmd(ctx context.Context, name string) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return nil
 		},
+		Hidden: true,
 	}
 
 	// here for legacy reasons
-	joinCmd := JoinCmd(ctx, name)
+	joinCmd := Join2Cmd(ctx, name)
 	joinCmd.Hidden = true
 	cmd.AddCommand(joinCmd)
 

e2e/local-artifact-mirror_test.go

@@ -1,6 +1,7 @@
 package e2e
 
 import (
+	"os"
 	"strings"
 	"testing"
 	"time"
@@ -18,12 +19,12 @@ func TestLocalArtifactMirror(t *testing.T) {
 		Nodes:        1,
 		Distro:       "debian-bookworm",
 		LicensePath:  "license.yaml",
-		ECBinaryPath: "../output/bin/embedded-cluster-original",
+		ECBinaryPath: "../output/bin/embedded-cluster",
 	})
 	defer tc.Cleanup()
 
 	t.Logf("%s: installing embedded-cluster on node 0", time.Now().Format(time.RFC3339))
-	line := []string{"default-install.sh", "--local-artifact-mirror-port", "50001"}
+	line := []string{"single-node-install.sh", "ui", os.Getenv("SHORT_SHA"), "--local-artifact-mirror-port", "50001"}
 	if stdout, stderr, err := tc.RunCommandOnNode(0, line); err != nil {
 		t.Fatalf("fail to install embedded-cluster on node 0: %v: %s: %s", err, stdout, stderr)
 	}

e2e/scripts/embedded-preflight.sh

@@ -132,7 +132,7 @@ has_applied_host_preflight() {
 
 main() {
     embed_preflight "$preflight_with_failure"
-    if /usr/local/bin/embedded-cluster install --yes 2>&1 | tee /tmp/log ; then
+    if /usr/local/bin/embedded-cluster install --yes --license /assets/license.yaml 2>&1 | tee /tmp/log ; then
         cat /tmp/log
         echo "preflight_with_failure: Expected installation to fail"
         exit 1
@@ -156,7 +156,7 @@ main() {
         echo "preflight_with_warning: Failed to run embedded-cluster preflights"
         exit 1
     fi
-    if ! /usr/local/bin/embedded-cluster install --yes 2>&1 | tee /tmp/log ; then
+    if ! /usr/local/bin/embedded-cluster install --yes --license /assets/license.yaml 2>&1 | tee /tmp/log ; then
         cat /etc/os-release
         echo "preflight_with_warning: Failed to install embedded-cluster"
         exit 1

e2e/sudo_test.go

@@ -15,32 +15,45 @@ func TestCommandsRequireSudo(t *testing.T) {
 		Nodes:               1,
 		CreateRegularUser:   true,
 		Image:               "debian/12",
+		LicensePath:         "license.yaml",
 		EmbeddedClusterPath: "../output/bin/embedded-cluster",
 	})
 	defer tc.Cleanup()
 	t.Logf(`%s: running "embedded-cluster version" as regular user`, time.Now().Format(time.RFC3339))
 	command := []string{"embedded-cluster", "version"}
-	if _, _, err := tc.RunRegularUserCommandOnNode(t, 0, command); err != nil {
+	stdout, _, err := tc.RunRegularUserCommandOnNode(t, 0, command)
+	if err != nil {
 		t.Errorf("expected no error running `version` as regular user, got %v", err)
 	}
+	t.Logf("version output:\n%s", stdout)
+
+	gotFailure := false
 	for _, cmd := range [][]string{
 		{"embedded-cluster", "node", "join", "https://test", "token"},
 		{"embedded-cluster", "join", "https://test", "token"},
 		{"embedded-cluster", "reset", "--force"},
 		{"embedded-cluster", "node", "reset", "--force"},
 		{"embedded-cluster", "shell"},
-		{"embedded-cluster", "install", "--yes"},
+		{"embedded-cluster", "install", "--yes", "--license", "/assets/license.yaml"},
 		{"embedded-cluster", "restore"},
 	} {
-		t.Logf("%s: running %q as regular user", time.Now().Format(time.RFC3339), strings.Join(cmd, "_"))
+		t.Logf("%s: running %q as regular user", time.Now().Format(time.RFC3339), "'"+strings.Join(cmd, " ")+"'")
 		stdout, stderr, err := tc.RunRegularUserCommandOnNode(t, 0, cmd)
 		if err == nil {
-			t.Fatalf("expected error running `%v` as regular user, got none", cmd)
+			t.Logf("stdout:\n%s\nstderr:%s\n", stdout, stderr)
+			t.Logf("expected error running `%v` as regular user, got none", cmd)
+			gotFailure = true
+			continue
 		}
 		if !strings.Contains(stderr, "command must be run as root") {
 			t.Logf("stdout:\n%s\nstderr:%s\n", stdout, stderr)
-			t.Fatalf("invalid error found running `%v` as regular user", cmd)
+			t.Logf("invalid error found running `%v` as regular user", cmd)
+			gotFailure = true
+			continue
 		}
 	}
+	if gotFailure {
+		t.Fatalf("at least one command did not fail as regular user")
+	}
 	t.Logf("%s: test complete", time.Now().Format(time.RFC3339))
 }

operator/pkg/cli/migratev2/installation_test.go

@@ -3,6 +3,7 @@ package migratev2
 import (
 	"context"
 	"fmt"
+	"log/slog"
 	"testing"
 
 	ecv1beta1 "github.com/replicatedhq/embedded-cluster/kinds/apis/v1beta1"
@@ -15,6 +16,10 @@ import (
 )
 
 func Test_setV2MigrationInProgress(t *testing.T) {
+	// Discard log messages
+	old := slog.SetLogLoggerLevel(slog.LevelError)
+	defer slog.SetLogLoggerLevel(old)
+
 	scheme := runtime.NewScheme()
 	require.NoError(t, ecv1beta1.AddToScheme(scheme))
 
@@ -76,10 +81,7 @@ func Test_setV2MigrationInProgress(t *testing.T) {
 				WithStatusSubresource(&ecv1beta1.Installation{}).
 				Build()
 
-			// Discard log messages
-			logf := func(format string, args ...any) {}
-
-			err := setV2MigrationInProgress(context.Background(), logf, cli, tt.args.installation)
+			err := setV2MigrationInProgress(context.Background(), cli, tt.args.installation)
 			require.NoError(t, err)
 
 			// Verify that the condition was set correctly
@@ -97,6 +99,10 @@ func Test_setV2MigrationInProgress(t *testing.T) {
 }
 
 func Test_setV2MigrationComplete(t *testing.T) {
+	// Discard log messages
+	old := slog.SetLogLoggerLevel(slog.LevelError)
+	defer slog.SetLogLoggerLevel(old)
+
 	scheme := runtime.NewScheme()
 	require.NoError(t, ecv1beta1.AddToScheme(scheme))
 
@@ -156,11 +162,7 @@ func Test_setV2MigrationComplete(t *testing.T) {
 				WithObjects(tt.args.installation).
 				WithStatusSubresource(&ecv1beta1.Installation{}).
 				Build()
-
-			// Discard log messages
-			logf := func(format string, args ...any) {}
-
-			err := setV2MigrationComplete(context.Background(), logf, cli, tt.args.installation)
+			err := setV2MigrationComplete(context.Background(), cli, tt.args.installation)
 			require.NoError(t, err)
 
 			// Verify that the condition was set correctly
@@ -178,6 +180,10 @@ func Test_setV2MigrationComplete(t *testing.T) {
 }
 
 func Test_setV2MigrationFailed(t *testing.T) {
+	// Discard log messages
+	old := slog.SetLogLoggerLevel(slog.LevelError)
+	defer slog.SetLogLoggerLevel(old)
+
 	scheme := runtime.NewScheme()
 	require.NoError(t, ecv1beta1.AddToScheme(scheme))
 
@@ -241,10 +247,7 @@ func Test_setV2MigrationFailed(t *testing.T) {
 				WithStatusSubresource(&ecv1beta1.Installation{}).
 				Build()
 
-			// Discard log messages
-			logf := func(format string, args ...any) {}
-
-			err := setV2MigrationFailed(context.Background(), logf, cli, tt.args.installation, tt.args.failure)
+			err := setV2MigrationFailed(context.Background(), cli, tt.args.installation, tt.args.failure)
 			require.NoError(t, err)
 
 			// Verify that the condition was set correctly

pkg/spinner/spinner.go

@@ -3,12 +3,17 @@ package spinner
 
 import (
 	"fmt"
+	"os"
 	"strings"
 	"time"
+
+	"github.com/mattn/go-isatty"
 )
 
 var blocks = []string{"◐", "◓", "◑", "◒"}
 
+var hasTTY = isatty.IsTerminal(os.Stdout.Fd())
+
 // WriteFn is a function that writes a formatted string.
 type WriteFn func(string, ...any) (int, error)
 
@@ -29,6 +34,7 @@ type MessageWriter struct {
 	printf WriteFn
 	mask   MaskFn
 	lbreak LineBreakerFn
+	tty    bool
 }
 
 // Write implements io.Writer for the MessageWriter.
@@ -95,6 +101,7 @@ func (m *MessageWriter) loop() {
 	var message string
 	var ticker = time.NewTicker(50 * time.Millisecond)
 	var end bool
+	var changed bool
 	for {
 		previous := message
 
@@ -113,27 +120,41 @@ func (m *MessageWriter) loop() {
 			message = m.mask(message)
 		}
 
-		if m.lbreak != nil && previous != message {
+		changed = previous != message
+
+		if m.lbreak != nil && changed {
 			if lbreak, lcontent := m.lbreak(message); lbreak {
 				if diff := len(previous) - len(lcontent); diff > 0 {
 					suffix := strings.Repeat(" ", diff)
 					lcontent = fmt.Sprintf("%s%s", lcontent, suffix)
 				}
-				m.printf("\033[K\r✔  %s\n", lcontent)
+				if m.tty {
+					m.printf("\033[K\r✔  %s\n", lcontent)
+				} else {
+					m.printf("✔  %s\n", lcontent)
+				}
 			}
 		}
 
 		pos := counter % len(blocks)
 		if !end {
-			m.printf("\033[K\r%s  %s", blocks[pos], message)
+			if m.tty {
+				m.printf("\033[K\r%s  %s", blocks[pos], message)
+			} else if changed {
+				m.printf("○  %s\n", message)
+			}
 			continue
 		}
 
 		prefix := "✔"
 		if m.err {
 			prefix = "✗"
 		}
-		m.printf("\033[K\r%s  %s\n", prefix, message)
+		if m.tty {
+			m.printf("\033[K\r%s  %s\n", prefix, message)
+		} else {
+			m.printf("%s  %s\n", prefix, message)
+		}
 		close(m.end)
 		return
 	}
@@ -145,6 +166,7 @@ func Start(opts ...Option) *MessageWriter {
 		ch:     make(chan string, 1024),
 		end:    make(chan struct{}),
 		printf: fmt.Printf,
+		tty:    hasTTY,
 	}
 	for _, opt := range opts {
 		opt(mw)