fix review comments

Signed-off-by: hejianpeng <hejianpeng2@huawei.com>

cmd/kurator/app/install/install.go

@@ -22,7 +22,7 @@ import (
 	"kurator.dev/kurator/cmd/kurator/app/install/istio"
 	"kurator.dev/kurator/cmd/kurator/app/install/karmada"
 	"kurator.dev/kurator/cmd/kurator/app/install/kubeedge"
-	"kurator.dev/kurator/cmd/kurator/app/install/pixie/vizier"
+	"kurator.dev/kurator/cmd/kurator/app/install/pixie"
 	"kurator.dev/kurator/cmd/kurator/app/install/prometheus"
 	"kurator.dev/kurator/cmd/kurator/app/install/submariner"
 	"kurator.dev/kurator/cmd/kurator/app/install/volcano"
@@ -45,6 +45,6 @@ func NewCmd(opts *generic.Options) *cobra.Command {
 	installCmd.AddCommand(volcano.NewCmd(opts))
 	installCmd.AddCommand(prometheus.NewCmd(opts))
 	installCmd.AddCommand(submariner.NewCmd(opts))
-	installCmd.AddCommand(vizier.NewCmd(opts))
+	installCmd.AddCommand(pixie.NewCmd(opts))
 	return installCmd
 }

cmd/kurator/app/install/pixie/pixie.go

@@ -0,0 +1,39 @@
+/*
+Copyright Kurator Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package pixie
+
+import (
+	"github.com/spf13/cobra"
+
+	"kurator.dev/kurator/cmd/kurator/app/install/pixie/vizier"
+	"kurator.dev/kurator/pkg/generic"
+)
+
+func NewCmd(opts *generic.Options) *cobra.Command {
+	pixieCmd := &cobra.Command{
+		Use:                   "pixie",
+		Short:                 "Install pixie  component",
+		DisableFlagsInUseLine: true,
+		FParseErrWhitelist: cobra.FParseErrWhitelist{
+			UnknownFlags: true,
+		},
+	}
+
+	pixieCmd.AddCommand(vizier.NewCmd(opts))
+
+	return pixieCmd
+}

cmd/kurator/app/install/pixie/vizier/vizier.go

@@ -35,28 +35,27 @@ var pluginArgs = plugin.InstallArgs{}
 
 func NewCmd(opts *generic.Options) *cobra.Command {
 	vizierCmd := &cobra.Command{
-		Use:   "pixie-vizier",
-		Short: "Install pixie vizier component",
+		Use:   "vizier",
+		Short: "Install vizier component",
 		RunE: func(c *cobra.Command, args []string) error {
 			plugin, err := plugin.NewPlugin(opts, &pluginArgs)
 			if err != nil {
 				logrus.Errorf("pixie vizier init error: %v", err)
-				return fmt.Errorf("pixie init error: %v", err)
+				return fmt.Errorf("pixie vizier init error: %v", err)
 			}
 
-			logrus.Debugf("start install pixie Global: %+v ", opts)
 			if err := plugin.Execute(args, nil); err != nil {
-				logrus.Errorf("pixie execute error: %v", err)
-				return fmt.Errorf("pixie execute error: %v", err)
+				logrus.Errorf("pixie vizier execute error: %v", err)
+				return fmt.Errorf("pixie vizier execute error: %v", err)
 			}
-			logrus.Info("pixie install completed.")
+			logrus.Info("pixie vizier install completed.")
 			return nil
 		},
 	}
 
-	vizierCmd.PersistentFlags().StringVar(&pluginArgs.PxNamespace, "px-namespace", pxNamespace, "The namespace use to install Vizier.")
-	vizierCmd.PersistentFlags().StringVar(&pluginArgs.CloudAddress, "cloud-addr", communityCloudAddr, "The address of the Pixie cloud instance that the Vizier should be connected to.")
-	vizierCmd.PersistentFlags().StringVar(&pluginArgs.DeployKey, "deploy-key", "", "The deploy key is used to link the deployed Vizier to a specific user/project.")
+	vizierCmd.PersistentFlags().StringVar(&pluginArgs.PxNamespace, "px-namespace", pxNamespace, "The namespace use to install vizier.")
+	vizierCmd.PersistentFlags().StringVar(&pluginArgs.CloudAddress, "cloud-addr", communityCloudAddr, "The address of the Pixie cloud instance that the vizier should be connected to.")
+	vizierCmd.PersistentFlags().StringVar(&pluginArgs.DeployKey, "deploy-key", "", "The deploy key is used to link the deployed vizier to a specific user/project.")
 
 	return vizierCmd
 }

pkg/client/client.go

@@ -144,6 +144,14 @@ func (c *Client) NewClusterClientSet(clusterName string) (kubeclient.Interface,
 	return kubeclient.NewForConfig(clusterConfig)
 }
 
+func (c *Client) NewClusterCRDClientset(clusterName string) (crdclientset.Interface, error) {
+	clusterConfig, err := c.memberClusterConfig(clusterName)
+	if err != nil {
+		return nil, err
+	}
+	return crdclientset.NewForConfig(clusterConfig)
+}
+
 func (c *Client) NewClusterHelmClient(clusterName string) (helmclient.Interface, error) {
 	clusterConfig, err := c.memberClusterConfig(clusterName)
 	if err != nil {

pkg/plugin/pixie/vizier/plugin.go

@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"path"
 	"path/filepath"
 	"runtime"
 
@@ -40,8 +39,8 @@ import (
 )
 
 const (
-	crdKind    = "CustomResourceDefinition"
-	vizierFile = "pixie-vizier.yaml"
+	crdKind       = "CustomResourceDefinition"
+	vizierCRDName = "viziers.px.dev"
 
 	repoName    = "pixie-operator"
 	repoAddress = "https://pixie-operator-charts.storage.googleapis.com"
@@ -80,12 +79,9 @@ func NewPlugin(s *generic.Options, args *InstallArgs) (*Plugin, error) {
 }
 
 func (p *Plugin) Execute(cmdArgs, environment []string) error {
-	b, err := p.installHelm()
-	if err != nil {
+	if err := p.installHelm(); err != nil {
 		return err
 	}
-	p.helm = b
-	logrus.Debugf("which helm: %s", p.helm)
 
 	if err := p.addRepo(); err != nil {
 		return err
@@ -96,11 +92,16 @@ func (p *Plugin) Execute(cmdArgs, environment []string) error {
 		return err
 	}
 
-	if _, err := karmadautil.EnsureNamespaceExist(p.KubeClient(), p.args.PxNamespace, p.options.DryRun); err != nil {
-		return fmt.Errorf("failed to ensure namespace %s, %w", p.args.PxNamespace, err)
-	}
-
 	for _, c := range clusters {
+		clusterClient, err := p.Client.NewClusterClientSet(c)
+		if err != nil {
+			return err
+		}
+
+		if _, err := karmadautil.EnsureNamespaceExist(clusterClient, p.args.PxNamespace, p.options.DryRun); err != nil {
+			return fmt.Errorf("failed to ensure namespace %s, %w", p.args.PxNamespace, err)
+		}
+
 		clusterHelmClient, err := p.Client.NewClusterHelmClient(c)
 		if err != nil {
 			return err
@@ -111,6 +112,14 @@ func (p *Plugin) Execute(cmdArgs, environment []string) error {
 			return err
 		}
 
+		clusterCRDClient, err := p.Client.NewClusterCRDClientset(c)
+		if err != nil {
+			return err
+		}
+		if err := util.WaitCRDReady(clusterCRDClient, vizierCRDName, p.options.WaitInterval, p.options.WaitTimeout); err != nil {
+			return fmt.Errorf("wait cluster %s CRD %s ready fail, %w", c, vizierCRDName, err)
+		}
+
 		_, err = p.applyTemplates(clusterHelmClient, c)
 		if err != nil {
 			return err
@@ -186,11 +195,6 @@ func (p *Plugin) applyTemplates(helmClient helmclient.Interface, cluster string)
 		return nil, errors.New(string(out))
 	}
 
-	vizierFilePath := path.Join(p.options.HomeDir, vizierFile)
-	if err = os.WriteFile(vizierFilePath, out, 0644); err != nil {
-		return nil, err
-	}
-
 	r, err := helmClient.Build(bytes.NewBuffer(out), false)
 	if err != nil {
 		return nil, err
@@ -222,28 +226,35 @@ func (p *Plugin) allClusters() ([]string, error) {
 	return clusterNames, nil
 }
 
-func (p *Plugin) installHelm() (string, error) {
+func (p *Plugin) installHelm() error {
 	helmComponent := p.options.Components["helm"]
 
 	// TODO: refactor all download code as https://github.com/kurator-dev/kurator/issues/61
 	installPath := filepath.Join(p.options.HomeDir, helmComponent.Name, helmComponent.Version)
 	helmPath := filepath.Join(installPath, fmt.Sprintf("%s-%s", util.OSExt(), runtime.GOARCH), helmBinary)
 	_, err := os.Stat(helmPath)
 	if err == nil {
-		return helmPath, nil
+		p.helm = helmPath
+		return nil
 	}
 
 	if os.IsNotExist(err) {
 		if err = os.MkdirAll(installPath, 0o750); err != nil {
-			return "", fmt.Errorf("unable to create directory %q: %w", installPath, err)
+			return fmt.Errorf("unable to create directory %q: %w", installPath, err)
 		}
 		// https://get.helm.sh/helm-v3.9.3-linux-amd64.tar.gz
 		url, _ := util.JoinUrlPath(helmComponent.ReleaseURLPrefix,
 			fmt.Sprintf("helm-%s-%s-%s.tar.gz", helmComponent.Version, util.OSExt(), runtime.GOARCH))
 		if _, err := util.DownloadResource(url, installPath); err != nil {
-			return "", fmt.Errorf("unable to get helm binary %q: %w", installPath, err)
+			return fmt.Errorf("unable to get helm binary %q: %w", installPath, err)
 		}
 	}
 
-	return util.VerifyExecutableBinary(helmPath)
+	b, err := util.VerifyExecutableBinary(helmPath)
+	if err != nil {
+		return err
+	}
+
+	p.helm = b
+	return err
 }