From 85eae834952cb34b15aa3c7dcfd5b816a2ffc1b4 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Mon, 9 Dec 2024 17:13:33 +0530 Subject: [PATCH 1/9] KUBESAW-172: Restart host-operator at the end of the register-member command Signed-off-by: Feny Mehta --- pkg/cmd/adm/register_member.go | 34 ++++++++++++++++++ pkg/cmd/adm/restart.go | 51 ++++++++++++++++----------- pkg/cmd/adm/restart_test.go | 2 +- pkg/cmd/adm/unregister_member.go | 4 +-- pkg/cmd/adm/unregister_member_test.go | 18 +++++----- 5 files changed, 77 insertions(+), 32 deletions(-) diff --git a/pkg/cmd/adm/register_member.go b/pkg/cmd/adm/register_member.go index e759eb0..b4c04b9 100644 --- a/pkg/cmd/adm/register_member.go +++ b/pkg/cmd/adm/register_member.go @@ -23,6 +23,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -351,6 +352,7 @@ type registerMemberValidated struct { memberClusterData clusterData warnings []string errors []string + //restart func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error } func getApiEndpointAndClient(ctx *extendedCommandContext, kubeConfigPath string) (apiEndpoint string, cl runtimeclient.Client, err error) { @@ -483,6 +485,11 @@ func (v *registerMemberValidated) perform(ctx *extendedCommandContext) error { return err } + // // restart Host Operator using the restart command + // if err := v.restart(ctx.CommandContext, "host", getRegMemConfigFlagsAndClient); err != nil { + // return err + // } + exampleSPC := &toolchainv1alpha1.SpaceProvisionerConfig{ TypeMeta: metav1.TypeMeta{ Kind: "SpaceProvisionerConfig", @@ -509,6 +516,33 @@ until the SpaceProvisionerConfig.spec.enabled is set to true. `, v.hostClusterData.apiEndpoint)) } +func getRegMemConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) (confg configuration.ClusterConfig, kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) { + kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() + + kubeConfigFlags.ClusterName = nil // `cluster` flag is redefined for our own purpose + kubeConfigFlags.AuthInfoName = nil // unused here, so we can hide it + kubeConfigFlags.Context = nil // unused here, so we can hide it + + cfg, err := configuration.LoadClusterConfig(ctx, clusterName) + if err != nil { + return cfg, nil, nil, err + } + kubeConfigFlags.Namespace = &cfg.OperatorNamespace + kubeConfigFlags.APIServer = &cfg.ServerAPI + kubeConfigFlags.BearerToken = &cfg.Token + kubeconfig, err := client.EnsureKsctlConfigFile() + if err != nil { + return cfg, nil, nil, err + } + kubeConfigFlags.KubeConfig = &kubeconfig + + cl, err := ctx.NewClient(cfg.Token, cfg.ServerAPI) + if err != nil { + return cfg, nil, nil, err + } + return cfg, kubeConfigFlags, cl, nil +} + func findToolchainClusterForMember(allToolchainClusters []toolchainv1alpha1.ToolchainCluster, memberAPIEndpoint, memberOperatorNamespace string) *toolchainv1alpha1.ToolchainCluster { for _, tc := range allToolchainClusters { if tc.Status.APIEndpoint == memberAPIEndpoint && tc.Status.OperatorNamespace == memberOperatorNamespace { diff --git a/pkg/cmd/adm/restart.go b/pkg/cmd/adm/restart.go index 2546061..6df1588 100644 --- a/pkg/cmd/adm/restart.go +++ b/pkg/cmd/adm/restart.go @@ -21,8 +21,9 @@ import ( ) type ( - RolloutRestartFunc func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error - RolloutStatusCheckerFunc func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error + RolloutRestartFunc func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error + RolloutStatusCheckerFunc func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error + ConfigFlagsAndClientGetterFunc func(ctx *clicontext.CommandContext, clusterName string) (cfg configuration.ClusterConfig, kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) ) // NewRestartCmd() is a function to restart the whole operator, it relies on the target cluster and fetches the cluster config @@ -45,52 +46,62 @@ func NewRestartCmd() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { term := ioutils.NewTerminal(cmd.InOrStdin, cmd.OutOrStdout) ctx := clicontext.NewCommandContext(term, client.DefaultNewClient) - return restart(ctx, args[0]) + return restart(ctx, args[0], getConfigFlagsAndClient) }, } return command } -func restart(ctx *clicontext.CommandContext, clusterName string) error { - kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() +func restart(ctx *clicontext.CommandContext, clusterName string, configFlagsClientGetter ConfigFlagsAndClientGetterFunc) error { ioStreams := genericiooptions.IOStreams{ In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr, } + + cfg, kubeConfigFlags, cl, err := configFlagsClientGetter(ctx, clusterName) + if err != nil { + return err + } + factory := cmdutil.NewFactory(cmdutil.NewMatchVersionFlags(kubeConfigFlags)) + + if !ctx.AskForConfirmation( + ioutils.WithMessagef("restart all the deployments in the cluster '%s' and namespace '%s' \n", clusterName, cfg.OperatorNamespace)) { + return nil + } + + return restartDeployments(ctx, cl, cfg.OperatorNamespace, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + return checkRolloutStatus(ctx, factory, ioStreams, deployment) + }, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + return restartNonOlmDeployments(ctx, deployment, factory, ioStreams) + }) +} + +func getConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) (confg configuration.ClusterConfig, kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) { + kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() + kubeConfigFlags.ClusterName = nil // `cluster` flag is redefined for our own purpose kubeConfigFlags.AuthInfoName = nil // unused here, so we can hide it kubeConfigFlags.Context = nil // unused here, so we can hide it cfg, err := configuration.LoadClusterConfig(ctx, clusterName) if err != nil { - return err + return cfg, nil, nil, err } kubeConfigFlags.Namespace = &cfg.OperatorNamespace kubeConfigFlags.APIServer = &cfg.ServerAPI kubeConfigFlags.BearerToken = &cfg.Token kubeconfig, err := client.EnsureKsctlConfigFile() if err != nil { - return err + return cfg, nil, nil, err } kubeConfigFlags.KubeConfig = &kubeconfig - factory := cmdutil.NewFactory(cmdutil.NewMatchVersionFlags(kubeConfigFlags)) - - if !ctx.AskForConfirmation( - ioutils.WithMessagef("restart all the deployments in the cluster '%s' and namespace '%s' \n", clusterName, cfg.OperatorNamespace)) { - return nil - } cl, err := ctx.NewClient(cfg.Token, cfg.ServerAPI) if err != nil { - return err + return cfg, nil, nil, err } - - return restartDeployments(ctx, cl, cfg.OperatorNamespace, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { - return checkRolloutStatus(ctx, factory, ioStreams, deployment) - }, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { - return restartNonOlmDeployments(ctx, deployment, factory, ioStreams) - }) + return cfg, kubeConfigFlags, cl, nil } // This function has the whole logic of getting the list of olm and non-olm based deployment, then proceed on restarting/deleting accordingly diff --git a/pkg/cmd/adm/restart_test.go b/pkg/cmd/adm/restart_test.go index 6292f44..6ba355a 100644 --- a/pkg/cmd/adm/restart_test.go +++ b/pkg/cmd/adm/restart_test.go @@ -331,7 +331,7 @@ func TestRestart(t *testing.T) { newClient, _ := NewFakeClients(t) ctx := clicontext.NewCommandContext(term, newClient) //when - err := restart(ctx, "host") + err := restart(ctx, "host", getConfigFlagsAndClient) //then require.NoError(t, err) diff --git a/pkg/cmd/adm/unregister_member.go b/pkg/cmd/adm/unregister_member.go index c082321..46ff9b9 100644 --- a/pkg/cmd/adm/unregister_member.go +++ b/pkg/cmd/adm/unregister_member.go @@ -14,7 +14,7 @@ import ( "k8s.io/apimachinery/pkg/types" ) -type restartFunc func(ctx *clicontext.CommandContext, clusterName string) error +type restartFunc func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error func NewUnregisterMemberCmd() *cobra.Command { return &cobra.Command{ @@ -64,5 +64,5 @@ func UnregisterMemberCluster(ctx *clicontext.CommandContext, clusterName string, } ctx.Printlnf("\nThe deletion of the Toolchain member cluster from the Host cluster has been triggered") - return restart(ctx, "host") + return restart(ctx, "host", getConfigFlagsAndClient) } diff --git a/pkg/cmd/adm/unregister_member_test.go b/pkg/cmd/adm/unregister_member_test.go index fb7575b..a456dd0 100644 --- a/pkg/cmd/adm/unregister_member_test.go +++ b/pkg/cmd/adm/unregister_member_test.go @@ -21,7 +21,7 @@ func TestUnregisterMemberWhenAnswerIsY(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string) error { + err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -46,7 +46,7 @@ func TestUnregisterMemberWhenRestartError(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string) error { + err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { return fmt.Errorf("restart did not happen") }) @@ -65,9 +65,9 @@ func TestUnregisterMemberCallsRestart(t *testing.T) { ctxAct := clicontext.NewCommandContext(term, newClient) called := 0 // when - err := UnregisterMemberCluster(ctxAct, "member1", func(ctx *clicontext.CommandContext, restartClusterName string) error { + err := UnregisterMemberCluster(ctxAct, "member1", func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { called++ - return mockRestart(ctx, restartClusterName) + return mockRestart(ctx, restartClusterName, getConfigFlagsAndClient) }) // then @@ -84,7 +84,7 @@ func TestUnregisterMemberWhenAnswerIsN(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string) error { + err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -107,7 +107,7 @@ func TestUnregisterMemberWhenNotFound(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string) error { + err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -130,7 +130,7 @@ func TestUnregisterMemberWhenUnknownClusterName(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "some", func(ctx *clicontext.CommandContext, clusterName string) error { + err := UnregisterMemberCluster(ctx, "some", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -155,7 +155,7 @@ func TestUnregisterMemberLacksPermissions(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string) error { + err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -164,7 +164,7 @@ func TestUnregisterMemberLacksPermissions(t *testing.T) { AssertToolchainClusterSpec(t, fakeClient, toolchainCluster) } -func mockRestart(ctx *clicontext.CommandContext, clusterName string) error { +func mockRestart(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { if clusterName == "host" && ctx != nil { return nil } From 7bff8035ca0fc1286e9460313e729d4cf8c10038 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Sat, 14 Dec 2024 16:10:33 +0530 Subject: [PATCH 2/9] Register member test cases Signed-off-by: Feny Mehta --- pkg/cmd/adm/register_member.go | 38 +++++++--------- pkg/cmd/adm/register_member_test.go | 65 ++++++++++++++++++++------- pkg/cmd/adm/unregister_member_test.go | 4 +- 3 files changed, 67 insertions(+), 40 deletions(-) diff --git a/pkg/cmd/adm/register_member.go b/pkg/cmd/adm/register_member.go index b4c04b9..f3d6d8a 100644 --- a/pkg/cmd/adm/register_member.go +++ b/pkg/cmd/adm/register_member.go @@ -45,6 +45,7 @@ type newClientFromRestConfigFunc func(*rest.Config) (runtimeclient.Client, error type extendedCommandContext struct { *clicontext.CommandContext NewClientFromRestConfig newClientFromRestConfigFunc + RestartFunc func(ctx *clicontext.CommandContext, clusterName string) (cfg configuration.ClusterConfig, kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) } func newExtendedCommandContext(term ioutils.Terminal, clientCtor newClientFromRestConfigFunc) *extendedCommandContext { @@ -73,7 +74,7 @@ func NewRegisterMemberCmd() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { term := ioutils.NewTerminal(cmd.InOrStdin, cmd.OutOrStdout) ctx := newExtendedCommandContext(term, client.DefaultNewClientFromRestConfig) - return registerMemberCluster(ctx, commandArgs) + return registerMemberCluster(ctx, commandArgs, restart) }, } @@ -95,8 +96,8 @@ func NewRegisterMemberCmd() *cobra.Command { return cmd } -func registerMemberCluster(ctx *extendedCommandContext, args registerMemberArgs) error { - validated, err := validateArgs(ctx, args) +func registerMemberCluster(ctx *extendedCommandContext, args registerMemberArgs, restart restartFunc) error { + validated, err := validateArgs(ctx, args, restart) if err != nil { return err } @@ -352,7 +353,7 @@ type registerMemberValidated struct { memberClusterData clusterData warnings []string errors []string - //restart func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error + restart func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error } func getApiEndpointAndClient(ctx *extendedCommandContext, kubeConfigPath string) (apiEndpoint string, cl runtimeclient.Client, err error) { @@ -376,7 +377,7 @@ func getApiEndpointAndClient(ctx *extendedCommandContext, kubeConfigPath string) return } -func validateArgs(ctx *extendedCommandContext, args registerMemberArgs) (*registerMemberValidated, error) { +func validateArgs(ctx *extendedCommandContext, args registerMemberArgs, restart restartFunc) (*registerMemberValidated, error) { hostApiEndpoint, hostClusterClient, err := getApiEndpointAndClient(ctx, args.hostKubeConfig) if err != nil { return nil, err @@ -446,6 +447,7 @@ func validateArgs(ctx *extendedCommandContext, args registerMemberArgs) (*regist }, warnings: warnings, errors: errors, + restart: restart, }, nil } @@ -485,10 +487,10 @@ func (v *registerMemberValidated) perform(ctx *extendedCommandContext) error { return err } - // // restart Host Operator using the restart command - // if err := v.restart(ctx.CommandContext, "host", getRegMemConfigFlagsAndClient); err != nil { - // return err - // } + // restart Host Operator using the restart command + if err := v.restart(ctx.CommandContext, "host", v.getRegMemConfigFlagsAndClient); err != nil { + return err + } exampleSPC := &toolchainv1alpha1.SpaceProvisionerConfig{ TypeMeta: metav1.TypeMeta{ @@ -516,7 +518,7 @@ until the SpaceProvisionerConfig.spec.enabled is set to true. `, v.hostClusterData.apiEndpoint)) } -func getRegMemConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) (confg configuration.ClusterConfig, kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) { +func (v *registerMemberValidated) getRegMemConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) (confg configuration.ClusterConfig, kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) { kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() kubeConfigFlags.ClusterName = nil // `cluster` flag is redefined for our own purpose @@ -527,20 +529,12 @@ func getRegMemConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName s if err != nil { return cfg, nil, nil, err } - kubeConfigFlags.Namespace = &cfg.OperatorNamespace - kubeConfigFlags.APIServer = &cfg.ServerAPI + kubeConfigFlags.Namespace = &v.hostClusterData.namespace + kubeConfigFlags.APIServer = &v.hostClusterData.apiEndpoint kubeConfigFlags.BearerToken = &cfg.Token - kubeconfig, err := client.EnsureKsctlConfigFile() - if err != nil { - return cfg, nil, nil, err - } - kubeConfigFlags.KubeConfig = &kubeconfig + kubeConfigFlags.KubeConfig = &v.hostClusterData.kubeConfig - cl, err := ctx.NewClient(cfg.Token, cfg.ServerAPI) - if err != nil { - return cfg, nil, nil, err - } - return cfg, kubeConfigFlags, cl, nil + return cfg, kubeConfigFlags, v.hostClusterData.client, nil } func findToolchainClusterForMember(allToolchainClusters []toolchainv1alpha1.ToolchainCluster, memberAPIEndpoint, memberOperatorNamespace string) *toolchainv1alpha1.ToolchainCluster { diff --git a/pkg/cmd/adm/register_member_test.go b/pkg/cmd/adm/register_member_test.go index 3d54ecf..f621e3e 100644 --- a/pkg/cmd/adm/register_member_test.go +++ b/pkg/cmd/adm/register_member_test.go @@ -13,6 +13,7 @@ import ( "github.com/codeready-toolchain/toolchain-common/pkg/test" "github.com/ghodss/yaml" "github.com/kubesaw/ksctl/pkg/configuration" + clicontext "github.com/kubesaw/ksctl/pkg/context" . "github.com/kubesaw/ksctl/pkg/test" "github.com/kubesaw/ksctl/pkg/utils" "github.com/stretchr/testify/assert" @@ -82,7 +83,9 @@ func TestRegisterMember(t *testing.T) { } // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.NoError(t, err) @@ -115,7 +118,9 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.Error(t, err) @@ -135,7 +140,9 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.Error(t, err) @@ -155,7 +162,9 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.NoError(t, err) @@ -171,7 +180,9 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, true)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, true), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.NoError(t, err) @@ -206,7 +217,9 @@ func TestRegisterMember(t *testing.T) { mockCreateToolchainClusterWithReadyCondition(t, fakeClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "2")) + err := registerMemberCluster(ctx, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "2"), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.NoError(t, err) @@ -227,8 +240,12 @@ func TestRegisterMember(t *testing.T) { ctx2 := newExtendedCommandContext(term2, newClient) // when - err1 := registerMemberCluster(ctx1, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) - err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "1")) + err1 := registerMemberCluster(ctx1, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) + err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "1"), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.NoError(t, err1) @@ -250,8 +267,12 @@ func TestRegisterMember(t *testing.T) { ctx2 := newExtendedCommandContext(term2, newClient) // when - err1 := registerMemberCluster(ctx1, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) - err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "")) + err1 := registerMemberCluster(ctx1, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) + err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, ""), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.NoError(t, err1) @@ -305,7 +326,9 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, fakeClient.Create(context.TODO(), preexistingToolchainCluster2.DeepCopy())) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.Error(t, err) @@ -337,7 +360,9 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, fakeClient.Create(context.TODO(), preexistingToolchainCluster.DeepCopy())) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.Error(t, err) @@ -369,7 +394,9 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, fakeClient.Create(context.TODO(), preexistingToolchainCluster.DeepCopy())) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.Error(t, err) @@ -402,7 +429,9 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, fakeClient.Create(context.TODO(), preexistingToolchainCluster.DeepCopy())) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.Error(t, err) @@ -419,7 +448,9 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.Error(t, err) @@ -438,7 +469,9 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false)) + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return mockRestart(ctx, restartClusterName) + }) // then require.Error(t, err) diff --git a/pkg/cmd/adm/unregister_member_test.go b/pkg/cmd/adm/unregister_member_test.go index a456dd0..64c3cfa 100644 --- a/pkg/cmd/adm/unregister_member_test.go +++ b/pkg/cmd/adm/unregister_member_test.go @@ -67,7 +67,7 @@ func TestUnregisterMemberCallsRestart(t *testing.T) { // when err := UnregisterMemberCluster(ctxAct, "member1", func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { called++ - return mockRestart(ctx, restartClusterName, getConfigFlagsAndClient) + return mockRestart(ctx, restartClusterName) }) // then @@ -164,7 +164,7 @@ func TestUnregisterMemberLacksPermissions(t *testing.T) { AssertToolchainClusterSpec(t, fakeClient, toolchainCluster) } -func mockRestart(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { +func mockRestart(ctx *clicontext.CommandContext, clusterName string) error { if clusterName == "host" && ctx != nil { return nil } From a2d911260129c640f9f1b5fa53fc8feef93c16de Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Sun, 15 Dec 2024 21:23:22 +0530 Subject: [PATCH 3/9] clean up Signed-off-by: Feny Mehta --- pkg/cmd/adm/register_member.go | 8 ++++---- pkg/cmd/adm/register_member_test.go | 16 ++++++++++++++++ pkg/cmd/adm/restart.go | 18 +++++++++--------- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/adm/register_member.go b/pkg/cmd/adm/register_member.go index f3d6d8a..e6f7e56 100644 --- a/pkg/cmd/adm/register_member.go +++ b/pkg/cmd/adm/register_member.go @@ -487,7 +487,7 @@ func (v *registerMemberValidated) perform(ctx *extendedCommandContext) error { return err } - // restart Host Operator using the restart command + // restart Host Operator using the adm-restart command if err := v.restart(ctx.CommandContext, "host", v.getRegMemConfigFlagsAndClient); err != nil { return err } @@ -518,7 +518,7 @@ until the SpaceProvisionerConfig.spec.enabled is set to true. `, v.hostClusterData.apiEndpoint)) } -func (v *registerMemberValidated) getRegMemConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) (confg configuration.ClusterConfig, kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) { +func (v *registerMemberValidated) getRegMemConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) (kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) { kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() kubeConfigFlags.ClusterName = nil // `cluster` flag is redefined for our own purpose @@ -527,14 +527,14 @@ func (v *registerMemberValidated) getRegMemConfigFlagsAndClient(ctx *clicontext. cfg, err := configuration.LoadClusterConfig(ctx, clusterName) if err != nil { - return cfg, nil, nil, err + return nil, nil, err } kubeConfigFlags.Namespace = &v.hostClusterData.namespace kubeConfigFlags.APIServer = &v.hostClusterData.apiEndpoint kubeConfigFlags.BearerToken = &cfg.Token kubeConfigFlags.KubeConfig = &v.hostClusterData.kubeConfig - return cfg, kubeConfigFlags, v.hostClusterData.client, nil + return kubeConfigFlags, v.hostClusterData.client, nil } func findToolchainClusterForMember(allToolchainClusters []toolchainv1alpha1.ToolchainCluster, memberAPIEndpoint, memberOperatorNamespace string) *toolchainv1alpha1.ToolchainCluster { diff --git a/pkg/cmd/adm/register_member_test.go b/pkg/cmd/adm/register_member_test.go index f621e3e..c328265 100644 --- a/pkg/cmd/adm/register_member_test.go +++ b/pkg/cmd/adm/register_member_test.go @@ -482,6 +482,22 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, fakeClient.List(context.TODO(), tcs, runtimeclient.InNamespace(test.MemberOperatorNs))) assert.Empty(t, tcs.Items) }) + + t.Run("reports error when host-operator is not restarted", func(t *testing.T) { + // given + term := NewFakeTerminalWithResponse("Y") + newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient) + ctx := newExtendedCommandContext(term, newClient) + + // when + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + return fmt.Errorf("restart did not happen") + }) + + // then + require.EqualError(t, err, "restart did not happen") + }) } func mockCreateToolchainClusterInNamespaceWithReadyCondition(t *testing.T, fakeClient *test.FakeClient, namespace string) { diff --git a/pkg/cmd/adm/restart.go b/pkg/cmd/adm/restart.go index 6df1588..e9ea06a 100644 --- a/pkg/cmd/adm/restart.go +++ b/pkg/cmd/adm/restart.go @@ -23,7 +23,7 @@ import ( type ( RolloutRestartFunc func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error RolloutStatusCheckerFunc func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error - ConfigFlagsAndClientGetterFunc func(ctx *clicontext.CommandContext, clusterName string) (cfg configuration.ClusterConfig, kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) + ConfigFlagsAndClientGetterFunc func(ctx *clicontext.CommandContext, clusterName string) (kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) ) // NewRestartCmd() is a function to restart the whole operator, it relies on the target cluster and fetches the cluster config @@ -59,25 +59,25 @@ func restart(ctx *clicontext.CommandContext, clusterName string, configFlagsClie ErrOut: os.Stderr, } - cfg, kubeConfigFlags, cl, err := configFlagsClientGetter(ctx, clusterName) + kubeConfigFlags, cl, err := configFlagsClientGetter(ctx, clusterName) if err != nil { return err } factory := cmdutil.NewFactory(cmdutil.NewMatchVersionFlags(kubeConfigFlags)) if !ctx.AskForConfirmation( - ioutils.WithMessagef("restart all the deployments in the cluster '%s' and namespace '%s' \n", clusterName, cfg.OperatorNamespace)) { + ioutils.WithMessagef("restart all the deployments in the cluster '%s' and namespace '%s' \n", clusterName, *kubeConfigFlags.Namespace)) { return nil } - return restartDeployments(ctx, cl, cfg.OperatorNamespace, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + return restartDeployments(ctx, cl, *kubeConfigFlags.Namespace, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { return checkRolloutStatus(ctx, factory, ioStreams, deployment) }, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { return restartNonOlmDeployments(ctx, deployment, factory, ioStreams) }) } -func getConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) (confg configuration.ClusterConfig, kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) { +func getConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) (kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) { kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() kubeConfigFlags.ClusterName = nil // `cluster` flag is redefined for our own purpose @@ -86,22 +86,22 @@ func getConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) cfg, err := configuration.LoadClusterConfig(ctx, clusterName) if err != nil { - return cfg, nil, nil, err + return nil, nil, err } kubeConfigFlags.Namespace = &cfg.OperatorNamespace kubeConfigFlags.APIServer = &cfg.ServerAPI kubeConfigFlags.BearerToken = &cfg.Token kubeconfig, err := client.EnsureKsctlConfigFile() if err != nil { - return cfg, nil, nil, err + return nil, nil, err } kubeConfigFlags.KubeConfig = &kubeconfig cl, err := ctx.NewClient(cfg.Token, cfg.ServerAPI) if err != nil { - return cfg, nil, nil, err + return nil, nil, err } - return cfg, kubeConfigFlags, cl, nil + return kubeConfigFlags, cl, nil } // This function has the whole logic of getting the list of olm and non-olm based deployment, then proceed on restarting/deleting accordingly From 359ba1a88a2e1b9c9bfd383fd6d71762ea47a7e9 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Mon, 16 Dec 2024 17:44:19 +0530 Subject: [PATCH 4/9] e2e error fix Signed-off-by: Feny Mehta --- pkg/cmd/adm/register_member.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/cmd/adm/register_member.go b/pkg/cmd/adm/register_member.go index e6f7e56..41d9b7f 100644 --- a/pkg/cmd/adm/register_member.go +++ b/pkg/cmd/adm/register_member.go @@ -525,13 +525,8 @@ func (v *registerMemberValidated) getRegMemConfigFlagsAndClient(ctx *clicontext. kubeConfigFlags.AuthInfoName = nil // unused here, so we can hide it kubeConfigFlags.Context = nil // unused here, so we can hide it - cfg, err := configuration.LoadClusterConfig(ctx, clusterName) - if err != nil { - return nil, nil, err - } kubeConfigFlags.Namespace = &v.hostClusterData.namespace kubeConfigFlags.APIServer = &v.hostClusterData.apiEndpoint - kubeConfigFlags.BearerToken = &cfg.Token kubeConfigFlags.KubeConfig = &v.hostClusterData.kubeConfig return kubeConfigFlags, v.hostClusterData.client, nil From 09ba7d7fcc8569124674200333eab7dde92628ce Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Wed, 18 Dec 2024 11:58:36 +0530 Subject: [PATCH 5/9] rename to restartFunc in reg-mem Signed-off-by: Feny Mehta --- pkg/cmd/adm/register_member.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/adm/register_member.go b/pkg/cmd/adm/register_member.go index 41d9b7f..8b5a168 100644 --- a/pkg/cmd/adm/register_member.go +++ b/pkg/cmd/adm/register_member.go @@ -353,7 +353,7 @@ type registerMemberValidated struct { memberClusterData clusterData warnings []string errors []string - restart func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error + restartFunc func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error } func getApiEndpointAndClient(ctx *extendedCommandContext, kubeConfigPath string) (apiEndpoint string, cl runtimeclient.Client, err error) { @@ -445,9 +445,9 @@ func validateArgs(ctx *extendedCommandContext, args registerMemberArgs, restart toolchainClusterName: memberToolchainClusterName, kubeConfig: args.memberKubeConfig, }, - warnings: warnings, - errors: errors, - restart: restart, + warnings: warnings, + errors: errors, + restartFunc: restart, }, nil } @@ -488,7 +488,7 @@ func (v *registerMemberValidated) perform(ctx *extendedCommandContext) error { } // restart Host Operator using the adm-restart command - if err := v.restart(ctx.CommandContext, "host", v.getRegMemConfigFlagsAndClient); err != nil { + if err := v.restartFunc(ctx.CommandContext, "host", v.getRegMemConfigFlagsAndClient); err != nil { return err } From a56a109b8e5c1cfe675dbf50ecd5de49c76a70a0 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Wed, 18 Dec 2024 13:20:23 +0530 Subject: [PATCH 6/9] some changes Signed-off-by: Feny Mehta --- pkg/cmd/adm/register_member_test.go | 42 +++++++++++++++++---------- pkg/cmd/adm/unregister_member_test.go | 8 +++-- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/adm/register_member_test.go b/pkg/cmd/adm/register_member_test.go index c328265..5ed5b25 100644 --- a/pkg/cmd/adm/register_member_test.go +++ b/pkg/cmd/adm/register_member_test.go @@ -84,7 +84,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -119,7 +119,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -141,7 +141,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -163,7 +163,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -181,7 +181,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, true), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -218,7 +218,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "2"), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -241,10 +241,10 @@ func TestRegisterMember(t *testing.T) { // when err1 := registerMemberCluster(ctx1, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "1"), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -268,10 +268,10 @@ func TestRegisterMember(t *testing.T) { // when err1 := registerMemberCluster(ctx1, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, ""), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -327,7 +327,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -361,7 +361,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -395,7 +395,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -430,7 +430,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -449,7 +449,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -470,7 +470,7 @@ func TestRegisterMember(t *testing.T) { // when err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { - return mockRestart(ctx, restartClusterName) + return nil }) // then @@ -498,6 +498,7 @@ func TestRegisterMember(t *testing.T) { // then require.EqualError(t, err, "restart did not happen") }) + } func mockCreateToolchainClusterInNamespaceWithReadyCondition(t *testing.T, fakeClient *test.FakeClient, namespace string) { @@ -634,3 +635,12 @@ func defaultRegisterMemberArgs() registerMemberArgs { return args } + +// func (v *registerMemberValidated) mockRestartRegMem(ctx *clicontext.CommandContext, clusterName string) error { +// kf, _, err := v.getRegMemConfigFlagsAndClient(ctx, clusterName) + +// if kf != nil && err == nil { +// return nil +// } +// return fmt.Errorf("error in getting config flags") +// } diff --git a/pkg/cmd/adm/unregister_member_test.go b/pkg/cmd/adm/unregister_member_test.go index 64c3cfa..2885e1f 100644 --- a/pkg/cmd/adm/unregister_member_test.go +++ b/pkg/cmd/adm/unregister_member_test.go @@ -67,7 +67,7 @@ func TestUnregisterMemberCallsRestart(t *testing.T) { // when err := UnregisterMemberCluster(ctxAct, "member1", func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { called++ - return mockRestart(ctx, restartClusterName) + return mockRestart(ctx, restartClusterName, getConfigFlagsAndClient) }) // then @@ -164,9 +164,13 @@ func TestUnregisterMemberLacksPermissions(t *testing.T) { AssertToolchainClusterSpec(t, fakeClient, toolchainCluster) } -func mockRestart(ctx *clicontext.CommandContext, clusterName string) error { +func mockRestart(ctx *clicontext.CommandContext, clusterName string, cfc ConfigFlagsAndClientGetterFunc) error { if clusterName == "host" && ctx != nil { return nil } + _, _, err := cfc(ctx, clusterName) + if err == nil { + return nil + } return fmt.Errorf("cluster name is wrong") } From 8c3156974989b7c7aaf51ebf3de9db62e38be93d Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Wed, 18 Dec 2024 13:37:49 +0530 Subject: [PATCH 7/9] clean up Signed-off-by: Feny Mehta --- pkg/cmd/adm/register_member.go | 1 - pkg/cmd/adm/register_member_test.go | 9 --------- 2 files changed, 10 deletions(-) diff --git a/pkg/cmd/adm/register_member.go b/pkg/cmd/adm/register_member.go index 8b5a168..46a934b 100644 --- a/pkg/cmd/adm/register_member.go +++ b/pkg/cmd/adm/register_member.go @@ -45,7 +45,6 @@ type newClientFromRestConfigFunc func(*rest.Config) (runtimeclient.Client, error type extendedCommandContext struct { *clicontext.CommandContext NewClientFromRestConfig newClientFromRestConfigFunc - RestartFunc func(ctx *clicontext.CommandContext, clusterName string) (cfg configuration.ClusterConfig, kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) } func newExtendedCommandContext(term ioutils.Terminal, clientCtor newClientFromRestConfigFunc) *extendedCommandContext { diff --git a/pkg/cmd/adm/register_member_test.go b/pkg/cmd/adm/register_member_test.go index 5ed5b25..27c71ec 100644 --- a/pkg/cmd/adm/register_member_test.go +++ b/pkg/cmd/adm/register_member_test.go @@ -635,12 +635,3 @@ func defaultRegisterMemberArgs() registerMemberArgs { return args } - -// func (v *registerMemberValidated) mockRestartRegMem(ctx *clicontext.CommandContext, clusterName string) error { -// kf, _, err := v.getRegMemConfigFlagsAndClient(ctx, clusterName) - -// if kf != nil && err == nil { -// return nil -// } -// return fmt.Errorf("error in getting config flags") -// } From a05065ec42b65641228054641e05b47a2fe10450 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Wed, 18 Dec 2024 16:48:46 +0530 Subject: [PATCH 8/9] cosmetic changes and cleanup Signed-off-by: Feny Mehta --- pkg/cmd/adm/register_member.go | 2 +- pkg/cmd/adm/register_member_test.go | 34 +++++++++++++-------------- pkg/cmd/adm/restart_test.go | 28 +++++++++++----------- pkg/cmd/adm/unregister_member_test.go | 22 ++++++++--------- 4 files changed, 42 insertions(+), 44 deletions(-) diff --git a/pkg/cmd/adm/register_member.go b/pkg/cmd/adm/register_member.go index 46a934b..c9d240c 100644 --- a/pkg/cmd/adm/register_member.go +++ b/pkg/cmd/adm/register_member.go @@ -487,7 +487,7 @@ func (v *registerMemberValidated) perform(ctx *extendedCommandContext) error { } // restart Host Operator using the adm-restart command - if err := v.restartFunc(ctx.CommandContext, "host", v.getRegMemConfigFlagsAndClient); err != nil { + if err := v.restartFunc(ctx.CommandContext, configuration.HostName, v.getRegMemConfigFlagsAndClient); err != nil { return err } diff --git a/pkg/cmd/adm/register_member_test.go b/pkg/cmd/adm/register_member_test.go index 27c71ec..6fac77e 100644 --- a/pkg/cmd/adm/register_member_test.go +++ b/pkg/cmd/adm/register_member_test.go @@ -83,7 +83,7 @@ func TestRegisterMember(t *testing.T) { } // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -118,7 +118,7 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -140,7 +140,7 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -162,7 +162,7 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -180,7 +180,7 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, true), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, true), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -217,7 +217,7 @@ func TestRegisterMember(t *testing.T) { mockCreateToolchainClusterWithReadyCondition(t, fakeClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "2"), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "2"), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -240,10 +240,10 @@ func TestRegisterMember(t *testing.T) { ctx2 := newExtendedCommandContext(term2, newClient) // when - err1 := registerMemberCluster(ctx1, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err1 := registerMemberCluster(ctx1, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) - err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "1"), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "1"), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -267,10 +267,10 @@ func TestRegisterMember(t *testing.T) { ctx2 := newExtendedCommandContext(term2, newClient) // when - err1 := registerMemberCluster(ctx1, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err1 := registerMemberCluster(ctx1, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) - err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, ""), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, ""), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -326,7 +326,7 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, fakeClient.Create(context.TODO(), preexistingToolchainCluster2.DeepCopy())) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -360,7 +360,7 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, fakeClient.Create(context.TODO(), preexistingToolchainCluster.DeepCopy())) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -394,7 +394,7 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, fakeClient.Create(context.TODO(), preexistingToolchainCluster.DeepCopy())) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -429,7 +429,7 @@ func TestRegisterMember(t *testing.T) { require.NoError(t, fakeClient.Create(context.TODO(), preexistingToolchainCluster.DeepCopy())) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -448,7 +448,7 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -469,7 +469,7 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -491,7 +491,7 @@ func TestRegisterMember(t *testing.T) { ctx := newExtendedCommandContext(term, newClient) // when - err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return fmt.Errorf("restart did not happen") }) diff --git a/pkg/cmd/adm/restart_test.go b/pkg/cmd/adm/restart_test.go index 6ba355a..d100542 100644 --- a/pkg/cmd/adm/restart_test.go +++ b/pkg/cmd/adm/restart_test.go @@ -91,7 +91,7 @@ func TestKubectlRolloutFunctionality(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) //when - err := restartDeployments(ctx, fakeClient, HostNamespacedName.Namespace, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + err := restartDeployments(ctx, fakeClient, HostNamespacedName.Namespace, func(ctx *clicontext.CommandContext, _ appsv1.Deployment) error { return checkRolloutStatus(ctx, tf, streams, *hostDep) }, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { return restartNonOlmDeployments(ctx, deployment, tf, streams) @@ -123,7 +123,7 @@ func TestKubectlRolloutFunctionality(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) //when - err := restartDeployments(ctx, fakeClient, HostNamespacedName.Namespace, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + err := restartDeployments(ctx, fakeClient, HostNamespacedName.Namespace, func(ctx *clicontext.CommandContext, _ appsv1.Deployment) error { return checkRolloutStatus(ctx, tf, streams, *hostDep) }, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { return restartNonOlmDeployments(ctx, deployment, tf, streams) @@ -140,7 +140,7 @@ func TestKubectlRolloutFunctionality(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) //when - err := restartDeployments(ctx, fakeClient, HostNamespacedName.Namespace, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + err := restartDeployments(ctx, fakeClient, HostNamespacedName.Namespace, func(ctx *clicontext.CommandContext, _ appsv1.Deployment) error { return checkRolloutStatus(ctx, tf, streams, *hostDep) }, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { return restartNonOlmDeployments(ctx, deployment, tf, streams) @@ -181,10 +181,10 @@ func TestRestartDeployment(t *testing.T) { //when err := restartDeployments(ctx, fakeClient, "toolchain-host-operator", - func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + func(_ *clicontext.CommandContext, deployment appsv1.Deployment) error { require.Equal(t, "host-operator-controller-manager", deployment.Name) return nil - }, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + }, func(_ *clicontext.CommandContext, deployment appsv1.Deployment) error { require.Equal(t, regServDeployment, deployment) return nil }) @@ -200,9 +200,9 @@ func TestRestartDeployment(t *testing.T) { //when err := restartDeployments(ctx, fakeClient, "toolchain-host-operator", - func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + func(_ *clicontext.CommandContext, _ appsv1.Deployment) error { return nil - }, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + }, func(_ *clicontext.CommandContext, _ appsv1.Deployment) error { return nil }) @@ -228,9 +228,9 @@ func TestRestartDeployment(t *testing.T) { //when err := restartDeployments(ctx, fakeClient, "toolchain-host-operator", - func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + func(_ *clicontext.CommandContext, _ appsv1.Deployment) error { return nil - }, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + }, func(_ *clicontext.CommandContext, _ appsv1.Deployment) error { return nil }) @@ -246,9 +246,9 @@ func TestRestartDeployment(t *testing.T) { expectedErr := fmt.Errorf("Could not do rollout restart of the deployment") //when err := restartDeployments(ctx, fakeClient, "toolchain-host-operator", - func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + func(_ *clicontext.CommandContext, _ appsv1.Deployment) error { return nil - }, func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + }, func(_ *clicontext.CommandContext, _ appsv1.Deployment) error { return expectedErr }) @@ -263,7 +263,7 @@ func TestRestartDeployment(t *testing.T) { //when err := restartDeployments(ctx, fakeClient, "toolchain-host-operator", - func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + func(_ *clicontext.CommandContext, _ appsv1.Deployment) error { return nil }, nil) @@ -278,7 +278,7 @@ func TestRestartDeployment(t *testing.T) { expectedErr := fmt.Errorf("Could not check the status of the deployment") //when err := restartDeployments(ctx, fakeClient, "toolchain-host-operator", - func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + func(_ *clicontext.CommandContext, _ appsv1.Deployment) error { return expectedErr }, nil) @@ -310,7 +310,7 @@ func TestRestartAutoScalerDeployment(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) //when err := restartDeployments(ctx, fakeClient, "toolchain-member-operator", - func(ctx *clicontext.CommandContext, deployment appsv1.Deployment) error { + func(_ *clicontext.CommandContext, _ appsv1.Deployment) error { return nil }, mockRolloutRestartInterceptor()) diff --git a/pkg/cmd/adm/unregister_member_test.go b/pkg/cmd/adm/unregister_member_test.go index 2885e1f..0e42cdd 100644 --- a/pkg/cmd/adm/unregister_member_test.go +++ b/pkg/cmd/adm/unregister_member_test.go @@ -21,7 +21,7 @@ func TestUnregisterMemberWhenAnswerIsY(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := UnregisterMemberCluster(ctx, "member1", func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -46,7 +46,7 @@ func TestUnregisterMemberWhenRestartError(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := UnregisterMemberCluster(ctx, "member1", func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return fmt.Errorf("restart did not happen") }) @@ -65,7 +65,7 @@ func TestUnregisterMemberCallsRestart(t *testing.T) { ctxAct := clicontext.NewCommandContext(term, newClient) called := 0 // when - err := UnregisterMemberCluster(ctxAct, "member1", func(ctx *clicontext.CommandContext, restartClusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := UnregisterMemberCluster(ctxAct, "member1", func(ctx *clicontext.CommandContext, restartClusterName string, _ ConfigFlagsAndClientGetterFunc) error { called++ return mockRestart(ctx, restartClusterName, getConfigFlagsAndClient) }) @@ -84,7 +84,7 @@ func TestUnregisterMemberWhenAnswerIsN(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := UnregisterMemberCluster(ctx, "member1", func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -107,7 +107,7 @@ func TestUnregisterMemberWhenNotFound(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := UnregisterMemberCluster(ctx, "member1", func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -130,7 +130,7 @@ func TestUnregisterMemberWhenUnknownClusterName(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "some", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := UnregisterMemberCluster(ctx, "some", func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -155,7 +155,7 @@ func TestUnregisterMemberLacksPermissions(t *testing.T) { ctx := clicontext.NewCommandContext(term, newClient) // when - err := UnregisterMemberCluster(ctx, "member1", func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error { + err := UnregisterMemberCluster(ctx, "member1", func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { return nil }) @@ -165,12 +165,10 @@ func TestUnregisterMemberLacksPermissions(t *testing.T) { } func mockRestart(ctx *clicontext.CommandContext, clusterName string, cfc ConfigFlagsAndClientGetterFunc) error { - if clusterName == "host" && ctx != nil { - return nil - } - _, _, err := cfc(ctx, clusterName) - if err == nil { + _, _, cfcerr := cfc(ctx, clusterName) + if clusterName == "host" && ctx != nil && cfcerr == nil { return nil } + return fmt.Errorf("cluster name is wrong") } From 53aaed191f5cd520728f87c1052715d65880a2d1 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Fri, 20 Dec 2024 17:19:02 +0530 Subject: [PATCH 9/9] improving the readability and cosmetic changes Signed-off-by: Feny Mehta --- pkg/cmd/adm/register_member.go | 18 ++++++++---------- pkg/cmd/adm/register_member_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/adm/register_member.go b/pkg/cmd/adm/register_member.go index c9d240c..7bd6d5a 100644 --- a/pkg/cmd/adm/register_member.go +++ b/pkg/cmd/adm/register_member.go @@ -96,7 +96,7 @@ func NewRegisterMemberCmd() *cobra.Command { } func registerMemberCluster(ctx *extendedCommandContext, args registerMemberArgs, restart restartFunc) error { - validated, err := validateArgs(ctx, args, restart) + validated, err := validateArgs(ctx, args) if err != nil { return err } @@ -115,7 +115,7 @@ func registerMemberCluster(ctx *extendedCommandContext, args registerMemberArgs, return nil } - return validated.perform(ctx) + return validated.perform(ctx, restart) } func (v *registerMemberValidated) getSourceAndTargetClusters(sourceClusterType configuration.ClusterType) (clusterData, clusterData) { @@ -352,7 +352,6 @@ type registerMemberValidated struct { memberClusterData clusterData warnings []string errors []string - restartFunc func(ctx *clicontext.CommandContext, clusterName string, cfcGetter ConfigFlagsAndClientGetterFunc) error } func getApiEndpointAndClient(ctx *extendedCommandContext, kubeConfigPath string) (apiEndpoint string, cl runtimeclient.Client, err error) { @@ -376,7 +375,7 @@ func getApiEndpointAndClient(ctx *extendedCommandContext, kubeConfigPath string) return } -func validateArgs(ctx *extendedCommandContext, args registerMemberArgs, restart restartFunc) (*registerMemberValidated, error) { +func validateArgs(ctx *extendedCommandContext, args registerMemberArgs) (*registerMemberValidated, error) { hostApiEndpoint, hostClusterClient, err := getApiEndpointAndClient(ctx, args.hostKubeConfig) if err != nil { return nil, err @@ -444,9 +443,8 @@ func validateArgs(ctx *extendedCommandContext, args registerMemberArgs, restart toolchainClusterName: memberToolchainClusterName, kubeConfig: args.memberKubeConfig, }, - warnings: warnings, - errors: errors, - restartFunc: restart, + warnings: warnings, + errors: errors, }, nil } @@ -473,7 +471,7 @@ func (v *registerMemberValidated) confirmationPrompt() ioutils.ConfirmationMessa return ioutils.WithMessagef(sb.String(), args...) } -func (v *registerMemberValidated) perform(ctx *extendedCommandContext) error { +func (v *registerMemberValidated) perform(ctx *extendedCommandContext, restart restartFunc) error { // add the host entry to the member cluster first. We assume that there is just 1 toolchain cluster entry in the member // cluster (i.e. it just points back to the host), so there's no need to determine the number of entries with the same // API endpoint. @@ -487,7 +485,7 @@ func (v *registerMemberValidated) perform(ctx *extendedCommandContext) error { } // restart Host Operator using the adm-restart command - if err := v.restartFunc(ctx.CommandContext, configuration.HostName, v.getRegMemConfigFlagsAndClient); err != nil { + if err := restart(ctx.CommandContext, configuration.HostName, v.getRegMemConfigFlagsAndClient); err != nil { return err } @@ -517,7 +515,7 @@ until the SpaceProvisionerConfig.spec.enabled is set to true. `, v.hostClusterData.apiEndpoint)) } -func (v *registerMemberValidated) getRegMemConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) (kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) { +func (v *registerMemberValidated) getRegMemConfigFlagsAndClient(_ *clicontext.CommandContext, _ string) (kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) { kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() kubeConfigFlags.ClusterName = nil // `cluster` flag is redefined for our own purpose diff --git a/pkg/cmd/adm/register_member_test.go b/pkg/cmd/adm/register_member_test.go index 6fac77e..3ba43f1 100644 --- a/pkg/cmd/adm/register_member_test.go +++ b/pkg/cmd/adm/register_member_test.go @@ -499,6 +499,24 @@ func TestRegisterMember(t *testing.T) { require.EqualError(t, err, "restart did not happen") }) + t.Run("Register-member calls restart ", func(t *testing.T) { + // given + term := NewFakeTerminalWithResponse("Y") + newClient, fakeClient := newFakeClientsFromRestConfig(t, &toolchainClusterMemberSa, &toolchainClusterHostSa) + mockCreateToolchainClusterWithReadyCondition(t, fakeClient) + ctx := newExtendedCommandContext(term, newClient) + called := 0 + // when + err := registerMemberCluster(ctx, newRegisterMemberArgsWith(hostKubeconfig, memberKubeconfig, false), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error { + called++ + return mockRestartReg(ctx.CommandContext, configuration.HostName, nil) + }) + + // then + require.NoError(t, err) + assert.Equal(t, 1, called) + }) + } func mockCreateToolchainClusterInNamespaceWithReadyCondition(t *testing.T, fakeClient *test.FakeClient, namespace string) { @@ -635,3 +653,10 @@ func defaultRegisterMemberArgs() registerMemberArgs { return args } + +func mockRestartReg(ctx *clicontext.CommandContext, clusterName string, _ ConfigFlagsAndClientGetterFunc) error { + if clusterName == "host" && ctx != nil { + return nil + } + return fmt.Errorf("cluster name is wrong") +}