address PR feedback

Signed-off-by: Paweł Bojanowski <pawelbojanowski@protonmail.com>
loft-sh · Sep 9, 2024 · f2a564c · f2a564c
1 parent 4d435ab
commit f2a564c
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 59 deletions.
diff --git a/cmd/vclusterctl/cmd/platform/add/vcluster.go b/cmd/vclusterctl/cmd/platform/add/vcluster.go
@@ -31,7 +31,7 @@ Example:
 vcluster platform add vcluster my-vcluster --namespace vcluster-my-vcluster --project my-project --import-name my-vcluster
 
 Add all vClusters in the host cluster:
-vcluster platform add vcluster ignored --project my-project --all
+vcluster platform add vcluster --project my-project --all
 
 ###############################################
 
@@ -41,7 +41,7 @@ vcluster platform add vcluster ignored --project my-project --all
 		Use:   "vcluster",
 		Short: "Adds an existing vCluster to the vCluster platform",
 		Long:  description,
-		Args:  cobra.ExactArgs(1),
+		Args:  cobra.RangeArgs(0, 1),
 		RunE: func(cobraCmd *cobra.Command, args []string) error {
 			return cmd.Run(cobraCmd.Context(), args)
 		},
@@ -56,12 +56,12 @@ vcluster platform add vcluster ignored --project my-project --all
 	addCmd.Flags().BytesBase64Var(&cmd.CertificateAuthorityData, "ca-data", []byte{}, "additional, base64 encoded certificate authority data that will be passed to the platform secret")
 	// This is hidden until the platform side will be ready to use it
 	_ = addCmd.Flags().MarkHidden("ca-data")
-	addCmd.Flags().BoolVar(&cmd.All, "all", false, "all will try to add all vCluster found in all namespaces in the host cluster. If this flag is set, vcluster name argument is ignored")
+	addCmd.Flags().BoolVar(&cmd.All, "all", false, "all will try to add Virtual Cluster found in all namespaces in the host cluster. If this flag is set, any provided vCluster name argument is ignored")
 
 	return addCmd
 }
 
 // Run executes the functionality
 func (cmd *VClusterCmd) Run(ctx context.Context, args []string) error {
-	return cli.AddVClusterHelm(ctx, &cmd.AddVClusterOptions, cmd.GlobalFlags, args[0], cmd.Log)
+	return cli.AddVClusterHelm(ctx, &cmd.AddVClusterOptions, cmd.GlobalFlags, args, cmd.Log)
 }
diff --git a/pkg/cli/add_vcluster_helm.go b/pkg/cli/add_vcluster_helm.go
@@ -4,11 +4,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"strings"
 	"time"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/tools/clientcmd"
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 
 	"github.com/loft-sh/log"
 	"github.com/loft-sh/log/survey"
@@ -36,43 +34,28 @@ func AddVClusterHelm(
 	ctx context.Context,
 	options *AddVClusterOptions,
 	globalFlags *flags.GlobalFlags,
-	vClusterName string,
+	args []string,
 	log log.Logger,
 ) error {
 	var vClusters []find.VCluster
+	if len(args) == 0 && !options.All {
+		return errors.New("empty vCluster name but no --all flag set, please either set vCluster name to add one cluster or set --all flag to add all of them")
+	}
 	if options.All {
 		log.Debugf("add vcluster called with --all flag")
-		kubeClientConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
-			clientcmd.NewDefaultClientConfigLoadingRules(), &clientcmd.ConfigOverrides{
-				CurrentContext: globalFlags.Context,
-			})
-		hostClusterRestConfig, err := kubeClientConfig.ClientConfig()
-		if err != nil {
-			return err
-		}
-		hostKubeClient, err := kubernetes.NewForConfig(hostClusterRestConfig)
+		log.Info("looking for vClusters in all namespaces")
+		vClustersInNamespace, err := find.ListVClusters(ctx, globalFlags.Context, "", "", log)
 		if err != nil {
 			return err
 		}
-		namespaces, err := hostKubeClient.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
-		if err != nil {
-			return err
-		}
-		log.Debugf("looking for vclusters in %d namespaces", len(namespaces.Items))
-		for _, ns := range namespaces.Items {
-			log.Infof("looking for a vcluster in %s namespace", ns.GetName())
-			vClustersInNamespace, err := find.ListVClusters(ctx, globalFlags.Context, "", ns.GetName(), log)
-			if err != nil {
-				return err
-			}
-			if len(vClustersInNamespace) == 0 {
-				log.Infof("no vClusters found in context %s and namespace %s", globalFlags.Context, ns.GetName())
-				continue
-			}
+		if len(vClustersInNamespace) == 0 {
+			log.Infof("no vClusters found in context %s", globalFlags.Context)
+		} else {
 			vClusters = append(vClusters, vClustersInNamespace...)
 		}
 	} else {
 		// check if vCluster exists
+		vClusterName := args[0]
 		vCluster, err := find.GetVCluster(ctx, globalFlags.Context, vClusterName, globalFlags.Namespace, log)
 		if err != nil {
 			return err
@@ -94,39 +77,18 @@ func AddVClusterHelm(
 	if err != nil {
 		return err
 	}
-	addErr := &VClusterAddError{}
+	var addErrors []error
 	log.Debugf("trying to add %d vClusters to platform", len(vClusters))
 	for _, vCluster := range vClusters {
 		vCluster := vCluster
 		log.Infof("adding %s vCluster to platform", vCluster.Name)
-		addErr.addErr(vCluster.Name, addVClusterHelm(ctx, options, globalFlags, vCluster.Name, &vCluster, kubeClient, log))
-	}
-
-	return addErr.CombinedError()
-}
-
-type VClusterAddError struct {
-	errs []error
-}
-
-func (vce *VClusterAddError) CombinedError() error {
-	if len(vce.errs) == 0 {
-		return nil
-	} else if len(vce.errs) == 1 {
-		return vce.errs[0]
-	}
-	errMsg := strings.Builder{}
-	for _, err := range vce.errs {
-		_, _ = errMsg.WriteString(err.Error() + "|")
+		err := addVClusterHelm(ctx, options, globalFlags, vCluster.Name, &vCluster, kubeClient, log)
+		if err != nil {
+			addErrors = append(addErrors, fmt.Errorf("cannot add %s: %w", vCluster.Name, err))
+		}
 	}
-	return errors.New(errMsg.String())
-}
 
-func (vce *VClusterAddError) addErr(vClusterName string, err error) {
-	if err == nil {
-		return
-	}
-	vce.errs = append(vce.errs, fmt.Errorf("cannot add vcluster %s: %w", vClusterName, err))
+	return utilerrors.NewAggregate(addErrors)
 }
 
 func addVClusterHelm(