From 778aa98ceedf5221e1c627104d2cb36b3d7e0bcd Mon Sep 17 00:00:00 2001 From: Ethan Mosbaugh Date: Fri, 31 Jan 2025 08:49:02 -0800 Subject: [PATCH 01/12] chore: no spinner if no terminal (#1778) --- pkg/spinner/spinner.go | 30 +++++++++++++--- pkg/spinner/spinner_test.go | 68 ++++++++++++++++++++++++------------- 2 files changed, 70 insertions(+), 28 deletions(-) diff --git a/pkg/spinner/spinner.go b/pkg/spinner/spinner.go index 84bf354d9..fefccb642 100644 --- a/pkg/spinner/spinner.go +++ b/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,19 +120,29 @@ 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 } @@ -133,7 +150,11 @@ func (m *MessageWriter) loop() { 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) diff --git a/pkg/spinner/spinner_test.go b/pkg/spinner/spinner_test.go index d25cf772a..8d4cc8673 100644 --- a/pkg/spinner/spinner_test.go +++ b/pkg/spinner/spinner_test.go @@ -11,7 +11,22 @@ import ( "github.com/stretchr/testify/assert" ) -func WriteTo(to io.Writer) WriteFn { +func startTest(opts ...Option) (*MessageWriter, *bytes.Buffer) { + buf := bytes.NewBuffer(nil) + opts = append( + []Option{ + WithWriter(writeTo(buf)), + func(m *MessageWriter) { + m.tty = true + }, + }, + opts..., + ) + pb := Start(opts...) + return pb, buf +} + +func writeTo(to io.Writer) WriteFn { return func(format string, args ...interface{}) (int, error) { fmt.Fprintf(to, format, args...) return 0, nil @@ -19,8 +34,7 @@ func WriteTo(to io.Writer) WriteFn { } func TestStartAndClosef(t *testing.T) { - buf := bytes.NewBuffer(nil) - pb := Start(WithWriter(WriteTo(buf))) + pb, buf := startTest() pb.Infof("hello") time.Sleep(time.Second) pb.Closef("closing with this value") @@ -28,16 +42,14 @@ func TestStartAndClosef(t *testing.T) { } func TestStartAndClose(t *testing.T) { - buf := bytes.NewBuffer(nil) - pb := Start(WithWriter(WriteTo(buf))) + pb, buf := startTest() pb.Infof("hello") pb.Close() assert.Contains(t, buf.String(), "hello") } func TestStartAndWrite(t *testing.T) { - buf := bytes.NewBuffer(nil) - pb := Start(WithWriter(WriteTo(buf))) + pb, buf := startTest() pb.Infof("hello") _, err := pb.Write([]byte("world")) assert.NoError(t, err) @@ -46,40 +58,35 @@ func TestStartAndWrite(t *testing.T) { } func TestStartAndTracef(t *testing.T) { - buf := bytes.NewBuffer(nil) - pb := Start(WithWriter(WriteTo(buf))) + pb, buf := startTest() pb.Tracef("tracef") pb.Close() assert.Contains(t, buf.String(), "tracef") } func TestStartAndDebugf(t *testing.T) { - buf := bytes.NewBuffer(nil) - pb := Start(WithWriter(WriteTo(buf))) + pb, buf := startTest() pb.Debugf("debugf") pb.Close() assert.Contains(t, buf.String(), "debugf") } func TestStartAndWarnf(t *testing.T) { - buf := bytes.NewBuffer(nil) - pb := Start(WithWriter(WriteTo(buf))) + pb, buf := startTest() pb.Warnf("warnf") pb.Close() assert.Contains(t, buf.String(), "warnf") } func TestStartAndErrorf(t *testing.T) { - buf := bytes.NewBuffer(nil) - pb := Start(WithWriter(WriteTo(buf))) + pb, buf := startTest() pb.Errorf("errorf") pb.Close() assert.Contains(t, buf.String(), "errorf") } func TestStartAndCloseWithError(t *testing.T) { - buf := bytes.NewBuffer(nil) - pb := Start(WithWriter(WriteTo(buf))) + pb, buf := startTest() for i := 0; i < 1000; i++ { pb.Infof("test nr %d", i) } @@ -88,7 +95,6 @@ func TestStartAndCloseWithError(t *testing.T) { } func TestMask(t *testing.T) { - buf := bytes.NewBuffer(nil) maskfn := func(s string) string { if s == "test 0" { return "masked 0" @@ -98,8 +104,7 @@ func TestMask(t *testing.T) { } return s } - pb := Start( - WithWriter(WriteTo(buf)), + pb, buf := startTest( WithMask(maskfn), ) pb.Infof("test 0") @@ -110,7 +115,6 @@ func TestMask(t *testing.T) { } func TestLineBreak(t *testing.T) { - buf := bytes.NewBuffer(nil) lbreak := func(s string) (bool, string) { if s == "test 3" { return true, "ping 2" @@ -120,8 +124,7 @@ func TestLineBreak(t *testing.T) { } return false, "" } - pb := Start( - WithWriter(WriteTo(buf)), + pb, buf := startTest( WithLineBreaker(lbreak), ) for i := 0; i < 100; i++ { @@ -132,8 +135,25 @@ func TestLineBreak(t *testing.T) { // ✔ ping 2 (\n) // ✔ ping 7 (\n) // ✔ test 99 (\n) - assert.Equal(t, strings.Count(buf.String(), "\n"), 3) + assert.Equal(t, 3, strings.Count(buf.String(), "\n")) assert.Contains(t, buf.String(), "ping 2") assert.Contains(t, buf.String(), "ping 7") assert.Contains(t, buf.String(), "test 99") } + +func TestNoTTY(t *testing.T) { + pb, buf := startTest( + func(m *MessageWriter) { + m.tty = false + }, + ) + + pb.Infof("Installing") + time.Sleep(time.Second) + pb.Infof("Waiting") + time.Sleep(time.Second) + pb.Infof("Done") + pb.Close() + + assert.Equal(t, "○ Installing\n○ Waiting\n○ Done\n✔ Done\n", buf.String()) +} From 6539fed97c19d01347666315412018616dffd847 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 12:14:07 -0500 Subject: [PATCH 02/12] fix panic in TestHostPreflightCustomSpec --- cmd/installer/cli/metrics.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/installer/cli/metrics.go b/cmd/installer/cli/metrics.go index 153a0916d..7283a6bce 100644 --- a/cmd/installer/cli/metrics.go +++ b/cmd/installer/cli/metrics.go @@ -36,7 +36,9 @@ 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) + if r.license != nil && r.license.Spec.Endpoint != "" { + metrics.ReportPreflightsFailed(ctx, r.license.Spec.Endpoint, r.clusterID, output, bypassed, r.cmd) + } } type JoinReporter struct { From b452e2efe0247cfb92a05349f9c23ccac768479a Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 12:50:54 -0500 Subject: [PATCH 03/12] fix compile --- operator/pkg/cli/migrate_v2.go | 4 +-- .../pkg/cli/migratev2/installation_test.go | 29 ++++++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/operator/pkg/cli/migrate_v2.go b/operator/pkg/cli/migrate_v2.go index 778d2d00b..a26280543 100644 --- a/operator/pkg/cli/migrate_v2.go +++ b/operator/pkg/cli/migrate_v2.go @@ -2,8 +2,6 @@ package cli import ( "fmt" - "log" - ecv1beta1 "github.com/replicatedhq/embedded-cluster/kinds/apis/v1beta1" "github.com/replicatedhq/embedded-cluster/operator/pkg/cli/migratev2" "github.com/replicatedhq/embedded-cluster/operator/pkg/k8sutil" @@ -42,7 +40,7 @@ func MigrateV2Cmd() *cobra.Command { return fmt.Errorf("failed to create kubernetes client: %w", err) } - err = migratev2.Run(ctx, log.Printf, cli, installation) + err = migratev2.Run(ctx, cli, installation) if err != nil { return fmt.Errorf("failed to run v2 migration: %w", err) } diff --git a/operator/pkg/cli/migratev2/installation_test.go b/operator/pkg/cli/migratev2/installation_test.go index 6c53a98c2..6cc451df3 100644 --- a/operator/pkg/cli/migratev2/installation_test.go +++ b/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 From a3cd01d0bfd1a53792f9dad86cbe2ebc7ee89473 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 13:33:05 -0500 Subject: [PATCH 04/12] require license for install2 --- cmd/installer/cli/install2.go | 10 ++++++---- cmd/installer/cli/install_runpreflights.go | 2 +- cmd/installer/cli/restore.go | 2 +- cmd/installer/cli/restore2.go | 2 +- e2e/scripts/embedded-preflight.sh | 2 +- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cmd/installer/cli/install2.go b/cmd/installer/cli/install2.go index 39e516c7d..82a4cbda7 100644 --- a/cmd/installer/cli/install2.go +++ b/cmd/installer/cli/install2.go @@ -83,10 +83,10 @@ func Install2Cmd(ctx context.Context, name string) *cobra.Command { var flags Install2CmdFlags cmd := &cobra.Command{ - Use: "install", - Short: fmt.Sprintf("Experimental installer for %s", name), + Use: "install", + Short: fmt.Sprintf("Experimental installer for %s", name), PreRunE: func(cmd *cobra.Command, args []string) error { - if err := preRunInstall2(cmd, &flags); err != nil { + if err := preRunInstall2(cmd, &flags, true); err != nil { return err } @@ -167,7 +167,7 @@ func addInstallAdminConsoleFlags(cmd *cobra.Command, flags *Install2CmdFlags) er return nil } -func preRunInstall2(cmd *cobra.Command, flags *Install2CmdFlags) error { +func preRunInstall2(cmd *cobra.Command, flags *Install2CmdFlags, requireLicense bool) error { if os.Getuid() != 0 { return fmt.Errorf("install command must be run as root") } @@ -212,6 +212,8 @@ func preRunInstall2(cmd *cobra.Command, flags *Install2CmdFlags) error { return fmt.Errorf("unable to parse license file: %w", err) } flags.license = l + } else if requireLicense { + return fmt.Errorf("license is required") } runtimeconfig.ApplyFlags(cmd.Flags()) diff --git a/cmd/installer/cli/install_runpreflights.go b/cmd/installer/cli/install_runpreflights.go index 7244528a4..6c46d1b0d 100644 --- a/cmd/installer/cli/install_runpreflights.go +++ b/cmd/installer/cli/install_runpreflights.go @@ -19,7 +19,7 @@ func InstallRunPreflightsCmd(ctx context.Context, name string) *cobra.Command { Use: "run-preflights", Short: "Run install host preflights", PreRunE: func(cmd *cobra.Command, args []string) error { - if err := preRunInstall2(cmd, &flags); err != nil { + if err := preRunInstall2(cmd, &flags, false); err != nil { return err } diff --git a/cmd/installer/cli/restore.go b/cmd/installer/cli/restore.go index 1e57518a5..b1c31fb0d 100644 --- a/cmd/installer/cli/restore.go +++ b/cmd/installer/cli/restore.go @@ -31,7 +31,7 @@ func RestoreCmd(ctx context.Context, name string) *cobra.Command { Use: "restore", Short: fmt.Sprintf("Restore a %s cluster", name), PreRunE: func(cmd *cobra.Command, args []string) error { - if err := preRunInstall2(cmd, &flags); err != nil { + if err := preRunInstall2(cmd, &flags, false); err != nil { return err } diff --git a/cmd/installer/cli/restore2.go b/cmd/installer/cli/restore2.go index af032f8f3..2926f5afa 100644 --- a/cmd/installer/cli/restore2.go +++ b/cmd/installer/cli/restore2.go @@ -92,7 +92,7 @@ func Restore2Cmd(ctx context.Context, name string) *cobra.Command { Use: "restore2", Short: fmt.Sprintf("Restore a %s cluster", name), PreRunE: func(cmd *cobra.Command, args []string) error { - if err := preRunInstall2(cmd, &flags); err != nil { + if err := preRunInstall2(cmd, &flags, false); err != nil { return err } diff --git a/e2e/scripts/embedded-preflight.sh b/e2e/scripts/embedded-preflight.sh index 424e7e6ca..81b4fb8ff 100755 --- a/e2e/scripts/embedded-preflight.sh +++ b/e2e/scripts/embedded-preflight.sh @@ -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 From 848295f47f897fd7d62bc6ea09dea70a50e4cd71 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 13:50:22 -0500 Subject: [PATCH 05/12] f --- e2e/scripts/embedded-preflight.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/scripts/embedded-preflight.sh b/e2e/scripts/embedded-preflight.sh index 81b4fb8ff..c7de17640 100755 --- a/e2e/scripts/embedded-preflight.sh +++ b/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 From 17d9b66388363e5e828e7aaf2d095d739fb27294 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 14:29:03 -0500 Subject: [PATCH 06/12] f --- cmd/installer/cli/metrics.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cmd/installer/cli/metrics.go b/cmd/installer/cli/metrics.go index 7283a6bce..9d1b983f5 100644 --- a/cmd/installer/cli/metrics.go +++ b/cmd/installer/cli/metrics.go @@ -36,9 +36,7 @@ func (r *InstallReporter) ReportInstallationFailed(ctx context.Context, err erro } func (r *InstallReporter) ReportPreflightsFailed(ctx context.Context, output preflightstypes.Output, bypassed bool) { - if r.license != nil && r.license.Spec.Endpoint != "" { - 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 { From 63e11841cb844e0675c0a39986e46114a13a4294 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 15:19:46 -0500 Subject: [PATCH 07/12] use single-node-install.sh in LAM e2e test --- e2e/local-artifact-mirror_test.go | 5 +-- e2e/scripts/default-install.sh | 55 ------------------------------- 2 files changed, 3 insertions(+), 57 deletions(-) delete mode 100755 e2e/scripts/default-install.sh diff --git a/e2e/local-artifact-mirror_test.go b/e2e/local-artifact-mirror_test.go index ebb963170..6787fbecd 100644 --- a/e2e/local-artifact-mirror_test.go +++ b/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) } diff --git a/e2e/scripts/default-install.sh b/e2e/scripts/default-install.sh deleted file mode 100755 index 0e75aa019..000000000 --- a/e2e/scripts/default-install.sh +++ /dev/null @@ -1,55 +0,0 @@ -#!/usr/bin/env bash -set -euox pipefail - -DIR=/usr/local/bin -. $DIR/common.sh - -check_openebs_storage_class() { - scs=$(kubectl get sc --no-headers | wc -l) - if [ "$scs" -ne "1" ]; then - echo "Expected 1 storage class, found $scs" - kubectl get sc - return 1 - fi -} - -main() { - local additional_args= - if [ -n "${1:-}" ]; then - additional_args="$*" - echo "Running install with additional args: $additional_args" - fi - - if embedded-cluster install --yes --ignore-host-preflights --license /assets/license.yaml $additional_args 2>&1 | tee /tmp/log ; then - echo "Expected installation to fail with a license provided" - exit 1 - fi - - if ! embedded-cluster install --yes --ignore-host-preflights $additional_args 2>&1 | tee /tmp/log ; then - cat /etc/os-release - echo "Failed to install embedded-cluster" - exit 1 - fi - if ! grep -q "Admin Console is ready!" /tmp/log; then - echo "Failed to validate that the Admin Console is ready" - exit 1 - fi - if ! wait_for_healthy_node; then - echo "Failed to wait for healthy node" - exit 1 - fi - if ! wait_for_pods_running 900; then - echo "Failed to wait for pods to be running" - exit 1 - fi - if ! check_openebs_storage_class; then - echo "Failed to validate if only openebs storage class is present" - exit 1 - fi - if ! systemctl restart embedded-cluster; then - echo "Failed to restart embedded-cluster service" - exit 1 - fi -} - -main "$@" From f888f15e31ec58a50ec4e8ba4846ec0f824d9793 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 15:26:48 -0500 Subject: [PATCH 08/12] improve sudo test logging --- e2e/sudo_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/e2e/sudo_test.go b/e2e/sudo_test.go index 952f3ae3a..59946b185 100644 --- a/e2e/sudo_test.go +++ b/e2e/sudo_test.go @@ -15,6 +15,7 @@ func TestCommandsRequireSudo(t *testing.T) { Nodes: 1, CreateRegularUser: true, Image: "debian/12", + LicensePath: "license.yaml", EmbeddedClusterPath: "../output/bin/embedded-cluster", }) defer tc.Cleanup() @@ -29,12 +30,13 @@ func TestCommandsRequireSudo(t *testing.T) { {"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, "_")) stdout, stderr, err := tc.RunRegularUserCommandOnNode(t, 0, cmd) if err == nil { + t.Logf("stdout:\n%s\nstderr:%s\n", stdout, stderr) t.Fatalf("expected error running `%v` as regular user, got none", cmd) } if !strings.Contains(stderr, "command must be run as root") { From f7973a4c2c99a3464a9095a3f28591b6c3a5041b Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 15:54:04 -0500 Subject: [PATCH 09/12] ??? --- e2e/sudo_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/e2e/sudo_test.go b/e2e/sudo_test.go index 59946b185..f25ebffa6 100644 --- a/e2e/sudo_test.go +++ b/e2e/sudo_test.go @@ -21,9 +21,12 @@ func TestCommandsRequireSudo(t *testing.T) { 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) + for _, cmd := range [][]string{ {"embedded-cluster", "node", "join", "https://test", "token"}, {"embedded-cluster", "join", "https://test", "token"}, @@ -33,7 +36,7 @@ func TestCommandsRequireSudo(t *testing.T) { {"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.Logf("stdout:\n%s\nstderr:%s\n", stdout, stderr) From 963d3909676b0ec13190fa8b7dc2558b7b90c2d5 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 15:58:52 -0500 Subject: [PATCH 10/12] see if it's just one thing failing in sudo test --- e2e/sudo_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/e2e/sudo_test.go b/e2e/sudo_test.go index f25ebffa6..16c9c573f 100644 --- a/e2e/sudo_test.go +++ b/e2e/sudo_test.go @@ -27,6 +27,7 @@ func TestCommandsRequireSudo(t *testing.T) { } 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"}, @@ -40,12 +41,19 @@ func TestCommandsRequireSudo(t *testing.T) { stdout, stderr, err := tc.RunRegularUserCommandOnNode(t, 0, cmd) if err == nil { t.Logf("stdout:\n%s\nstderr:%s\n", stdout, stderr) - t.Fatalf("expected error running `%v` as regular user, got none", cmd) + 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)) } From eb71edaa76e11c9481b7d76b21e3639254dd624f Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 16:35:52 -0500 Subject: [PATCH 11/12] use join2 command in 'node join'. Hide 'node' commands --- cmd/installer/cli/node.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/installer/cli/node.go b/cmd/installer/cli/node.go index bf0d4aace..53b5895e5 100644 --- a/cmd/installer/cli/node.go +++ b/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) From b14cdff3862f3d27118df17d3f81de20c56a57c3 Mon Sep 17 00:00:00 2001 From: Andrew Lavery Date: Fri, 31 Jan 2025 16:47:05 -0500 Subject: [PATCH 12/12] improve how we are requiring the license flag for installation --- cmd/installer/cli/install2.go | 13 +++++-------- cmd/installer/cli/install_runpreflights.go | 2 +- cmd/installer/cli/restore.go | 2 +- cmd/installer/cli/restore2.go | 2 +- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/cmd/installer/cli/install2.go b/cmd/installer/cli/install2.go index af902a888..c85d54738 100644 --- a/cmd/installer/cli/install2.go +++ b/cmd/installer/cli/install2.go @@ -85,7 +85,7 @@ func Install2Cmd(ctx context.Context, name string) *cobra.Command { Use: "install", Short: fmt.Sprintf("Experimental installer for %s", name), PreRunE: func(cmd *cobra.Command, args []string) error { - if err := preRunInstall2(cmd, &flags, true); err != nil { + if err := preRunInstall2(cmd, &flags); err != nil { return err } @@ -157,16 +157,15 @@ 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 } -func preRunInstall2(cmd *cobra.Command, flags *Install2CmdFlags, requireLicense bool) error { +func preRunInstall2(cmd *cobra.Command, flags *Install2CmdFlags) error { if os.Getuid() != 0 { return fmt.Errorf("install command must be run as root") } @@ -211,8 +210,6 @@ func preRunInstall2(cmd *cobra.Command, flags *Install2CmdFlags, requireLicense return fmt.Errorf("unable to parse license file: %w", err) } flags.license = l - } else if requireLicense { - return fmt.Errorf("license is required") } runtimeconfig.ApplyFlags(cmd.Flags()) diff --git a/cmd/installer/cli/install_runpreflights.go b/cmd/installer/cli/install_runpreflights.go index 6c46d1b0d..7244528a4 100644 --- a/cmd/installer/cli/install_runpreflights.go +++ b/cmd/installer/cli/install_runpreflights.go @@ -19,7 +19,7 @@ func InstallRunPreflightsCmd(ctx context.Context, name string) *cobra.Command { Use: "run-preflights", Short: "Run install host preflights", PreRunE: func(cmd *cobra.Command, args []string) error { - if err := preRunInstall2(cmd, &flags, false); err != nil { + if err := preRunInstall2(cmd, &flags); err != nil { return err } diff --git a/cmd/installer/cli/restore.go b/cmd/installer/cli/restore.go index 06707fe0c..1c2cc7490 100644 --- a/cmd/installer/cli/restore.go +++ b/cmd/installer/cli/restore.go @@ -31,7 +31,7 @@ func RestoreCmd(ctx context.Context, name string) *cobra.Command { Use: "restore-legacy", Short: fmt.Sprintf("Restore a %s cluster", name), PreRunE: func(cmd *cobra.Command, args []string) error { - if err := preRunInstall2(cmd, &flags, false); err != nil { + if err := preRunInstall2(cmd, &flags); err != nil { return err } diff --git a/cmd/installer/cli/restore2.go b/cmd/installer/cli/restore2.go index 1ff797612..24b889534 100644 --- a/cmd/installer/cli/restore2.go +++ b/cmd/installer/cli/restore2.go @@ -92,7 +92,7 @@ func Restore2Cmd(ctx context.Context, name string) *cobra.Command { Use: "restore", Short: fmt.Sprintf("Restore a %s cluster", name), PreRunE: func(cmd *cobra.Command, args []string) error { - if err := preRunInstall2(cmd, &flags, false); err != nil { + if err := preRunInstall2(cmd, &flags); err != nil { return err }