Skip to content

Commit

Permalink
Use only subm cluster to recover broker-info.subm
Browse files Browse the repository at this point in the history 
...instead of specifying both Broker and Submariner clusters.

Add required RBAC permissions for the client cluster to access the
Broker.

System tests are also accordingly adjusted.

Epic: submariner-io/enhancements#143

Signed-off-by: Janki Chhatbar <[email protected]>
  • Loading branch information
@Jaanki
Jaanki committed Mar 30, 2023
1 parent 7627740 commit 77da496
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 149 deletions.
153 changes: 47 additions & 106 deletions cmd/subctl/recover_broker_info.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,42 +19,35 @@ limitations under the License.
package subctl

import (
"errors"
"context"
"fmt"

"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))
},
}

Expand All @@ -63,114 +56,62 @@ 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
}

clusters, err := brokerCluster.GetClusters(brokerNamespace)
if err != nil {
return status.Error(err, "error listing joined clusters")
}

ok, err := tryToRecoverFromBroker(brokerCluster, brokerObj, brokerNamespace, clusters, status)
if ok || err != nil {
return err
}

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)
}
func recoverBrokerInfoFromSubm(submCluster *cluster.Info, namespace string, status reporter.Interface) error {
brokerNamespace := submCluster.Submariner.Spec.BrokerK8sRemoteNamespace
brokerRestConfig := submCluster.RestConfig

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
}
status.Start("Checking if the Broker is installed on the Submariner cluster %q in namespace %q", submCluster.Name, brokerNamespace)
defer status.End()

clusters, err := clusterInfo.GetClusters(namespace)
brokerObj, found, err := getBroker(brokerRestConfig, brokerNamespace)
if err != nil {
return status.Error(err, "error listing joined clusters")
return status.Error(err, "Error getting Broker")
}

ok, err := tryToRecoverFromBroker(clusterInfo, brokerObj, namespace, clusters, status)
if ok || err != nil {
return err
}
if !found {
status.Warning("Broker not found. Trying to connect to the Broker installed on a different cluster")

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), "")
}
brokerRestConfig, brokerNamespace, err = restconfig.ForBroker(submCluster.Submariner, submCluster.ServiceDiscovery)
if err != nil {
return status.Error(err, "Error getting the Broker's REST config")
}

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, found, err = getBroker(brokerRestConfig, brokerNamespace)
if err != nil {
return status.Error(err, "")
}

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), "")
}
if !found {
return status.Error(fmt.Errorf("no Broker resource found in namespace %q", brokerNamespace), "")
}

if err != nil {
return nil, status.Error(err, "")
status.Success("Found Broker installed on a different cluster in namespace %s", brokerNamespace)
}

status.End()

return brokerObj, nil
//nolint:wrapcheck // No need to wrap errors here.
return broker.RecoverData(submCluster, brokerObj, brokerNamespace, brokerRestConfig, status)
}

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"), "")
func getBroker(config *rest.Config, namespace string) (*v1alpha1.Broker, bool, error) {
brokerClientProducer, err := client.NewProducerFromRestConfig(config)
if err != nil {
return nil, false, errors.Wrap(err, "Error creating broker client Producer")
}

status.Success("Found %d cluster(s) joined to the Broker", len(clusters))
status.End()
brokerObj := &v1alpha1.Broker{}
err = brokerClientProducer.ForGeneral().Get(
context.TODO(), controllerClient.ObjectKey{
Namespace: namespace,
Name: brokercr.Name,
}, brokerObj)

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)
if apierrors.IsNotFound(err) {
return nil, false, nil
}

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
}
}
if err != nil {
return nil, false, errors.Wrap(err, "error retrieving Broker")
}

return false
return brokerObj, true, nil
}
15 changes: 15 additions & 0 deletions pkg/broker/rbac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{""},
Expand Down Expand Up @@ -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"},
Expand All @@ -105,6 +115,11 @@ func NewBrokerClusterRole() *rbacv1.Role {
APIGroups: []string{""},
Resources: []string{"secrets"},
},
{
Verbs: []string{"get", "list"},
APIGroups: []string{""},
Resources: []string{"serviceaccounts"},
},
},
}
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/broker/recover_broker_info.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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")
Expand Down
12 changes: 0 additions & 12 deletions pkg/cluster/info.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{}

Expand Down
32 changes: 4 additions & 28 deletions scripts/test/system.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
_subctl recover-broker-info --context cluster1

0 comments on commit 77da496

Please sign in to comment.