Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KUBESAW-172: Restart host-operator at the end of the register-member command #90

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
+154 −69
Open

KUBESAW-172: Restart host-operator at the end of the register-member command #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
85eae83
KUBESAW-172: Restart host-operator at the end of the register-member …
fbm3307 Dec 9, 2024
7bff803
Register member test cases
fbm3307 Dec 14, 2024
cbb8628
Merge branch 'master' into ks172restart_host
fbm3307 Dec 14, 2024
a2d9112
clean up
fbm3307 Dec 15, 2024
175a4c4
Merge branch 'ks172restart_host' of https://github.com/fbm3307/ksctl …
fbm3307 Dec 15, 2024
359ba1a
e2e error fix
fbm3307 Dec 16, 2024
09ba7d7
rename to restartFunc in reg-mem
fbm3307 Dec 18, 2024
a56a109
some changes
fbm3307 Dec 18, 2024
8c31569
clean up
fbm3307 Dec 18, 2024
a05065e
cosmetic changes and cleanup
fbm3307 Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions pkg/cmd/adm/register_member.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
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"
Expand Down Expand Up @@ -72,7 +73,7 @@
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)

Check warning on line 76 in pkg/cmd/adm/register_member.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/adm/register_member.go#L76 

Added line #L76 was not covered by tests
},
}

Expand All @@ -94,8 +95,8 @@
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
}
Expand Down Expand Up @@ -351,6 +352,7 @@
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) {
Expand All @@ -374,7 +376,7 @@
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
Expand Down Expand Up @@ -442,8 +444,9 @@
toolchainClusterName: memberToolchainClusterName,
kubeConfig: args.memberKubeConfig,
},
warnings: warnings,
errors: errors,
warnings: warnings,
errors: errors,
restartFunc: restart,
}, nil
}

Expand Down Expand Up @@ -483,6 +486,11 @@
return err
}

// restart Host Operator using the adm-restart command
if err := v.restartFunc(ctx.CommandContext, configuration.HostName, v.getRegMemConfigFlagsAndClient); err != nil {
return err
}

exampleSPC := &toolchainv1alpha1.SpaceProvisionerConfig{
TypeMeta: metav1.TypeMeta{
Kind: "SpaceProvisionerConfig",
Expand All @@ -509,6 +517,20 @@
`, v.hostClusterData.apiEndpoint))
}

func (v *registerMemberValidated) getRegMemConfigFlagsAndClient(ctx *clicontext.CommandContext, clusterName string) (kubeConfigFlag *genericclioptions.ConfigFlags, rccl runtimeclient.Client, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor - do you think it makes sense to cover this with a test?

Copy link
Contributor Author

@fbm3307 fbm3307 Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, but couldn't find a easier way to test it, and also the similar functionality we did not test it in restart.go, but somehow thats being covered, do you have pointers for it to be tested?

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

kubeConfigFlags.Namespace = &v.hostClusterData.namespace
kubeConfigFlags.APIServer = &v.hostClusterData.apiEndpoint
kubeConfigFlags.KubeConfig = &v.hostClusterData.kubeConfig

return kubeConfigFlags, v.hostClusterData.client, nil

Check warning on line 531 in pkg/cmd/adm/register_member.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/adm/register_member.go#L520-L531 

Added lines #L520 - L531 were not covered by tests
}

func findToolchainClusterForMember(allToolchainClusters []toolchainv1alpha1.ToolchainCluster, memberAPIEndpoint, memberOperatorNamespace string) *toolchainv1alpha1.ToolchainCluster {
for _, tc := range allToolchainClusters {
if tc.Status.APIEndpoint == memberAPIEndpoint && tc.Status.OperatorNamespace == memberOperatorNamespace {
Expand Down
82 changes: 66 additions & 16 deletions pkg/cmd/adm/register_member_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.NoError(t, err)
Expand Down Expand Up @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.Error(t, err)
Expand All @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.Error(t, err)
Expand All @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.NoError(t, err)
Expand All @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.NoError(t, err)
Expand Down Expand Up @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.NoError(t, err)
Expand All @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})
err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, "1"), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.NoError(t, err1)
Expand All @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})
err2 := registerMemberCluster(ctx2, newRegisterMemberArgsWithSuffix(hostKubeconfig, memberKubeconfig, false, ""), func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.NoError(t, err1)
Expand Down Expand Up @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.Error(t, err)
Expand Down Expand Up @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.Error(t, err)
Expand Down Expand Up @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.Error(t, err)
Expand Down Expand Up @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.Error(t, err)
Expand All @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.Error(t, err)
Expand All @@ -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(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
return nil
})

// then
require.Error(t, err)
Expand All @@ -449,6 +482,23 @@ 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(_ *clicontext.CommandContext, _ string, _ 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) {
Expand Down
51 changes: 31 additions & 20 deletions pkg/cmd/adm/restart.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@
)

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) (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
Expand All @@ -45,52 +46,62 @@
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)

Check warning on line 49 in pkg/cmd/adm/restart.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/adm/restart.go#L49 

Added line #L49 was not covered by tests
},
}
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,
}

kubeConfigFlags, cl, err := configFlagsClientGetter(ctx, clusterName)
if err != nil {
return err
}

Check warning on line 65 in pkg/cmd/adm/restart.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/adm/restart.go#L64-L65 

Added lines #L64 - L65 were not covered by tests
factory := cmdutil.NewFactory(cmdutil.NewMatchVersionFlags(kubeConfigFlags))

if !ctx.AskForConfirmation(
ioutils.WithMessagef("restart all the deployments in the cluster '%s' and namespace '%s' \n", clusterName, *kubeConfigFlags.Namespace)) {
return nil
}

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

Check warning on line 77 in pkg/cmd/adm/restart.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/adm/restart.go#L73-L77 

Added lines #L73 - L77 were not covered by tests
}

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
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 nil, nil, err

Check warning on line 89 in pkg/cmd/adm/restart.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/adm/restart.go#L89 

Added line #L89 was not covered by tests
}
kubeConfigFlags.Namespace = &cfg.OperatorNamespace
kubeConfigFlags.APIServer = &cfg.ServerAPI
kubeConfigFlags.BearerToken = &cfg.Token
kubeconfig, err := client.EnsureKsctlConfigFile()
if err != nil {
return err
return nil, nil, err

Check warning on line 96 in pkg/cmd/adm/restart.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/adm/restart.go#L96 

Added line #L96 was not covered by tests
}
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 nil, nil, err

Check warning on line 102 in pkg/cmd/adm/restart.go

View check run for this annotation

Codecov / codecov/patch

pkg/cmd/adm/restart.go#L102 

Added line #L102 was not covered by tests
}

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 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
Expand Down
Loading
Loading