diff --git a/ctl/authz/authz.go b/ctl/authz/authz.go index 7998af4f1..d5fcf238b 100644 --- a/ctl/authz/authz.go +++ b/ctl/authz/authz.go @@ -22,7 +22,6 @@ import ( "fmt" "io" "net/http" - "os" "text/tabwriter" "github.com/spf13/cobra" @@ -59,10 +58,13 @@ func NewEnableCmd() *cobra.Command { Short: "Enable xdp authz eBPF program for Kmesh's authz offloading", Example: "kmeshctl authz enable\nkmeshctl authz enable pod1 pod2", Args: cobra.ArbitraryArgs, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { // If no pod names are given, apply to all kmesh daemon pods. - SetAuthzForPods(args, "true") + if err := SetAuthzForPods(args, "true"); err != nil { + return err + } log.Info("Authorization has been enabled.") + return nil }, } return cmd @@ -75,9 +77,12 @@ func NewDisableCmd() *cobra.Command { Short: "Disable xdp authz eBPF program for Kmesh's authz offloading", Example: "kmeshctl authz disable\nkmeshctl authz disable pod1 pod2", Args: cobra.ArbitraryArgs, - Run: func(cmd *cobra.Command, args []string) { - SetAuthzForPods(args, "false") + RunE: func(cmd *cobra.Command, args []string) error { + if err := SetAuthzForPods(args, "false"); err != nil { + return err + } log.Info("Authorization has been disabled.") + return nil }, } return cmd @@ -90,11 +95,10 @@ func NewStatusCmd() *cobra.Command { Short: "Display the current authorization status", Example: "kmeshctl authz status\nkmeshctl authz status pod1 pod2", Args: cobra.ArbitraryArgs, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { cli, err := utils.CreateKubeClient() if err != nil { - log.Errorf("failed to create cli client: %v", err) - os.Exit(1) + return fmt.Errorf("failed to create cli client: %v", err) } // Determine which pods to query. @@ -102,8 +106,7 @@ func NewStatusCmd() *cobra.Command { if len(args) == 0 { podList, err := cli.PodsForSelector(context.TODO(), utils.KmeshNamespace, utils.KmeshLabel) if err != nil { - log.Errorf("failed to get kmesh podList: %v", err) - os.Exit(1) + return fmt.Errorf("failed to get kmesh podList: %v", err) } for _, pod := range podList.Items { podNames = append(podNames, pod.GetName()) @@ -130,6 +133,7 @@ func NewStatusCmd() *cobra.Command { } // Output the results in a table format. + out := cmd.OutOrStdout() var buf bytes.Buffer tw := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0) fmt.Fprintln(tw, "POD\tAUTHORIZATION STATUS") @@ -137,7 +141,8 @@ func NewStatusCmd() *cobra.Command { fmt.Fprintf(tw, "%s\t%s\n", s.Pod, s.Status) } tw.Flush() - fmt.Print(buf.String()) + fmt.Fprint(out, buf.String()) + return nil }, } return cmd @@ -145,42 +150,43 @@ func NewStatusCmd() *cobra.Command { // SetAuthzForPods applies the authz setting (enable/disable) for the given pod(s). // If no pod names are specified, it applies the setting to all kmesh daemon pods. -func SetAuthzForPods(podNames []string, info string) { +func SetAuthzForPods(podNames []string, info string) error { cli, err := utils.CreateKubeClient() if err != nil { - log.Errorf("failed to create cli client: %v", err) - os.Exit(1) + return fmt.Errorf("failed to create cli client: %v", err) } if len(podNames) == 0 { // Apply to all kmesh daemon pods. podList, err := cli.PodsForSelector(context.TODO(), utils.KmeshNamespace, utils.KmeshLabel) if err != nil { - log.Errorf("failed to get kmesh podList: %v", err) - os.Exit(1) + return fmt.Errorf("failed to get kmesh podList: %v", err) } for _, pod := range podList.Items { - SetAuthzPerKmeshDaemon(cli, pod.GetName(), info) + if err := SetAuthzPerKmeshDaemon(cli, pod.GetName(), info); err != nil { + return err + } } } else { // Process for specified pods. for _, podName := range podNames { - SetAuthzPerKmeshDaemon(cli, podName, info) + if err := SetAuthzPerKmeshDaemon(cli, podName, info); err != nil { + return err + } } } + return nil } // SetAuthzPerKmeshDaemon sends a POST request to a specific kmesh daemon pod // to set the authz flag based on the info parameter ("true" or "false"). -func SetAuthzPerKmeshDaemon(cli kube.CLIClient, podName, info string) { +func SetAuthzPerKmeshDaemon(cli kube.CLIClient, podName, info string) error { fw, err := utils.CreateKmeshPortForwarder(cli, podName) if err != nil { - log.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err) - os.Exit(1) + return fmt.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err) } if err := fw.Start(); err != nil { - log.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err) - os.Exit(1) + return fmt.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err) } defer fw.Close() @@ -188,23 +194,21 @@ func SetAuthzPerKmeshDaemon(cli kube.CLIClient, podName, info string) { req, err := http.NewRequest(http.MethodPost, url, nil) if err != nil { - log.Errorf("Error creating request: %v", err) - return + return fmt.Errorf("error creating request: %v", err) } req.Header.Set("Content-Type", "application/json") client := &http.Client{} resp, err := client.Do(req) if err != nil { - log.Errorf("failed to make HTTP request: %v", err) - return + return fmt.Errorf("failed to make HTTP request: %v", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - log.Errorf("Error: received status code %d", resp.StatusCode) - return + return fmt.Errorf("error: received status code %d", resp.StatusCode) } + return nil } // fetchAuthzStatus sends a GET request to a specific kmesh daemon pod diff --git a/ctl/common/common.go b/ctl/common/common.go index b804bafc1..6753eced8 100644 --- a/ctl/common/common.go +++ b/ctl/common/common.go @@ -30,9 +30,10 @@ import ( func GetRootCommand() *cobra.Command { rootCmd := &cobra.Command{ - Use: "kmeshctl", - Short: "Kmesh command line tools to operate and debug Kmesh", - SilenceUsage: true, + Use: "kmeshctl", + Short: "Kmesh command line tools to operate and debug Kmesh", + SilenceUsage: true, + SilenceErrors: true, CompletionOptions: cobra.CompletionOptions{ DisableDefaultCmd: true, }, diff --git a/ctl/dump/dump.go b/ctl/dump/dump.go index 962c5aa8a..89113246d 100644 --- a/ctl/dump/dump.go +++ b/ctl/dump/dump.go @@ -23,7 +23,6 @@ import ( "io" "net" "net/http" - "os" "strings" "text/tabwriter" @@ -57,8 +56,8 @@ kmeshctl dump dual-engine # Output as raw JSON: kmeshctl dump kernel-native -o json`, Args: cobra.ExactArgs(2), - Run: func(cmd *cobra.Command, args []string) { - _ = RunDump(cmd, args, outputFormat) + RunE: func(cmd *cobra.Command, args []string) error { + return RunDump(cmd, args, outputFormat) }, } @@ -70,49 +69,46 @@ func RunDump(cmd *cobra.Command, args []string, outputFormat string) error { podName := args[0] mode := args[1] if mode != constants.KernelNativeMode && mode != constants.DualEngineMode { - log.Errorf("Error: Argument must be 'kernel-native' or 'dual-engine'") - os.Exit(1) + return fmt.Errorf("argument must be 'kernel-native' or 'dual-engine'") } cli, err := utils.CreateKubeClient() if err != nil { - log.Errorf("failed to create cli client: %v", err) - os.Exit(1) + return fmt.Errorf("failed to create cli client: %v", err) } fw, err := utils.CreateKmeshPortForwarder(cli, podName) if err != nil { - log.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err) - os.Exit(1) + return fmt.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err) } if err := fw.Start(); err != nil { - log.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err) + return fmt.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err) } + defer fw.Close() url := fmt.Sprintf("http://%s%s/%s", fw.Address(), configDumpPrefix, mode) resp, err := http.Get(url) if err != nil { - log.Errorf("failed to make HTTP request: %v", err) - os.Exit(1) + return fmt.Errorf("failed to make HTTP request: %v", err) } defer resp.Body.Close() body, err := io.ReadAll(resp.Body) if err != nil { - log.Errorf("failed to read HTTP response body: %v", err) - os.Exit(1) + return fmt.Errorf("failed to read HTTP response body: %v", err) } + out := cmd.OutOrStdout() if outputFormat == "json" { - fmt.Println(string(body)) + fmt.Fprintln(out, string(body)) return nil } switch mode { case constants.KernelNativeMode: - printKernelNativeTable(body) + printKernelNativeTable(out, body) case constants.DualEngineMode: - printDualEngineTable(body) + printDualEngineTable(out, body) } return nil @@ -120,15 +116,15 @@ func RunDump(cmd *cobra.Command, args []string, outputFormat string) error { // printKernelNativeTable parses and displays kernel-native config dump as tables. // Static and dynamic resources of the same type are consolidated under a single header. -func printKernelNativeTable(body []byte) { +func printKernelNativeTable(out io.Writer, body []byte) { configDump := &adminv2.ConfigDump{} if err := protojson.Unmarshal(body, configDump); err != nil { log.Errorf("failed to parse config dump: %v, falling back to raw output", err) - fmt.Println(string(body)) + fmt.Fprintln(out, string(body)) return } - w := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0) + w := tabwriter.NewWriter(out, 0, 0, 3, ' ', 0) static, dynamic := configDump.GetStaticResources(), configDump.GetDynamicResources() // Clusters @@ -145,7 +141,7 @@ func printKernelNativeTable(body []byte) { } } _ = w.Flush() - fmt.Println() + fmt.Fprintln(out) } // Listeners @@ -176,7 +172,7 @@ func printKernelNativeTable(body []byte) { printListeners(dynamic) } _ = w.Flush() - fmt.Println() + fmt.Fprintln(out) } // Routes @@ -196,7 +192,7 @@ func printKernelNativeTable(body []byte) { printRoutes(dynamic) } _ = w.Flush() - fmt.Println() + fmt.Fprintln(out) } } @@ -230,15 +226,15 @@ type policyEntry struct { } // printDualEngineTable parses and displays dual-engine config dump as tables. -func printDualEngineTable(body []byte) { +func printDualEngineTable(out io.Writer, body []byte) { var dump workloadDump if err := json.Unmarshal(body, &dump); err != nil { log.Errorf("failed to parse workload dump: %v, falling back to raw output", err) - fmt.Println(string(body)) + fmt.Fprintln(out, string(body)) return } - w := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0) + w := tabwriter.NewWriter(out, 0, 0, 3, ' ', 0) if len(dump.Workloads) > 0 { fmt.Fprintln(w, "NAME\tNAMESPACE\tADDRESSES\tPROTOCOL\tSTATUS") @@ -252,7 +248,7 @@ func printDualEngineTable(body []byte) { ) } _ = w.Flush() - fmt.Println() + fmt.Fprintln(out) } if len(dump.Services) > 0 { @@ -266,7 +262,7 @@ func printDualEngineTable(body []byte) { ) } _ = w.Flush() - fmt.Println() + fmt.Fprintln(out) } if len(dump.Policies) > 0 { @@ -280,7 +276,7 @@ func printDualEngineTable(body []byte) { ) } _ = w.Flush() - fmt.Println() + fmt.Fprintln(out) } } diff --git a/ctl/log/log.go b/ctl/log/log.go index 7b1eaefbe..5a16be8d7 100644 --- a/ctl/log/log.go +++ b/ctl/log/log.go @@ -23,21 +23,17 @@ import ( "io" "net/http" "net/url" - "os" "strings" "github.com/spf13/cobra" "kmesh.net/kmesh/ctl/utils" - "kmesh.net/kmesh/pkg/logger" ) const ( patternLoggers = "/debug/loggers" ) -var log = logger.NewLoggerScope("kmeshctl/log") - type LoggerInfo struct { Name string `json:"name,omitempty"` Level string `json:"level,omitempty"` @@ -56,8 +52,8 @@ kmeshctl log # Get default logger's level: kmeshctl log default`, Args: cobra.MinimumNArgs(1), - Run: func(cmd *cobra.Command, args []string) { - RunGetOrSetLoggerLevel(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return RunGetOrSetLoggerLevel(cmd, args) }, } cmd.Flags().String("set", "", "Set the logger level (e.g., default:debug)") @@ -102,34 +98,33 @@ func GetJson(url string, val any) error { return nil } -func GetLoggerNames(url string) { +func GetLoggerNames(out io.Writer, url string) error { var loggerNames []string if err := GetJson(url, &loggerNames); err != nil { - log.Errorf("failed to get logger names: %v", err) - return + return fmt.Errorf("failed to get logger names: %v", err) } - fmt.Printf("Existing Loggers:\n") + fmt.Fprintf(out, "Existing Loggers:\n") for _, logger := range loggerNames { - fmt.Printf("\t%s\n", logger) + fmt.Fprintf(out, "\t%s\n", logger) } + return nil } -func GetLoggerLevel(url string) { +func GetLoggerLevel(out io.Writer, url string) error { var loggerInfo LoggerInfo if err := GetJson(url, &loggerInfo); err != nil { - log.Errorf("failed to get logger level: %v", err) - return + return fmt.Errorf("failed to get logger level: %v", err) } - fmt.Printf("Logger Name: %s\n", loggerInfo.Name) - fmt.Printf("Logger Level: %s\n", loggerInfo.Level) + fmt.Fprintf(out, "Logger Name: %s\n", loggerInfo.Name) + fmt.Fprintf(out, "Logger Level: %s\n", loggerInfo.Level) + return nil } -func SetLoggerLevel(url string, setFlag string) { +func SetLoggerLevel(out io.Writer, url string, setFlag string) error { if !strings.Contains(setFlag, ":") { - log.Errorf("Invalid set flag, which should be loggerName:loggerLevel (e.g. default:debug)") - os.Exit(1) + return fmt.Errorf("invalid set flag, which should be loggerName:loggerLevel (e.g. default:debug)") } splits := strings.Split(setFlag, ":") loggerName := splits[0] @@ -141,66 +136,59 @@ func SetLoggerLevel(url string, setFlag string) { } data, err := json.Marshal(loggerInfo) if err != nil { - log.Errorf("Error marshaling logger info: %v", err) - return + return fmt.Errorf("error marshaling logger info: %v", err) } req, err := http.NewRequest(http.MethodPost, url, bytes.NewBuffer(data)) if err != nil { - log.Errorf("Error creating request: %v", err) - return + return fmt.Errorf("error creating request: %v", err) } req.Header.Set("Content-Type", "application/json") client := &http.Client{} resp, err := client.Do(req) if err != nil { - log.Errorf("failed to make HTTP request: %v", err) - return + return fmt.Errorf("failed to make HTTP request: %v", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - log.Errorf("Error: received status code %d", resp.StatusCode) + return fmt.Errorf("error: received status code %d", resp.StatusCode) } body, err := io.ReadAll(resp.Body) if err != nil { - log.Errorf("failed to read HTTP response body: %v", err) - return + return fmt.Errorf("failed to read HTTP response body: %v", err) } - fmt.Println(string(body)) + fmt.Fprintln(out, string(body)) + return nil } -func RunGetOrSetLoggerLevel(cmd *cobra.Command, args []string) { +func RunGetOrSetLoggerLevel(cmd *cobra.Command, args []string) error { podName := args[0] cli, err := utils.CreateKubeClient() if err != nil { - log.Errorf("failed to create cli client: %v", err) - os.Exit(1) + return fmt.Errorf("failed to create cli client: %v", err) } fw, err := utils.CreateKmeshPortForwarder(cli, podName) if err != nil { - log.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err) - os.Exit(1) + return fmt.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err) } if err := fw.Start(); err != nil { - log.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err) - os.Exit(1) + return fmt.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err) } defer fw.Close() loggersURL := fmt.Sprintf("http://%s%s", fw.Address(), patternLoggers) + out := cmd.OutOrStdout() setFlag, _ := cmd.Flags().GetString("set") if setFlag == "" { if len(args) >= 2 { - GetLoggerLevel(loggerLevelURL(loggersURL, args[1])) - } else { - GetLoggerNames(loggersURL) + return GetLoggerLevel(out, loggerLevelURL(loggersURL, args[1])) } - } else { - SetLoggerLevel(loggersURL, setFlag) + return GetLoggerNames(out, loggersURL) } + return SetLoggerLevel(out, loggersURL, setFlag) } diff --git a/ctl/main.go b/ctl/main.go index 4efad7c54..ed42c8af0 100644 --- a/ctl/main.go +++ b/ctl/main.go @@ -17,6 +17,7 @@ package main import ( + "fmt" "os" "kmesh.net/kmesh/ctl/common" @@ -25,6 +26,7 @@ import ( func main() { rootCmd := common.GetRootCommand() if err := rootCmd.Execute(); err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } } diff --git a/ctl/monitoring/monitoring.go b/ctl/monitoring/monitoring.go index 1cb576233..abbafbff2 100644 --- a/ctl/monitoring/monitoring.go +++ b/ctl/monitoring/monitoring.go @@ -22,14 +22,12 @@ import ( "fmt" "io" "net/http" - "os" "strings" "github.com/spf13/cobra" "kmesh.net/kmesh/ctl/utils" "kmesh.net/kmesh/pkg/kube" - "kmesh.net/kmesh/pkg/logger" ) const ( @@ -47,8 +45,6 @@ const ( CONNECTION = "connection metrics" ) -var log = logger.NewLoggerScope("kmeshctl/monitoring") - func NewCmd() *cobra.Command { cmd := &cobra.Command{ Use: "monitoring", @@ -78,8 +74,8 @@ kmeshctl monitoring --connectionMetrics enable/disable #Enable/Disable services', workloads' and 'connections' metrics and accesslog generated from bpf in each node: kmeshctl monitoring --all enable/disable`, Args: cobra.MaximumNArgs(1), - Run: func(cmd *cobra.Command, args []string) { - ControlMonitoring(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return ControlMonitoring(cmd, args) }, } cmd.Flags().String("accesslog", "", "Control accesslog enable or disable") @@ -89,58 +85,72 @@ kmeshctl monitoring --all enable/disable`, return cmd } -func ControlMonitoring(cmd *cobra.Command, args []string) { +func ControlMonitoring(cmd *cobra.Command, args []string) error { client, err := utils.CreateKubeClient() if err != nil { - log.Errorf("failed to create cli client: %v", err) - os.Exit(1) + return fmt.Errorf("failed to create cli client: %v", err) } accesslogFlag, _ := cmd.Flags().GetString("accesslog") allFlag, _ := cmd.Flags().GetString("all") workloadMetricsFlag, _ := cmd.Flags().GetString("workloadMetrics") connectionMetricsFlag, _ := cmd.Flags().GetString("connectionMetrics") if accesslogFlag == "" && allFlag == "" && workloadMetricsFlag == "" && connectionMetricsFlag == "" { - log.Print("no parameters. Need --accesslog, --workloadMetrics, --connectionMetrics or --all") - return + return fmt.Errorf("no parameters. Need --accesslog, --workloadMetrics, --connectionMetrics or --all") } podName, hasKmeshPod := getKmeshDaemonPod(args) if hasKmeshPod { // Processes triggers for specified kmesh daemon. if allFlag != "" { - SetObservabilityPerKmeshDaemon(client, podName, allFlag, MONITORING, patternMonitoring) + if err := SetObservabilityPerKmeshDaemon(client, podName, allFlag, MONITORING, patternMonitoring); err != nil { + return err + } } if accesslogFlag != "" { - SetObservabilityPerKmeshDaemon(client, podName, accesslogFlag, ACCESSLOG, patternAccesslog) + if err := SetObservabilityPerKmeshDaemon(client, podName, accesslogFlag, ACCESSLOG, patternAccesslog); err != nil { + return err + } } if workloadMetricsFlag != "" { - SetObservabilityPerKmeshDaemon(client, podName, workloadMetricsFlag, WORKLOAD, patternWorkloadMetrics) + if err := SetObservabilityPerKmeshDaemon(client, podName, workloadMetricsFlag, WORKLOAD, patternWorkloadMetrics); err != nil { + return err + } } if connectionMetricsFlag != "" { - SetObservabilityPerKmeshDaemon(client, podName, connectionMetricsFlag, CONNECTION, patternConnectionMetrics) + if err := SetObservabilityPerKmeshDaemon(client, podName, connectionMetricsFlag, CONNECTION, patternConnectionMetrics); err != nil { + return err + } } } else { // Perform operations on all kmesh daemons. podList, err := client.PodsForSelector(context.TODO(), utils.KmeshNamespace, utils.KmeshLabel) if err != nil { - log.Errorf("failed to get kmesh podList: %v", err) - os.Exit(1) + return fmt.Errorf("failed to get kmesh podList: %v", err) } for _, pod := range podList.Items { if allFlag != "" { - SetObservabilityPerKmeshDaemon(client, pod.GetName(), allFlag, MONITORING, patternMonitoring) + if err := SetObservabilityPerKmeshDaemon(client, pod.GetName(), allFlag, MONITORING, patternMonitoring); err != nil { + return err + } } if accesslogFlag != "" { - SetObservabilityPerKmeshDaemon(client, pod.GetName(), accesslogFlag, ACCESSLOG, patternAccesslog) + if err := SetObservabilityPerKmeshDaemon(client, pod.GetName(), accesslogFlag, ACCESSLOG, patternAccesslog); err != nil { + return err + } } if workloadMetricsFlag != "" { - SetObservabilityPerKmeshDaemon(client, pod.GetName(), workloadMetricsFlag, WORKLOAD, patternWorkloadMetrics) + if err := SetObservabilityPerKmeshDaemon(client, pod.GetName(), workloadMetricsFlag, WORKLOAD, patternWorkloadMetrics); err != nil { + return err + } } if connectionMetricsFlag != "" { - SetObservabilityPerKmeshDaemon(client, pod.GetName(), connectionMetricsFlag, CONNECTION, patternConnectionMetrics) + if err := SetObservabilityPerKmeshDaemon(client, pod.GetName(), connectionMetricsFlag, CONNECTION, patternConnectionMetrics); err != nil { + return err + } } } } + return nil } func getKmeshDaemonPod(args []string) (string, bool) { @@ -153,25 +163,22 @@ func getKmeshDaemonPod(args []string) (string, bool) { return args[0], true } -func SetObservabilityPerKmeshDaemon(cli kube.CLIClient, podName, info string, observablityType string, pattern string) { +func SetObservabilityPerKmeshDaemon(cli kube.CLIClient, podName, info string, observablityType string, pattern string) error { var status string if info == "enable" { status = "true" } else if info == "disable" { status = "false" } else { - log.Errorf("Error: Argument must be 'enable' or 'disable'") - os.Exit(1) + return fmt.Errorf("argument must be 'enable' or 'disable'") } fw, err := utils.CreateKmeshPortForwarder(cli, podName) if err != nil { - log.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err) - os.Exit(1) + return fmt.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err) } if err := fw.Start(); err != nil { - log.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err) - os.Exit(1) + return fmt.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err) } defer fw.Close() @@ -179,32 +186,30 @@ func SetObservabilityPerKmeshDaemon(cli kube.CLIClient, podName, info string, ob req, err := http.NewRequest(http.MethodPost, url, nil) if err != nil { - log.Errorf("Error creating request: %v", err) - return + return fmt.Errorf("error creating request: %v", err) } req.Header.Set("Content-Type", "application/json") client := &http.Client{} resp, err := client.Do(req) if err != nil { - log.Errorf("failed to make HTTP request: %v", err) - return + return fmt.Errorf("failed to make HTTP request: %v", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - log.Errorf("Error: received status code %d", resp.StatusCode) if observablityType == MONITORING { - return + return fmt.Errorf("error: received status code %d", resp.StatusCode) } bodyBytes, readErr := io.ReadAll(resp.Body) if readErr != nil { - log.Errorf("Error reading response body: %v", readErr) - return + return fmt.Errorf("error reading response body: %v", readErr) } bodyString := string(bodyBytes) if resp.StatusCode == http.StatusBadRequest && bytes.Contains(bodyBytes, []byte(fmt.Sprintf("Kmesh monitoring is disable, cannot enable %s.", observablityType))) { - log.Errorf("failed to enable %s: %v. Need to start Kmesh's Monitoring. Please run `kmeshctl monitoring -h` for more help.", observablityType, bodyString) + return fmt.Errorf("failed to enable %s: %v. Need to start Kmesh's Monitoring. Please run `kmeshctl monitoring -h` for more help.", observablityType, bodyString) } + return fmt.Errorf("error: received status code %d", resp.StatusCode) } + return nil } diff --git a/ctl/secret/secret.go b/ctl/secret/secret.go index 58b788ce8..104dbabed 100644 --- a/ctl/secret/secret.go +++ b/ctl/secret/secret.go @@ -22,7 +22,7 @@ import ( "encoding/hex" "encoding/json" "fmt" - "os" + "io" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" @@ -32,11 +32,12 @@ import ( "kmesh.net/kmesh/ctl/utils" "kmesh.net/kmesh/pkg/controller/encryption" "kmesh.net/kmesh/pkg/kube" - "kmesh.net/kmesh/pkg/logger" ) -var log = logger.NewLoggerScope("kmeshctl/secret") -var clientset kube.CLIClient +var ( + clientset kube.CLIClient + clientInit error +) const ( SecretName = "kmesh-ipsec" @@ -46,8 +47,6 @@ const ( ) func NewCmd() *cobra.Command { - clientset = createKubeClientOrExit() - cmd := &cobra.Command{ Use: "secret", Short: "Use secrets to manage secret configuration data for IPsec", @@ -57,7 +56,8 @@ kmeshctl secret get kmeshctl secret delete `, Args: cobra.NoArgs, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { + return nil }, } @@ -70,8 +70,8 @@ kmeshctl secret create # Generate IPsec configuration with user-defined key: kmeshctl secret create --key=$(echo -n "{36-character user-defined key here}" | xxd -p -c 64)`, Args: cobra.NoArgs, - Run: func(cmd *cobra.Command, args []string) { - CreateOrUpdateSecret(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return CreateOrUpdateSecret(cmd, args) }, } @@ -84,8 +84,8 @@ kmeshctl secret create --key=$(echo -n "{36-character user-defined key here}" | Example: `# Get IPsec key and configuration by kmeshctl. The results will be displayed in JSON format. kmeshctl secret get`, Args: cobra.NoArgs, - Run: func(cmd *cobra.Command, args []string) { - GetSecret() + RunE: func(cmd *cobra.Command, args []string) error { + return GetSecret(cmd.OutOrStdout()) }, } @@ -95,8 +95,8 @@ kmeshctl secret get`, Short: "Delete IPsec key and configuration by kmeshctl", Example: `kmeshctl secret delete`, Args: cobra.NoArgs, - Run: func(cmd *cobra.Command, args []string) { - DeleteSecret() + RunE: func(cmd *cobra.Command, args []string) error { + return DeleteSecret() }, } @@ -108,18 +108,31 @@ kmeshctl secret get`, return cmd } -func createKubeClientOrExit() kube.CLIClient { - clientset, err := utils.CreateKubeClient() +func ensureClientset() (kube.CLIClient, error) { + if clientset == nil && clientInit == nil { + clientset, clientInit = createKubeClient() + } + if clientInit != nil { + return nil, clientInit + } + return clientset, nil +} + +func createKubeClient() (kube.CLIClient, error) { + cli, err := utils.CreateKubeClient() if err != nil { - log.Errorf("failed to connect k8s client, %v", err) - os.Exit(1) + return nil, fmt.Errorf("failed to connect k8s client, %v", err) } - return clientset + return cli, nil } -func CreateOrUpdateSecret(cmd *cobra.Command, args []string) { +func CreateOrUpdateSecret(cmd *cobra.Command, args []string) error { + cli, err := ensureClientset() + if err != nil { + return err + } + var ipSecKey, ipSecKeyOld encryption.IpSecKey - var err error ipSecKey.AeadKeyName = AeadAlgoName @@ -131,46 +144,40 @@ func CreateOrUpdateSecret(cmd *cobra.Command, args []string) { aeadKey = make([]byte, AeadKeyLength) _, err := rand.Read(aeadKey) if err != nil { - log.Errorf("failed to generate random key: %v", err) - os.Exit(1) + return fmt.Errorf("failed to generate random key: %v", err) } } else { aeadKey, err = hex.DecodeString(aeadKeyArg) if err != nil { - log.Errorf("failed to decode hex string: %v, input: %v", err, aeadKeyArg) - os.Exit(1) + return fmt.Errorf("failed to decode hex string: %v, input: %v", err, aeadKeyArg) } } if len(aeadKey) != AeadKeyLength { - log.Errorf("invalid key length: expected %d bytes, got %d bytes (key must be 256-bit + 32-bit salt)", AeadKeyLength, len(aeadKey)) - os.Exit(1) + return fmt.Errorf("invalid key length: expected %d bytes, got %d bytes (key must be 256-bit + 32-bit salt)", AeadKeyLength, len(aeadKey)) } ipSecKey.AeadKey = aeadKey ipSecKey.Length = AeadAlgoICVLength - secretOld, err := clientset.Kube().CoreV1().Secrets(utils.KmeshNamespace).Get(context.TODO(), SecretName, metav1.GetOptions{}) + secretOld, err := cli.Kube().CoreV1().Secrets(utils.KmeshNamespace).Get(context.TODO(), SecretName, metav1.GetOptions{}) if err != nil { if !apierrors.IsNotFound(err) { - log.Errorf("failed to get secret: %v, %v", SecretName, err) - os.Exit(1) + return fmt.Errorf("failed to get secret: %v, %v", SecretName, err) } ipSecKey.Spi = 1 } else { err = json.Unmarshal(secretOld.Data["ipSec"], &ipSecKeyOld) if err != nil { - log.Errorf("failed to unmarshal secret: %v, %v", secretOld, err) - os.Exit(1) + return fmt.Errorf("failed to unmarshal secret: %v, %v", secretOld, err) } ipSecKey.Spi = ipSecKeyOld.Spi + 1 } secretData, err := json.Marshal(ipSecKey) if err != nil { - log.Errorf("failed to convert ipsec key to secret data, %v", err) - os.Exit(1) + return fmt.Errorf("failed to convert ipsec key to secret data, %v", err) } secret := &corev1.Secret{ @@ -184,41 +191,41 @@ func CreateOrUpdateSecret(cmd *cobra.Command, args []string) { } if ipSecKey.Spi == 1 { - _, err = clientset.Kube().CoreV1().Secrets(utils.KmeshNamespace).Create(context.TODO(), secret, metav1.CreateOptions{}) + _, err = cli.Kube().CoreV1().Secrets(utils.KmeshNamespace).Create(context.TODO(), secret, metav1.CreateOptions{}) if err != nil { - log.Errorf("failed to create %v secret, %v", SecretName, err) - os.Exit(1) + return fmt.Errorf("failed to create %v secret, %v", SecretName, err) } } else { - _, err = clientset.Kube().CoreV1().Secrets(utils.KmeshNamespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) + _, err = cli.Kube().CoreV1().Secrets(utils.KmeshNamespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) if err != nil { - log.Errorf("failed to update %v secret, %v", SecretName, err) - os.Exit(1) + return fmt.Errorf("failed to update %v secret, %v", SecretName, err) } } + return nil } -func GetSecret() { - secret, err := clientset.Kube().CoreV1().Secrets(utils.KmeshNamespace).Get(context.TODO(), SecretName, metav1.GetOptions{}) +func GetSecret(out io.Writer) error { + cli, err := ensureClientset() + if err != nil { + return err + } + + secret, err := cli.Kube().CoreV1().Secrets(utils.KmeshNamespace).Get(context.TODO(), SecretName, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { - log.Errorf("secret %s not found", SecretName) - os.Exit(1) + return fmt.Errorf("secret %s not found", SecretName) } - log.Errorf("failed to get secret: %v", err) - os.Exit(1) + return fmt.Errorf("failed to get secret: %v", err) } if secret.Data == nil || secret.Data["ipSec"] == nil { - log.Errorf("invalid secret data: missing ipSec field") - os.Exit(1) + return fmt.Errorf("invalid secret data: missing ipSec field") } // Parse the IPsec data var ipSecKey encryption.IpSecKey if err := json.Unmarshal(secret.Data["ipSec"], &ipSecKey); err != nil { - log.Errorf("failed to unmarshal secret data: %v", err) - os.Exit(1) + return fmt.Errorf("failed to unmarshal secret data: %v", err) } // Create a display structure with hex string key @@ -236,25 +243,29 @@ func GetSecret() { displayData, err := json.MarshalIndent(displayKey, "", " ") if err != nil { - log.Errorf("failed to marshal display data: %v", err) - os.Exit(1) + return fmt.Errorf("failed to marshal display data: %v", err) } - fmt.Printf("Secret name: %s\n", SecretName) - fmt.Printf("Namespace: %s\n", utils.KmeshNamespace) - fmt.Printf("Created: %s\n", secret.CreationTimestamp.Format("2006-01-02 15:04:05")) - fmt.Println("IPsec Configuration:") - fmt.Println(string(displayData)) + fmt.Fprintf(out, "Secret name: %s\n", SecretName) + fmt.Fprintf(out, "Namespace: %s\n", utils.KmeshNamespace) + fmt.Fprintf(out, "Created: %s\n", secret.CreationTimestamp.Format("2006-01-02 15:04:05")) + fmt.Fprintln(out, "IPsec Configuration:") + fmt.Fprintln(out, string(displayData)) + return nil } -func DeleteSecret() { - err := clientset.Kube().CoreV1().Secrets(utils.KmeshNamespace).Delete(context.TODO(), SecretName, metav1.DeleteOptions{}) +func DeleteSecret() error { + cli, err := ensureClientset() + if err != nil { + return err + } + + err = cli.Kube().CoreV1().Secrets(utils.KmeshNamespace).Delete(context.TODO(), SecretName, metav1.DeleteOptions{}) if err != nil { if apierrors.IsNotFound(err) { - log.Errorf("secret %s not found", SecretName) - os.Exit(1) + return fmt.Errorf("secret %s not found", SecretName) } - log.Errorf("failed to delete secret: %v", err) - os.Exit(1) + return fmt.Errorf("failed to delete secret: %v", err) } + return nil } diff --git a/ctl/version/version.go b/ctl/version/version.go index 3ef5ce547..77b8f0635 100644 --- a/ctl/version/version.go +++ b/ctl/version/version.go @@ -22,7 +22,6 @@ import ( "fmt" "io" "net/http" - "os" "regexp" "strings" @@ -45,19 +44,18 @@ kmeshctl version # Show version info of a specific kmesh daemon kmeshctl version `, - Run: func(cmd *cobra.Command, args []string) { - runVersion(cmd, args) + RunE: func(cmd *cobra.Command, args []string) error { + return runVersion(cmd, args) }, } return cmd } // runVersion output the version info of kmeshctl or kmesh-daemon. -func runVersion(cmd *cobra.Command, args []string) { +func runVersion(cmd *cobra.Command, args []string) error { cli, err := utils.CreateKubeClient() if err != nil { - log.Errorf("failed to create kube client: %v", err) - os.Exit(1) + return fmt.Errorf("failed to create kube client: %v", err) } if len(args) == 0 { @@ -70,8 +68,7 @@ func runVersion(cmd *cobra.Command, args []string) { podList, err := cli.PodsForSelector(context.TODO(), utils.KmeshNamespace, utils.KmeshLabel) if err != nil { - log.Errorf("failed to get kmesh daemon pods: %v", err) - os.Exit(1) + return fmt.Errorf("failed to get kmesh daemon pods: %v", err) } daemonVersions := map[string]int{} @@ -91,7 +88,7 @@ func runVersion(cmd *cobra.Command, args []string) { counts = append(counts, fmt.Sprintf("%s (%d daemons)", k, v)) } cmd.Printf("%s\n", strings.Join(counts, ", ")) - return + return nil } podName := args[0] @@ -99,11 +96,11 @@ func runVersion(cmd *cobra.Command, args []string) { if v.GitVersion != "" { data, err := json.MarshalIndent(&v, "", " ") if err != nil { - log.Errorf("Failed to marshal version info: %v", err) - os.Exit(1) + return fmt.Errorf("failed to marshal version info: %v", err) } cmd.Printf("%s\n", string(data)) } + return nil } func getVersion(client kube.CLIClient, podName string) (version version.Info) { diff --git a/test/e2e/baseline_test.go b/test/e2e/baseline_test.go index 9b4a22277..b75d31439 100644 --- a/test/e2e/baseline_test.go +++ b/test/e2e/baseline_test.go @@ -725,9 +725,9 @@ func TestRemoveAddNsOrServiceWaypoint(t *testing.T) { // and all pass through waypoint. func TestMixNsAndServiceWaypoint(t *testing.T) { framework.NewTest(t).Run(func(t framework.TestContext) { - waypoint := "namespace-waypoint" + waypoint := "mix-namespace-waypoint" - newWaypointProxyOrFail(t, t, apps.Namespace, waypoint, constants.ServiceTraffic) + newWaypointProxyOrFail(t, t, apps.Namespace, waypoint, constants.AllTraffic) t.Cleanup(func() { deleteWaypointProxyOrFail(t, t, apps.Namespace, waypoint) }) @@ -737,6 +737,9 @@ func TestMixNsAndServiceWaypoint(t *testing.T) { UnsetWaypoint(t, apps.Namespace.Name(), "", Namespace) }) + // Wait for waypoint configuration to propagate to eBPF/XDS + time.Sleep(10 * time.Second) + runTestContext(t, func(t framework.TestContext, src echo.Instance, dst echo.Instance, opt echo.CallOptions) { if opt.Scheme != scheme.HTTP { return @@ -804,9 +807,9 @@ func TestBookinfo(t *testing.T) { // Set namespace waypoint to verify that bookinfo could be accessed normally event if each hop // is processed by waypoint. - waypoint := "namespace-waypoint" + waypoint := "bookinfo-namespace-waypoint" - newWaypointProxyOrFail(t, t, apps.Namespace, waypoint, constants.ServiceTraffic) + newWaypointProxyOrFail(t, t, apps.Namespace, waypoint, constants.AllTraffic) t.Cleanup(func() { deleteWaypointProxyOrFail(t, t, apps.Namespace, waypoint) }) @@ -816,6 +819,9 @@ func TestBookinfo(t *testing.T) { UnsetWaypoint(t, namespace, "", Namespace) }) + // Wait for waypoint configuration to propagate to eBPF/XDS + time.Sleep(10 * time.Second) + if err := retry.Until(checkBookinfo, retry.Timeout(900*time.Second), retry.Delay(3*time.Second)); err != nil { t.Fatal("failed to access bookinfo correctly when there is a namespace waypoint: %v", err) } diff --git a/test/e2e/manage_test.go b/test/e2e/manage_test.go index f80b9243a..65296ec01 100644 --- a/test/e2e/manage_test.go +++ b/test/e2e/manage_test.go @@ -27,6 +27,7 @@ import ( "context" "fmt" "testing" + "time" "istio.io/api/label" "istio.io/istio/pkg/config/constants" @@ -189,7 +190,7 @@ func TestCrossNamespace(t *testing.T) { dst := apps.ServiceWithWaypointAtServiceGranularity - unenrolledNSTest := func() { + unenrolledNSTest := func(t framework.TestContext) { tests := []struct { svc echo.Instances enrolled bool @@ -224,10 +225,11 @@ func TestCrossNamespace(t *testing.T) { } t.NewSubTest("cross namespace access, the new namespace is not managed by Kmesh").Run(func(t framework.TestContext) { - unenrolledNSTest() + unenrolledNSTest(t) }) enrollNamespaceOrFail(t, anotherNS.Name()) + time.Sleep(5 * time.Second) t.NewSubTest("cross namespace access, the new namespace is managed by Kmesh").Run(func(t framework.TestContext) { for _, src := range all { @@ -244,9 +246,10 @@ func TestCrossNamespace(t *testing.T) { }) unenrollNamespaceOrFail(t, anotherNS.Name()) + time.Sleep(5 * time.Second) t.NewSubTest("cross namespace access, the new namespace is not managed by Kmesh **AGAIN**").Run(func(t framework.TestContext) { - unenrolledNSTest() + unenrolledNSTest(t) }) }) } diff --git a/test/e2e/run_test.sh b/test/e2e/run_test.sh index 37580d8b1..03ea44ff5 100755 --- a/test/e2e/run_test.sh +++ b/test/e2e/run_test.sh @@ -188,7 +188,7 @@ function setup_kmesh() { # Set BPF debug log for i in {1..5}; do echo "Attempt $i of 5: kmeshctl log $POD --set bpf:debug" - output=$(kmeshctl log $POD --set bpf:debug 2>&1) + output=$(kmeshctl log $POD --set bpf:debug 2>&1 || true) if echo "$output" | grep -q "set BPF Log Level: 3"; then echo "BPF debug log set successfully" break @@ -201,7 +201,7 @@ function setup_kmesh() { # Set default debug log for i in {1..5}; do echo "Attempt $i of 5: kmeshctl log $POD --set default:debug" - output=$(kmeshctl log $POD --set default:debug 2>&1) + output=$(kmeshctl log $POD --set default:debug 2>&1 || true) if echo "$output" | grep -q "OK"; then echo "Default debug log set successfully" break @@ -222,7 +222,7 @@ function setup_kmesh_log() { # Set BPF debug log for i in {1..5}; do echo "Attempt $i of 5: kmeshctl log $POD --set bpf:debug" - output=$(kmeshctl log $POD --set bpf:debug 2>&1) + output=$(kmeshctl log $POD --set bpf:debug 2>&1 || true) if echo "$output" | grep -q "set BPF Log Level: 3"; then echo "BPF debug log set successfully" break @@ -235,7 +235,7 @@ function setup_kmesh_log() { # Set default debug log for i in {1..5}; do echo "Attempt $i of 5: kmeshctl log $POD --set default:debug" - output=$(kmeshctl log $POD --set default:debug 2>&1) + output=$(kmeshctl log $POD --set default:debug 2>&1 || true) if echo "$output" | grep -q "OK"; then echo "Default debug log set successfully" break @@ -440,19 +440,11 @@ bash -c "$cmd" EXIT_CODE=$? set -e -# Log collection is diagnostic and must not replace the test exit status. -if [ "$EXIT_CODE" -ne 0 ]; then - echo "E2E tests failed with exit code $EXIT_CODE." - if [[ -r $LOGFILE ]]; then - cat "$LOGFILE" || echo "Failed to read Kmesh daemon log: $LOGFILE" - elif [[ ${DEBUG:-false} == "true" ]]; then - echo "Kmesh daemon log was not created: $LOGFILE" - else - echo "Kmesh daemon log was not captured; rerun with --debug to enable log capture." - fi +if [ $EXIT_CODE -ne 0 ]; then + cat $LOGFILE fi -rm -f -- "$LOGFILE" || echo "Failed to remove Kmesh daemon log: $LOGFILE" +rm -rf $LOGFILE if [[ -n ${CLEANUP_KIND} ]]; then cleanup_kind_cluster @@ -462,4 +454,4 @@ if [[ -n ${CLEANUP_REGISTRY} ]]; then cleanup_docker_registry fi -exit "$EXIT_CODE" +exit $EXIT_CODE