Use only subm cluster to recover broker-info.subm

instead of specifying both Broker and Submariner clusters. System tests are also accordingly adjusted. Depends on: submariner-io/submariner-operator#2547 Epic: submariner-io/enhancements#143 Signed-off-by: Janki Chhatbar <[email protected]>
Jaanki · Mar 29, 2023 · 7f44bbe · 7f44bbe
1 parent f358caa
commit 7f44bbe
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 156 deletions.
diff --git a/cmd/subctl/recover_broker_info.go b/cmd/subctl/recover_broker_info.go
@@ -19,42 +19,34 @@ limitations under the License.
 package subctl
 
 import (
-	"errors"
-	"fmt"
+	"context"
 
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/submariner-io/admiral/pkg/reporter"
 	"github.com/submariner-io/subctl/internal/cli"
-	"github.com/submariner-io/subctl/internal/constants"
 	"github.com/submariner-io/subctl/internal/exit"
 	"github.com/submariner-io/subctl/internal/restconfig"
 	"github.com/submariner-io/subctl/pkg/broker"
+	"github.com/submariner-io/subctl/pkg/brokercr"
+	"github.com/submariner-io/subctl/pkg/client"
 	"github.com/submariner-io/subctl/pkg/cluster"
 	"github.com/submariner-io/submariner-operator/api/v1alpha1"
-	submarinerv1 "github.com/submariner-io/submariner/pkg/apis/submariner.io/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/client-go/rest"
+	controllerClient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-var recoverRestConfigProducer = restconfig.NewProducer().WithPrefixedContext("broker").
-	WithDefaultNamespace(constants.DefaultBrokerNamespace).
-	WithPrefixedNamespace("broker", constants.DefaultBrokerNamespace)
+var recoverRestConfigProducer = restconfig.NewProducer()
 
 // recoverBrokerInfo represents the reconstruct command.
 var recoverBrokerInfo = &cobra.Command{
 	Use:   "recover-broker-info",
 	Short: "Recovers the broker-info.subm file from the installed Broker",
 	Run: func(cmd *cobra.Command, args []string) {
 		status := cli.NewReporter()
-		// if --brokerconfig flag provided, get the broker and proceed to get Submariner
-		contextFound, err := recoverRestConfigProducer.RunOnSelectedPrefixedContext("broker",
-			recoverBrokerFromConfigContext, status)
-		exit.OnError(err)
-
-		// if --brokerconfig not provided, search for broker on current context and proceed to get Submariner
-		if !contextFound {
-			err = recoverRestConfigProducer.RunOnSelectedContext(recoverBrokerFromCurrentContext, status)
-		}
-		exit.OnError(err)
+
+		exit.OnError(recoverRestConfigProducer.RunOnSelectedContext(restconfig.IfConnectivityInstalled(recoverBrokerInfoFromSubm), status))
 	},
 }
 
@@ -63,114 +55,55 @@ func init() {
 	rootCmd.AddCommand(recoverBrokerInfo)
 }
 
-func recoverBrokerFromConfigContext(brokerCluster *cluster.Info, brokerNamespace string, status reporter.Interface) error {
-	brokerObj, err := getBroker(brokerCluster, brokerNamespace, "Please try again with another cluster", status)
-	if err != nil {
-		return err
-	}
+func recoverBrokerInfoFromSubm(submCluster *cluster.Info, brokerNamespace string, status reporter.Interface) error {
+	brokerNamespace = submCluster.Submariner.Spec.BrokerK8sRemoteNamespace
+	brokerRestConfig := submCluster.RestConfig
 
-	clusters, err := brokerCluster.GetClusters(brokerNamespace)
+	status.Start("Checking if the Broker is installed on the Submariner cluster %q in namespace %q", submCluster.Name, brokerNamespace)
+
+	brokerObj, err := getBroker(brokerRestConfig, brokerNamespace)
 	if err != nil {
-		return status.Error(err, "error listing joined clusters")
+		status.Warning("Error getting Broker: %s", err)
 	}
 
-	ok, err := tryToRecoverFromBroker(brokerCluster, brokerObj, brokerNamespace, clusters, status)
-	if ok || err != nil {
-		return err
+	if brokerObj == nil {
+		status.Start("Trying to connect to the Broker installed on another cluster")
+
+		brokerRestConfig, brokerNamespace, err = restconfig.ForBroker(submCluster.Submariner, submCluster.ServiceDiscovery)
+		if err != nil || brokerRestConfig == nil {
+			return status.Error(err, "Error getting the broker's rest config")
+		}
+
+		brokerObj, err = getBroker(brokerRestConfig, brokerNamespace)
+		if err != nil {
+			return status.Error(err, "Error getting Broker")
+		}
 	}
 
-	status.Warning("Submariner is not installed on the same cluster as Broker")
-	status.Start("Checking if Submariner is installed on a different cluster")
 	//nolint:wrapcheck // No need to wrap errors here.
-	return recoverRestConfigProducer.RunOnSelectedContext(
-		func(submCluster *cluster.Info, namespace string, status reporter.Interface) error {
-			if isSubmJoinedToBroker(clusters, submCluster) {
-				status.Success("Found a Submariner installation on cluster %q joined to the Broker", submCluster.Name)
-				//nolint:wrapcheck // No need to wrap errors here.
-				return broker.RecoverData(brokerCluster, submCluster, brokerObj, namespace, status)
-			}
-
-			return status.Error(
-				fmt.Errorf("submariner is not installed on cluster %s. "+
-					"Please specify the cluster where Submariner is installed via `--kubeconfig` or `--context` flag"+
-					"", submCluster.Name), "")
-		}, status)
+	return broker.RecoverData(submCluster, brokerObj, brokerNamespace, brokerRestConfig, status)
 }
 
-func recoverBrokerFromCurrentContext(clusterInfo *cluster.Info, namespace string, status reporter.Interface) error {
-	// if --brokerconfig not provided, search for broker on current context and proceed to get Submariner
-	brokerObj, err := getBroker(clusterInfo, namespace, "Please specify the cluster where the Broker is installed "+
-		"via the `--brokerconfig` or `--brokercontext` flag and the namespace via the `--brokernamespace` flag", status)
-	if err != nil {
-		return err
-	}
-
-	clusters, err := clusterInfo.GetClusters(namespace)
+func getBroker(config *rest.Config, namespace string) (*v1alpha1.Broker, error) {
+	brokerClientProducer, err := client.NewProducerFromRestConfig(config)
 	if err != nil {
-		return status.Error(err, "error listing joined clusters")
+		return nil, errors.Wrap(err, "Error creating broker client Producer")
 	}
 
-	ok, err := tryToRecoverFromBroker(clusterInfo, brokerObj, namespace, clusters, status)
-	if ok || err != nil {
-		return err
-	}
-
-	return status.Error(
-		fmt.Errorf("submariner is not installed on cluster %q. "+
-			"Please specify the cluster where Submariner is installed via `--kubeconfig` or `--context` flag"+
-			"", clusterInfo.Name), "")
-}
-
-func getBroker(clusterInfo *cluster.Info, namespace, notFoundMsg string, status reporter.Interface) (*v1alpha1.Broker, error) {
-	status.Start("Checking if the Broker is installed on cluster %q in namespace %q", clusterInfo.Name, namespace)
+	brokerObj := &v1alpha1.Broker{}
+	err = brokerClientProducer.ForGeneral().Get(
+		context.TODO(), controllerClient.ObjectKey{
+			Namespace: namespace,
+			Name:      brokercr.Name,
+		}, brokerObj)
 
-	brokerObj, err := clusterInfo.GetBroker(namespace)
 	if apierrors.IsNotFound(err) {
-		return nil, status.Error(fmt.Errorf("the Broker is not installed on the specified cluster in namespace %s. %s", notFoundMsg,
-			namespace), "")
+		return nil, errors.Wrapf(err, "the Broker is not installed on the specified cluster in namespace %s", namespace)
 	}
 
 	if err != nil {
-		return nil, status.Error(err, "")
+		return nil, errors.Wrap(err, "error retrieving Broker")
 	}
 
-	status.End()
-
 	return brokerObj, nil
 }
-
-func tryToRecoverFromBroker(brokerCluster *cluster.Info, brokerObj *v1alpha1.Broker,
-	brokerNamespace string, clusters []submarinerv1.Cluster, status reporter.Interface,
-) (bool, error) {
-	status.Start("Checking if there are any clusters joined to the Broker")
-
-	if len(clusters) == 0 {
-		return false, status.Error(
-			errors.New(
-				"no clusters are joined to the Broker. Please re-run the `deploy-broker` command to regenerate the broker-info."+
-					"subm file"), "")
-	}
-
-	status.Success("Found %d cluster(s) joined to the Broker", len(clusters))
-	status.End()
-
-	if isSubmJoinedToBroker(clusters, brokerCluster) {
-		status.Success("Found a local Submariner installation joined to the Broker")
-		//nolint:wrapcheck // No need to wrap errors here.
-		return true, broker.RecoverData(brokerCluster, brokerCluster, brokerObj, brokerNamespace, status)
-	}
-
-	return false, nil
-}
-
-func isSubmJoinedToBroker(clusters []submarinerv1.Cluster, clusterInfo *cluster.Info) bool {
-	if clusterInfo.Submariner != nil {
-		for i := range clusters {
-			if clusters[i].Spec.ClusterID == clusterInfo.Submariner.Spec.ClusterID {
-				return true
-			}
-		}
-	}
-
-	return false
-}
diff --git a/go.mod b/go.mod
@@ -19,7 +19,7 @@ require (
 	github.com/submariner-io/lighthouse v0.15.0-m4.0.20230327162337-e5988504cdca
 	github.com/submariner-io/shipyard v0.15.0-m4
 	github.com/submariner-io/submariner v0.15.0-m4
-	github.com/submariner-io/submariner-operator v0.15.0-m4
+	github.com/submariner-io/submariner-operator v0.15.0-m4.0.20230328092440-b2d0c314e0fe
 	github.com/uw-labs/lichen v0.1.7
 	golang.org/x/net v0.8.0
 	golang.org/x/oauth2 v0.6.0
@@ -28,7 +28,7 @@ require (
 	k8s.io/apiextensions-apiserver v0.26.3
 	k8s.io/apimachinery v0.26.3
 	k8s.io/client-go v0.26.3
-	k8s.io/utils v0.0.0-20230202215443-34013725500c
+	k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5
 	sigs.k8s.io/controller-runtime v0.14.5
 	sigs.k8s.io/mcs-api v0.1.0
 	sigs.k8s.io/yaml v1.3.0

diff --git a/go.sum b/go.sum
@@ -549,8 +549,8 @@ github.com/submariner-io/shipyard v0.15.0-m4 h1:ji+WLE9yvOqUQJnKYhLm0Ajb5WTobkux
 github.com/submariner-io/shipyard v0.15.0-m4/go.mod h1:txktXeFOmI8g5wOxmuvNgheyZLyRloXbdo4i5gQwGNY=
 github.com/submariner-io/submariner v0.15.0-m4 h1:dMCFH09rPK3Vlkl0CutIyT2zf2EiLEWnXUKzHizmHJo=
 github.com/submariner-io/submariner v0.15.0-m4/go.mod h1:jTzblmp5fMVIlfWuuLdm+mzJXRDAsIDnp90bXzm8JyU=
-github.com/submariner-io/submariner-operator v0.15.0-m4 h1:kzakmq+SAm5qBKu8vNz0O9PhXCEkNauX8W15tmKZEew=
-github.com/submariner-io/submariner-operator v0.15.0-m4/go.mod h1:YDtbTY0xJFmrYiwdkdA8X3xxuOQ1co7a/hPiSVChnTA=
+github.com/submariner-io/submariner-operator v0.15.0-m4.0.20230328092440-b2d0c314e0fe h1:4bvGcqP/HYM2lzwWAV4Fv54zAKfp6hRE8BND608y0aw=
+github.com/submariner-io/submariner-operator v0.15.0-m4.0.20230328092440-b2d0c314e0fe/go.mod h1:qCqBKpiZQeSCmIrvu/ZX6OTtWjVBEfGM4C2gtbKb7sQ=
 github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
@@ -876,8 +876,8 @@ k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89/go.mod h1:sZAwmy6armz5eXlNoLmJcl
 k8s.io/utils v0.0.0-20200603063816-c1c6865ac451/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
 k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
 k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
-k8s.io/utils v0.0.0-20230202215443-34013725500c h1:YVqDar2X7YiQa/DVAXFMDIfGF8uGrHQemlrwRU5NlVI=
-k8s.io/utils v0.0.0-20230202215443-34013725500c/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
+k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5 h1:kmDqav+P+/5e1i9tFfHq1qcF3sOrDp+YEkVDAHu7Jwk=
+k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.7/go.mod h1:PHgbrJT7lCHcxMU+mDHEm+nx46H4zuuHZkDP6icnhu0=
 sigs.k8s.io/controller-runtime v0.6.1/go.mod h1:XRYBPdbf5XJu9kpS84VJiZ7h/u1hF3gEORz0efEja7A=
 sigs.k8s.io/controller-runtime v0.14.5 h1:6xaWFqzT5KuAQ9ufgUaj1G/+C4Y1GRkhrxl+BJ9i+5s=

diff --git a/pkg/broker/rbac.go b/pkg/broker/rbac.go
@@ -54,6 +54,11 @@ func NewBrokerAdminRole() *rbacv1.Role {
 				APIGroups: []string{"submariner.io"},
 				Resources: []string{"clusters", "endpoints"},
 			},
+			{
+				Verbs:     []string{"get", "list"},
+				APIGroups: []string{"submariner.io"},
+				Resources: []string{"brokers"},
+			},
 			{
 				Verbs:     []string{"create", "get", "list", "update", "delete", "watch"},
 				APIGroups: []string{""},
@@ -90,6 +95,11 @@ func NewBrokerClusterRole() *rbacv1.Role {
 				APIGroups: []string{"submariner.io"},
 				Resources: []string{"clusters", "endpoints"},
 			},
+			{
+				Verbs:     []string{"get", "list"},
+				APIGroups: []string{"submariner.io"},
+				Resources: []string{"brokers"},
+			},
 			{
 				Verbs:     []string{"create", "get", "list", "watch", "patch", "update", "delete"},
 				APIGroups: []string{"multicluster.x-k8s.io"},
@@ -103,7 +113,12 @@ func NewBrokerClusterRole() *rbacv1.Role {
 			{
 				Verbs:     []string{"get", "list", "watch"},
 				APIGroups: []string{""},
-				Resources: []string{"secrets"},
+				Resources: []string{"secrets", "serviceaccounts"},
+			},
+			{
+				Verbs:     []string{"get", "list"},
+				APIGroups: []string{""},
+				Resources: []string{"serviceaccounts"},
 			},
 		},
 	}

diff --git a/pkg/broker/recover_broker_info.go b/pkg/broker/recover_broker_info.go
@@ -25,10 +25,11 @@ import (
 	"github.com/submariner-io/subctl/pkg/cluster"
 	"github.com/submariner-io/submariner-operator/api/v1alpha1"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/client-go/rest"
 )
 
-func RecoverData(
-	brokerCluster, submCluster *cluster.Info, broker *v1alpha1.Broker, namespace string, status reporter.Interface,
+func RecoverData(submCluster *cluster.Info, broker *v1alpha1.Broker, brokerNamespace string,
+	brokerRestConfig *rest.Config, status reporter.Interface,
 ) error {
 	status.Start("Retrieving data to reconstruct broker-info.subm")
 	defer status.End()
@@ -42,7 +43,7 @@ func RecoverData(
 
 	status.Success("Successfully retrieved the data. Writing it to broker-info.subm")
 
-	err = WriteInfoToFile(brokerCluster.RestConfig, namespace, decodedPSKSecret,
+	err = WriteInfoToFile(brokerRestConfig, brokerNamespace, decodedPSKSecret,
 		sets.New(broker.Spec.Components...), broker.Spec.DefaultCustomDomains, status)
 
 	return status.Error(err, "error reconstructing broker-info.subm")

diff --git a/pkg/cluster/info.go b/pkg/cluster/info.go
@@ -25,7 +25,6 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/submariner-io/subctl/internal/constants"
-	"github.com/submariner-io/subctl/pkg/brokercr"
 	"github.com/submariner-io/subctl/pkg/client"
 	"github.com/submariner-io/subctl/pkg/image"
 	"github.com/submariner-io/submariner-operator/api/v1alpha1"
@@ -190,17 +189,6 @@ func (c *Info) OperatorNamespace() string {
 	return constants.OperatorNamespace
 }
 
-func (c *Info) GetBroker(namespace string) (*v1alpha1.Broker, error) {
-	broker := &v1alpha1.Broker{}
-	err := c.ClientProducer.ForGeneral().Get(
-		context.TODO(), controllerClient.ObjectKey{
-			Namespace: namespace,
-			Name:      brokercr.Name,
-		}, broker)
-
-	return broker, errors.Wrap(err, "error retrieving Broker")
-}
-
 func (c *Info) GetClusters(namespace string) ([]submarinerv1.Cluster, error) {
 	clusters := &submarinerv1.ClusterList{}
 

diff --git a/scripts/test/system.sh b/scripts/test/system.sh
@@ -180,42 +180,18 @@ _subctl cloud prepare generic --kubecontext cluster1 && exit 1
 echo "Renaming broker-info.subm to broker-info.subm.orig"
 mv "${DAPPER_SOURCE}"/output/broker-info.subm "${DAPPER_SOURCE}"/output/broker-info.subm.orig
 
-# Test subctl recover-broker-info invocations with both Broker and Submariner installed on the cluster
-_subctl recover-broker-info --brokercontext cluster1 --context cluster1
-cmp "${DAPPER_SOURCE}"/output/broker-info.subm.orig broker-info.subm
-rm -f broker-info.subm
-
+# Test subctl recover-broker-info invocations
 _subctl recover-broker-info --context cluster1
 cmp "${DAPPER_SOURCE}"/output/broker-info.subm.orig broker-info.subm
 rm -f broker-info.subm
 
-# Failure tests
-# Invalid Broker namespace
-_subctl recover-broker-info --namespace non-existent --context cluster1 && exit 1
-_subctl recover-broker-info --brokernamespace non-existent --brokercontext cluster1 && exit 1
-
-# No Broker installed
-_subctl recover-broker-info --brokercontext cluster2 && exit 1
-_subctl recover-broker-info --context cluster2 && exit 1
-
-# Test subctl uninstall invocations
-_subctl uninstall -y --kubeconfig "${KUBECONFIGS_DIR}"/kind-config-cluster1
-
-# Test subctl recover-broker-info invocations with Submariner not installed on Broker cluster
-_subctl recover-broker-info --brokercontext cluster1 --context cluster2
+_subctl recover-broker-info --context cluster2
 cmp "${DAPPER_SOURCE}"/output/broker-info.subm.orig broker-info.subm
 rm -f broker-info.subm
 
-_subctl recover-broker-info --brokercontext cluster1 --context cluster1 && exit 1
-_subctl recover-broker-info --context cluster1 && exit 1
-
 # Test subctl uninstall invocations
 _subctl uninstall -y --context cluster2
+_subctl uninstall -y --kubeconfig "${KUBECONFIGS_DIR}"/kind-config-cluster1
 
 # Test subctl recover-broker-info invocations with Submariner not installed on any cluster
-_subctl recover-broker-info --brokercontext cluster1 --context cluster1 && exit 1
-_subctl recover-broker-info --brokercontext cluster1 --context cluster2 && exit 1
 _subctl recover-broker-info --context cluster1 && exit 1
-
-# Uninstall Broker
-_subctl uninstall -y --kubeconfig "${KUBECONFIGS_DIR}"/kind-config-cluster1