From 360d0cf3caff73e97383e943e88b6ad2efbc33d3 Mon Sep 17 00:00:00 2001 From: charvenetis Date: Fri, 20 Feb 2026 20:36:31 +0200 Subject: [PATCH 1/4] feat: add logs subcommand for mgrctl (Fixes #398) Signed-off-by: charvenetis --- mgrctl/cmd/cmd.go | 2 + mgrctl/cmd/logs/logs.go | 112 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 mgrctl/cmd/logs/logs.go diff --git a/mgrctl/cmd/cmd.go b/mgrctl/cmd/cmd.go index 2ba43ba70cc..01d982a416a 100644 --- a/mgrctl/cmd/cmd.go +++ b/mgrctl/cmd/cmd.go @@ -14,6 +14,7 @@ import ( "github.com/uyuni-project/uyuni-tools/mgrctl/cmd/api" "github.com/uyuni-project/uyuni-tools/mgrctl/cmd/cp" "github.com/uyuni-project/uyuni-tools/mgrctl/cmd/exec" + "github.com/uyuni-project/uyuni-tools/mgrctl/cmd/logs" "github.com/uyuni-project/uyuni-tools/mgrctl/cmd/proxy" "github.com/uyuni-project/uyuni-tools/mgrctl/cmd/term" "github.com/uyuni-project/uyuni-tools/shared/completion" @@ -55,6 +56,7 @@ func NewUyunictlCommand() *cobra.Command { rootCmd.AddCommand(cp.NewCommand(globalFlags)) rootCmd.AddCommand(completion.NewCommand(globalFlags)) rootCmd.AddCommand(proxy.NewCommand(globalFlags)) + rootCmd.AddCommand(logs.NewCommand(globalFlags)) rootCmd.AddCommand(utils.GetConfigHelpCommand()) diff --git a/mgrctl/cmd/logs/logs.go b/mgrctl/cmd/logs/logs.go new file mode 100644 index 00000000000..070189e5550 --- /dev/null +++ b/mgrctl/cmd/logs/logs.go @@ -0,0 +1,112 @@ +// SPDX-FileCopyrightText: 2024 SUSE LLC +// +// SPDX-License-Identifier: Apache-2.0 + +package logs + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + "github.com/uyuni-project/uyuni-tools/mgrctl/cmd/exec" + "github.com/uyuni-project/uyuni-tools/shared" + "github.com/uyuni-project/uyuni-tools/shared/kubernetes" + . "github.com/uyuni-project/uyuni-tools/shared/l10n" + "github.com/uyuni-project/uyuni-tools/shared/podman" + "github.com/uyuni-project/uyuni-tools/shared/types" + "github.com/uyuni-project/uyuni-tools/shared/utils" +) + +type flagpole struct { + Taskomatic bool + Web bool + Salt bool + Reposync bool + Files string + Follow bool + Backend string +} + +// NewCommand returns a new cobra.Command for logs. +func NewCommand(globalFlags *types.GlobalFlags) *cobra.Command { + var flags flagpole + + logsCmd := &cobra.Command{ + Use: "logs", + Short: L("Show or follow logs of Uyuni services inside the container"), + RunE: func(cmd *cobra.Command, args []string) error { + return utils.CommandHelper(globalFlags, cmd, args, &flags, nil, run) + }, + } + + logsCmd.Flags().Bool("taskomatic", false, L("Show Taskomatic logs")) + logsCmd.Flags().Bool("web", false, L("Show Web UI logs")) + logsCmd.Flags().Bool("salt", false, L("Show Salt logs")) + logsCmd.Flags().Bool("reposync", false, L("Show Reposync logs")) + logsCmd.Flags().String("files", "", L("Regex to filter reposync log files")) + logsCmd.Flags().BoolP("follow", "f", false, L("Follow log output")) + + utils.AddBackendFlag(logsCmd) + return logsCmd +} + +func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) error { + cnx := shared.NewConnection(flags.Backend, podman.ServerContainerName, kubernetes.ServerFilter) + podName, err := cnx.GetPodName() + if err != nil { + return err + } + + command, err := cnx.GetCommand() + if err != nil { + return err + } + + var paths []string + if flags.Taskomatic { + paths = append(paths, "/var/log/rhn/rhn_tasko*.log") + } + if flags.Web { + paths = append(paths, "/var/log/rhn/rhn_web*.log") + } + if flags.Salt { + paths = append(paths, "/var/log/salt/api", "/var/log/salt/master", "/var/log/salt/minion") + } + if flags.Reposync { + if flags.Files != "" { + findCmd := fmt.Sprintf("find /var/log/rhn/reposync/ -type f | grep -E '%s'", flags.Files) + paths = append(paths, fmt.Sprintf("$(%s)", findCmd)) + } else { + paths = append(paths, "/var/log/rhn/reposync/*.log") + } + } + + if len(paths) == 0 { + return fmt.Errorf(L("please specify at least one service to get logs for (e.g., --web, --salt)")) + } + + tailCmd := "tail" + if flags.Follow { + tailCmd += " -f" + } + + shCmd := fmt.Sprintf("%s %s", tailCmd, strings.Join(paths, " ")) + + commandArgs := []string{"exec", "-i", "-t"} + + if command == "kubectl" { + namespace, err := cnx.GetNamespace("") + if err != nil { + return err + } + commandArgs = append(commandArgs, "-n", namespace, "-c", "uyuni", podName, "--") + } else { + commandArgs = append(commandArgs, podName) + } + + commandArgs = append(commandArgs, "bash", "-c", shCmd) + + // Re-use the execution logic from the exec package + return exec.RunRawCmd(command, commandArgs) +} From 14290f49f64dd42dc814cf2ef67a81a95d283491 Mon Sep 17 00:00:00 2001 From: charvenetis Date: Wed, 25 Feb 2026 14:27:48 +0200 Subject: [PATCH 2/4] fix: resolve linter error and address AI review feedback --- mgrctl/cmd/logs/logs.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/mgrctl/cmd/logs/logs.go b/mgrctl/cmd/logs/logs.go index 070189e5550..2e9d99e9b7b 100644 --- a/mgrctl/cmd/logs/logs.go +++ b/mgrctl/cmd/logs/logs.go @@ -5,7 +5,10 @@ package logs import ( + "errors" "fmt" + "os" + os_exec "os/exec" "strings" "github.com/spf13/cobra" @@ -63,6 +66,10 @@ func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) er return err } + if flags.Files != "" && !flags.Reposync { + return errors.New(L("--files flag can only be used with --reposync")) + } + var paths []string if flags.Taskomatic { paths = append(paths, "/var/log/rhn/rhn_tasko*.log") @@ -75,7 +82,10 @@ func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) er } if flags.Reposync { if flags.Files != "" { - findCmd := fmt.Sprintf("find /var/log/rhn/reposync/ -type f | grep -E '%s'", flags.Files) + script := "files=$(find /var/log/rhn/reposync/ -type f | grep -E %q); " + + "if [ -z \"$files\" ]; then echo 'No matching reposync logs found.' >&2; exit 1; fi; " + + "echo $files" + findCmd := fmt.Sprintf(script, flags.Files) paths = append(paths, fmt.Sprintf("$(%s)", findCmd)) } else { paths = append(paths, "/var/log/rhn/reposync/*.log") @@ -83,7 +93,7 @@ func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) er } if len(paths) == 0 { - return fmt.Errorf(L("please specify at least one service to get logs for (e.g., --web, --salt)")) + return errors.New(L("please specify at least one service to get logs for (e.g., --web, --salt)")) } tailCmd := "tail" @@ -108,5 +118,15 @@ func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) er commandArgs = append(commandArgs, "bash", "-c", shCmd) // Re-use the execution logic from the exec package - return exec.RunRawCmd(command, commandArgs) + err = exec.RunRawCmd(command, commandArgs) + + if err != nil { + var exitErr *os_exec.ExitError + if errors.As(err, &exitErr) { + os.Exit(exitErr.ExitCode()) + } + return err + } + + return nil } From 584095f32bfd35f2752bea86d36d67991be97139 Mon Sep 17 00:00:00 2001 From: charvenetis Date: Wed, 25 Feb 2026 19:00:05 +0200 Subject: [PATCH 3/4] refactor: reduce cognitive complexity to satisfy SonarQube --- mgrctl/cmd/logs/logs.go | 42 +++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/mgrctl/cmd/logs/logs.go b/mgrctl/cmd/logs/logs.go index 2e9d99e9b7b..84c6b26cc5f 100644 --- a/mgrctl/cmd/logs/logs.go +++ b/mgrctl/cmd/logs/logs.go @@ -54,22 +54,14 @@ func NewCommand(globalFlags *types.GlobalFlags) *cobra.Command { return logsCmd } -func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) error { - cnx := shared.NewConnection(flags.Backend, podman.ServerContainerName, kubernetes.ServerFilter) - podName, err := cnx.GetPodName() - if err != nil { - return err - } - - command, err := cnx.GetCommand() - if err != nil { - return err - } - +func validateFlags(flags *flagpole) error { if flags.Files != "" && !flags.Reposync { return errors.New(L("--files flag can only be used with --reposync")) } + return nil +} +func getLogPaths(flags *flagpole) []string { var paths []string if flags.Taskomatic { paths = append(paths, "/var/log/rhn/rhn_tasko*.log") @@ -85,24 +77,41 @@ func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) er script := "files=$(find /var/log/rhn/reposync/ -type f | grep -E %q); " + "if [ -z \"$files\" ]; then echo 'No matching reposync logs found.' >&2; exit 1; fi; " + "echo $files" - findCmd := fmt.Sprintf(script, flags.Files) - paths = append(paths, fmt.Sprintf("$(%s)", findCmd)) + paths = append(paths, fmt.Sprintf("$(%s)", fmt.Sprintf(script, flags.Files))) } else { paths = append(paths, "/var/log/rhn/reposync/*.log") } } + return paths +} +func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) error { + if err := validateFlags(flags); err != nil { + return err + } + + paths := getLogPaths(flags) if len(paths) == 0 { return errors.New(L("please specify at least one service to get logs for (e.g., --web, --salt)")) } + cnx := shared.NewConnection(flags.Backend, podman.ServerContainerName, kubernetes.ServerFilter) + podName, err := cnx.GetPodName() + if err != nil { + return err + } + + command, err := cnx.GetCommand() + if err != nil { + return err + } + tailCmd := "tail" if flags.Follow { tailCmd += " -f" } shCmd := fmt.Sprintf("%s %s", tailCmd, strings.Join(paths, " ")) - commandArgs := []string{"exec", "-i", "-t"} if command == "kubectl" { @@ -116,8 +125,6 @@ func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) er } commandArgs = append(commandArgs, "bash", "-c", shCmd) - - // Re-use the execution logic from the exec package err = exec.RunRawCmd(command, commandArgs) if err != nil { @@ -127,6 +134,5 @@ func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) er } return err } - return nil } From a9764877818e40bdad2660caa9e77061cd9ff5e7 Mon Sep 17 00:00:00 2001 From: charvenetis Date: Wed, 25 Feb 2026 19:23:12 +0200 Subject: [PATCH 4/4] fix: address maintainer feedback, add unit tests, and refactor execution runner --- mgrctl/cmd/logs/logs.go | 25 ++++++++++++------ mgrctl/cmd/logs/logs_test.go | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 mgrctl/cmd/logs/logs_test.go diff --git a/mgrctl/cmd/logs/logs.go b/mgrctl/cmd/logs/logs.go index 84c6b26cc5f..281965dbcfc 100644 --- a/mgrctl/cmd/logs/logs.go +++ b/mgrctl/cmd/logs/logs.go @@ -12,7 +12,6 @@ import ( "strings" "github.com/spf13/cobra" - "github.com/uyuni-project/uyuni-tools/mgrctl/cmd/exec" "github.com/uyuni-project/uyuni-tools/shared" "github.com/uyuni-project/uyuni-tools/shared/kubernetes" . "github.com/uyuni-project/uyuni-tools/shared/l10n" @@ -31,6 +30,9 @@ type flagpole struct { Backend string } +// runCmd allows unit tests to mock or skip the main execution loop. +var runCmd = run + // NewCommand returns a new cobra.Command for logs. func NewCommand(globalFlags *types.GlobalFlags) *cobra.Command { var flags flagpole @@ -39,7 +41,7 @@ func NewCommand(globalFlags *types.GlobalFlags) *cobra.Command { Use: "logs", Short: L("Show or follow logs of Uyuni services inside the container"), RunE: func(cmd *cobra.Command, args []string) error { - return utils.CommandHelper(globalFlags, cmd, args, &flags, nil, run) + return utils.CommandHelper(globalFlags, cmd, args, &flags, nil, runCmd) }, } @@ -47,7 +49,8 @@ func NewCommand(globalFlags *types.GlobalFlags) *cobra.Command { logsCmd.Flags().Bool("web", false, L("Show Web UI logs")) logsCmd.Flags().Bool("salt", false, L("Show Salt logs")) logsCmd.Flags().Bool("reposync", false, L("Show Reposync logs")) - logsCmd.Flags().String("files", "", L("Regex to filter reposync log files")) + logsCmd.Flags().String("files", "", + L("Regular expression to filter reposync log files (matches against the full file path)")) logsCmd.Flags().BoolP("follow", "f", false, L("Follow log output")) utils.AddBackendFlag(logsCmd) @@ -70,13 +73,19 @@ func getLogPaths(flags *flagpole) []string { paths = append(paths, "/var/log/rhn/rhn_web*.log") } if flags.Salt { - paths = append(paths, "/var/log/salt/api", "/var/log/salt/master", "/var/log/salt/minion") + // Minion does not run inside the Uyuni server container + paths = append(paths, "/var/log/salt/api", "/var/log/salt/master") } if flags.Reposync { if flags.Files != "" { script := "files=$(find /var/log/rhn/reposync/ -type f | grep -E %q); " + - "if [ -z \"$files\" ]; then echo 'No matching reposync logs found.' >&2; exit 1; fi; " + - "echo $files" + "if [ -z \"$files\" ]; then " + // Use literal path on follow so tail can wait for file creation + if flags.Follow { + script += "echo \"/var/log/rhn/reposync/*.log\"; else echo $files; fi" + } else { + script += "echo 'No matching reposync logs found.' >&2; exit 1; fi; echo $files" + } paths = append(paths, fmt.Sprintf("$(%s)", fmt.Sprintf(script, flags.Files))) } else { paths = append(paths, "/var/log/rhn/reposync/*.log") @@ -108,7 +117,7 @@ func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) er tailCmd := "tail" if flags.Follow { - tailCmd += " -f" + tailCmd += " -F" } shCmd := fmt.Sprintf("%s %s", tailCmd, strings.Join(paths, " ")) @@ -125,8 +134,8 @@ func run(_ *types.GlobalFlags, flags *flagpole, _ *cobra.Command, _ []string) er } commandArgs = append(commandArgs, "bash", "-c", shCmd) - err = exec.RunRawCmd(command, commandArgs) + _, err = utils.NewRunner(command, commandArgs...).Exec() if err != nil { var exitErr *os_exec.ExitError if errors.As(err, &exitErr) { diff --git a/mgrctl/cmd/logs/logs_test.go b/mgrctl/cmd/logs/logs_test.go new file mode 100644 index 00000000000..6c03a70a0fb --- /dev/null +++ b/mgrctl/cmd/logs/logs_test.go @@ -0,0 +1,50 @@ +// SPDX-FileCopyrightText: 2024 SUSE LLC +// +// SPDX-License-Identifier: Apache-2.0 + +package logs + +import ( + "strings" + "testing" +) + +func TestValidateFlags(t *testing.T) { + // Test --files without --reposync (Should Fail) + invalidFlags := &flagpole{Files: "myregex"} + err := validateFlags(invalidFlags) + if err == nil { + t.Errorf("Expected error when using --files without --reposync, got nil") + } + + // Test --files with --reposync (Should Pass) + validFlags := &flagpole{Reposync: true, Files: "myregex"} + err = validateFlags(validFlags) + if err != nil { + t.Errorf("Expected no error when using --files with --reposync, got %v", err) + } +} + +func TestGetLogPaths(t *testing.T) { + flags := &flagpole{ + Salt: true, + } + + paths := getLogPaths(flags) + + // Test 1: Ensure salt paths exist but minion is absent + pathStr := strings.Join(paths, " ") + if !strings.Contains(pathStr, "/var/log/salt/api") || !strings.Contains(pathStr, "/var/log/salt/master") { + t.Errorf("Expected salt api and master paths, got: %v", paths) + } + if strings.Contains(pathStr, "/var/log/salt/minion") { + t.Errorf("Did not expect minion path in salt logs, got: %v", paths) + } + + // Test 2: Ensure empty paths if no flags are set + emptyFlags := &flagpole{} + emptyPaths := getLogPaths(emptyFlags) + if len(emptyPaths) != 0 { + t.Errorf("Expected 0 paths for empty flags, got: %v", emptyPaths) + } +}