fix some client usages and drop config in CLIContext (istio#45507)

WillardHu · Jun 21, 2023 · fcfc1d0 · fcfc1d0
1 parent 314baf1
commit fcfc1d0
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 38 deletions.
diff --git a/istioctl/cmd/root.go b/istioctl/cmd/root.go
@@ -118,7 +118,7 @@ debug and diagnose their Istio mesh.
 	flags := rootCmd.PersistentFlags()
 	rootOptions := cli.AddRootFlags(flags)
 
-	ctx := cli.NewCLIContext(*rootOptions)
+	ctx := cli.NewCLIContext(rootOptions)
 
 	rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
 		if err := configureLogging(cmd, args); err != nil {
@@ -241,7 +241,7 @@ debug and diagnose their Istio mesh.
 	hideInheritedFlags(upgradeCmd, cli.FlagNamespace, cli.FlagIstioNamespace, FlagCharts)
 	rootCmd.AddCommand(upgradeCmd)
 
-	bugReportCmd := bugreport.Cmd(root.LoggingOptions)
+	bugReportCmd := bugreport.Cmd(ctx, root.LoggingOptions)
 	hideInheritedFlags(bugReportCmd, cli.FlagNamespace, cli.FlagIstioNamespace)
 	rootCmd.AddCommand(bugReportCmd)
 

diff --git a/istioctl/pkg/cli/context.go b/istioctl/pkg/cli/context.go
@@ -41,11 +41,6 @@ type Context interface {
 	// ConfigureDefaultNamespace sets the default namespace to use for commands that don't specify a namespace.
 	// This should be called before NamespaceOrDefault is called.
 	ConfigureDefaultNamespace()
-	// KubeConfig returns the kubeconfig specified by the user
-	KubeConfig() string
-	// KubeContext returns the kubecontext specified by the user
-	KubeContext() string
-	// TODO(hanxiaop) entirely drop KubeConfig and KubeContext, use CLIClient instead. Currently this is used not only in istioctl package.
 }
 
 type instance struct {
@@ -67,9 +62,18 @@ func newKubeClientWithRevision(kubeconfig, configContext, revision string) (kube
 	return kube.NewCLIClient(kube.NewClientConfigForRestConfig(rc), revision)
 }
 
-func NewCLIContext(rootFlags RootFlags) Context {
+func NewCLIContext(rootFlags *RootFlags) Context {
+	if rootFlags == nil {
+		rootFlags = &RootFlags{
+			kubeconfig:       ptr.Of[string](""),
+			configContext:    ptr.Of[string](""),
+			namespace:        ptr.Of[string](""),
+			istioNamespace:   ptr.Of[string](""),
+			defaultNamespace: "",
+		}
+	}
 	return &instance{
-		RootFlags: rootFlags,
+		RootFlags: *rootFlags,
 	}
 }
 

diff --git a/istioctl/pkg/version/version.go b/istioctl/pkg/version/version.go
@@ -89,7 +89,11 @@ func getRemoteInfoWrapper(ctx cli.Context, pc **cobra.Command, opts *clioptions.
 
 func getProxyInfoWrapper(ctx cli.Context, opts *clioptions.ControlPlaneOptions) func() (*[]istioVersion.ProxyInfo, error) {
 	return func() (*[]istioVersion.ProxyInfo, error) {
-		return proxy.GetProxyInfo(ctx.KubeConfig(), ctx.KubeContext(), opts.Revision, ctx.IstioNamespace())
+		client, err := ctx.CLIClientWithRevision(opts.Revision)
+		if err != nil {
+			return nil, err
+		}
+		return proxy.GetProxyInfo(client, ctx.IstioNamespace())
 	}
 }
 

diff --git a/operator/cmd/mesh/uninstall.go b/operator/cmd/mesh/uninstall.go
@@ -35,6 +35,7 @@ import (
 	"istio.io/istio/operator/pkg/util/clog"
 	"istio.io/istio/operator/pkg/util/progress"
 	"istio.io/istio/pkg/config/constants"
+	"istio.io/istio/pkg/kube"
 	"istio.io/istio/pkg/log"
 	proxyinfo "istio.io/istio/pkg/proxy"
 )
@@ -134,6 +135,15 @@ func uninstall(cmd *cobra.Command, rootArgs *RootArgs, uiArgs *uninstallArgs, lo
 	if err != nil {
 		l.LogAndFatal(err)
 	}
+	var kubeClientWithRev kube.CLIClient
+	if uiArgs.revision != "" && uiArgs.revision != "default" {
+		kubeClientWithRev, err = kube.NewCLIClient(kube.BuildClientCmd(uiArgs.kubeConfigPath, uiArgs.context), uiArgs.revision)
+		if err != nil {
+			return err
+		}
+	} else {
+		kubeClientWithRev = kubeClient
+	}
 
 	if uiArgs.revision != "" {
 		revisions, err := tag.ListRevisionDescriptions(kubeClient)
@@ -170,7 +180,7 @@ func uninstall(cmd *cobra.Command, rootArgs *RootArgs, uiArgs *uninstallArgs, lo
 		if err != nil {
 			return err
 		}
-		preCheckWarnings(cmd, uiArgs, uiArgs.revision, objectsList, nil, l)
+		preCheckWarnings(cmd, kubeClientWithRev, uiArgs, uiArgs.revision, objectsList, nil, l)
 
 		if err := h.DeleteObjectsList(objectsList, ""); err != nil {
 			return fmt.Errorf("failed to delete control plane resources by revision: %v", err)
@@ -188,7 +198,7 @@ func uninstall(cmd *cobra.Command, rootArgs *RootArgs, uiArgs *uninstallArgs, lo
 	if err != nil {
 		return err
 	}
-	preCheckWarnings(cmd, uiArgs, iop.Spec.Revision, nil, cpObjects, l)
+	preCheckWarnings(cmd, kubeClientWithRev, uiArgs, iop.Spec.Revision, nil, cpObjects, l)
 	h, err = helmreconciler.NewHelmReconciler(client, kubeClient, iop, opts)
 	if err != nil {
 		return fmt.Errorf("failed to create reconciler: %v", err)
@@ -203,10 +213,10 @@ func uninstall(cmd *cobra.Command, rootArgs *RootArgs, uiArgs *uninstallArgs, lo
 // preCheckWarnings checks possible breaking changes and issue warnings to users, it checks the following:
 // 1. checks proxies still pointing to the target control plane revision.
 // 2. lists to be pruned resources if user uninstall by --revision flag.
-func preCheckWarnings(cmd *cobra.Command, uiArgs *uninstallArgs,
+func preCheckWarnings(cmd *cobra.Command, kubeClient kube.CLIClient, uiArgs *uninstallArgs,
 	rev string, resourcesList []*unstructured.UnstructuredList, objectsList object.K8sObjects, l *clog.ConsoleLogger,
 ) {
-	pids, err := proxyinfo.GetIDsFromProxyInfo(uiArgs.kubeConfigPath, uiArgs.context, rev, uiArgs.istioNamespace)
+	pids, err := proxyinfo.GetIDsFromProxyInfo(kubeClient, uiArgs.istioNamespace)
 	if err != nil {
 		l.LogAndError(err.Error())
 	}

diff --git a/operator/pkg/helmreconciler/prune.go b/operator/pkg/helmreconciler/prune.go
@@ -167,7 +167,7 @@ func (h *HelmReconciler) PruneControlPlaneByRevisionWithController(iopSpec *v1al
 		if pilotExists {
 			// TODO(ramaraochavali): Find a better alternative instead of using debug interface
 			// of istiod as it is typically not recommended in production environments.
-			pids, err := proxy.GetIDsFromProxyInfo("", "", iopSpec.Revision, ns)
+			pids, err := proxy.GetIDsFromProxyInfo(kubeClient, ns)
 			if err != nil {
 				return errStatus, fmt.Errorf("failed to check proxy infos: %v", err)
 			}

diff --git a/pkg/proxy/proxyinfo.go b/pkg/proxy/proxyinfo.go
@@ -31,11 +31,7 @@ type sidecarSyncStatus struct {
 }
 
 // GetProxyInfo retrieves infos of proxies that connect to the Istio control plane of specific revision.
-func GetProxyInfo(kubeconfig, configContext, revision, istioNamespace string) (*[]istioVersion.ProxyInfo, error) {
-	kubeClient, err := kube.NewCLIClient(kube.BuildClientCmd(kubeconfig, configContext), revision)
-	if err != nil {
-		return nil, err
-	}
+func GetProxyInfo(kubeClient kube.CLIClient, istioNamespace string) (*[]istioVersion.ProxyInfo, error) {
 	// Ask Pilot for the Envoy sidecar sync status, which includes the sidecar version info
 	allSyncz, err := kubeClient.AllDiscoveryDo(context.TODO(), istioNamespace, "debug/syncz")
 	if err != nil {
@@ -63,9 +59,9 @@ func GetProxyInfo(kubeconfig, configContext, revision, istioNamespace string) (*
 }
 
 // GetIDsFromProxyInfo is a helper function to retrieve list of IDs from Proxy.
-func GetIDsFromProxyInfo(kubeconfig, configContext, revision, istioNamespace string) ([]string, error) {
+func GetIDsFromProxyInfo(kubeClient kube.CLIClient, istioNamespace string) ([]string, error) {
 	var IDs []string
-	pi, err := GetProxyInfo(kubeconfig, configContext, revision, istioNamespace)
+	pi, err := GetProxyInfo(kubeClient, istioNamespace)
 	if err != nil {
 		return IDs, fmt.Errorf("failed to get proxy infos: %v", err)
 	}

diff --git a/tools/bug-report/main.go b/tools/bug-report/main.go
@@ -18,12 +18,14 @@ import (
 	"fmt"
 	"os"
 
+	"istio.io/istio/istioctl/pkg/cli"
 	"istio.io/istio/pkg/log"
 	"istio.io/istio/tools/bug-report/pkg/bugreport"
 )
 
 func main() {
-	if err := bugreport.Cmd(log.DefaultOptions()).Execute(); err != nil {
+	ctx := cli.NewCLIContext(nil)
+	if err := bugreport.Cmd(ctx, log.DefaultOptions()).Execute(); err != nil {
 		fmt.Println(err)
 		os.Exit(-1)
 	}

diff --git a/tools/bug-report/pkg/bugreport/bugreport.go b/tools/bug-report/pkg/bugreport/bugreport.go
@@ -30,6 +30,7 @@ import (
 	"github.com/spf13/cobra"
 
 	label2 "istio.io/api/label"
+	"istio.io/istio/istioctl/pkg/cli"
 	"istio.io/istio/istioctl/pkg/util/ambient"
 	"istio.io/istio/operator/pkg/util"
 	"istio.io/istio/pkg/kube"
@@ -60,7 +61,7 @@ var (
 )
 
 // Cmd returns a cobra command for bug-report.
-func Cmd(logOpts *log.Options) *cobra.Command {
+func Cmd(ctx cli.Context, logOpts *log.Options) *cobra.Command {
 	rootCmd := &cobra.Command{
 		Use:          "bug-report",
 		Short:        "Cluster information and log capture support tool.",
@@ -79,7 +80,7 @@ e.g.
 --include ns1,ns2 (only namespaces ns1 and ns2)
 --include n*//p*/l=v* (pods with name beginning with 'p' in namespaces beginning with 'n' and having label 'l' with value beginning with 'v'.)`,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runBugReportCommand(cmd, logOpts)
+			return runBugReportCommand(ctx, cmd, logOpts)
 		},
 	}
 	rootCmd.AddCommand(version.CobraCommand())
@@ -99,7 +100,7 @@ var (
 	lock    = sync.RWMutex{}
 )
 
-func runBugReportCommand(_ *cobra.Command, logOpts *log.Options) error {
+func runBugReportCommand(ctx cli.Context, _ *cobra.Command, logOpts *log.Options) error {
 	runner := kubectlcmd.NewRunner(gConfig.RequestsPerSecondLimit)
 	runner.ReportRunningTasks()
 	if err := configLogs(logOpts); err != nil {
@@ -149,7 +150,7 @@ func runBugReportCommand(_ *cobra.Command, logOpts *log.Options) error {
 	}
 	logRuntime(curTime, "Done collecting cluster resource")
 
-	dumpRevisionsAndVersions(resources, config.KubeConfigPath, config.Context, config.IstioNamespace, config.DryRun)
+	dumpRevisionsAndVersions(ctx, resources, config.IstioNamespace, config.DryRun)
 
 	log.Infof("Cluster resource tree:\n\n%s\n\n", resources)
 	paths, err := filter.GetMatchingPaths(config, resources)
@@ -209,14 +210,14 @@ func runBugReportCommand(_ *cobra.Command, logOpts *log.Options) error {
 	return nil
 }
 
-func dumpRevisionsAndVersions(resources *cluster2.Resources, kubeconfig, configContext, istioNamespace string, dryRun bool) {
+func dumpRevisionsAndVersions(ctx cli.Context, resources *cluster2.Resources, istioNamespace string, dryRun bool) {
 	defer logRuntime(time.Now(), "Done getting control plane revisions/versions")
 
 	text := ""
 	text += fmt.Sprintf("CLI version:\n%s\n\n", version.Info.LongForm())
 
 	revisions := getIstioRevisions(resources)
-	istioVersions, proxyVersions := getIstioVersions(kubeconfig, configContext, istioNamespace, revisions)
+	istioVersions, proxyVersions := getIstioVersions(ctx, istioNamespace, revisions)
 	text += "The following Istio control plane revisions/versions were found in the cluster:\n"
 	for rev, ver := range istioVersions {
 		text += fmt.Sprintf("Revision %s:\n%s\n\n", rev, ver)
@@ -244,13 +245,18 @@ func getIstioRevisions(resources *cluster2.Resources) []string {
 
 // getIstioVersions returns a mapping of revision to aggregated version string for Istio components and revision to
 // slice of versions for proxies. Any errors are embedded in the revision strings.
-func getIstioVersions(kubeconfig, configContext, istioNamespace string, revisions []string) (map[string]string, map[string][]string) {
+func getIstioVersions(ctx cli.Context, istioNamespace string, revisions []string) (map[string]string, map[string][]string) {
 	istioVersions := make(map[string]string)
 	proxyVersionsMap := make(map[string]sets.String)
 	proxyVersions := make(map[string][]string)
 	for _, revision := range revisions {
-		istioVersions[revision] = getIstioVersion(kubeconfig, configContext, istioNamespace, revision)
-		proxyInfo, err := proxy.GetProxyInfo(kubeconfig, configContext, revision, istioNamespace)
+		client, err := ctx.CLIClientWithRevision(revision)
+		if err != nil {
+			log.Error(err)
+			continue
+		}
+		istioVersions[revision] = getIstioVersion(client, istioNamespace)
+		proxyInfo, err := proxy.GetProxyInfo(client, istioNamespace)
 		if err != nil {
 			log.Error(err)
 			continue
@@ -267,12 +273,7 @@ func getIstioVersions(kubeconfig, configContext, istioNamespace string, revision
 	return istioVersions, proxyVersions
 }
 
-func getIstioVersion(kubeconfig, configContext, istioNamespace, revision string) string {
-	kubeClient, err := kube.NewCLIClient(kube.BuildClientCmd(kubeconfig, configContext), revision)
-	if err != nil {
-		return err.Error()
-	}
-
+func getIstioVersion(kubeClient kube.CLIClient, istioNamespace string) string {
 	versions, err := kubeClient.GetIstioVersions(context.TODO(), istioNamespace)
 	if err != nil {
 		return err.Error()