From efe3728d21683786bb7143ce6b2f57d68e0a18d5 Mon Sep 17 00:00:00 2001 From: Hiromu Asahina Date: Wed, 18 Jan 2023 02:03:06 +0900 Subject: [PATCH] Refactor rollout --- cmd/clusterctl/client/client.go | 8 +- cmd/clusterctl/client/client_test.go | 8 +- cmd/clusterctl/client/rollout.go | 75 +++-- cmd/clusterctl/client/rollout_test.go | 261 +++++++++++++----- cmd/clusterctl/cmd/rollout/pause.go | 2 +- cmd/clusterctl/cmd/rollout/restart.go | 2 +- cmd/clusterctl/cmd/rollout/resume.go | 2 +- cmd/clusterctl/cmd/rollout/undo.go | 2 +- .../src/developer/providers/v1.3-to-v1.4.md | 1 + 9 files changed, 263 insertions(+), 98 deletions(-) diff --git a/cmd/clusterctl/client/client.go b/cmd/clusterctl/client/client.go index 32d42dfac1ca..05d229503220 100644 --- a/cmd/clusterctl/client/client.go +++ b/cmd/clusterctl/client/client.go @@ -79,13 +79,13 @@ type Client interface { // AlphaClient exposes the alpha features in clusterctl high-level client library. type AlphaClient interface { // RolloutRestart provides rollout restart of cluster-api resources - RolloutRestart(options RolloutOptions) error + RolloutRestart(options RolloutRestartOptions) error // RolloutPause provides rollout pause of cluster-api resources - RolloutPause(options RolloutOptions) error + RolloutPause(options RolloutPauseOptions) error // RolloutResume provides rollout resume of paused cluster-api resources - RolloutResume(options RolloutOptions) error + RolloutResume(options RolloutResumeOptions) error // RolloutUndo provides rollout rollback of cluster-api resources - RolloutUndo(options RolloutOptions) error + RolloutUndo(options RolloutUndoOptions) error // TopologyPlan dry runs the topology reconciler TopologyPlan(options TopologyPlanOptions) (*TopologyPlanOutput, error) } diff --git a/cmd/clusterctl/client/client_test.go b/cmd/clusterctl/client/client_test.go index 486384a2e0bc..b1374b805255 100644 --- a/cmd/clusterctl/client/client_test.go +++ b/cmd/clusterctl/client/client_test.go @@ -124,7 +124,7 @@ func (f fakeClient) ProcessYAML(options ProcessYAMLOptions) (YamlPrinter, error) return f.internalClient.ProcessYAML(options) } -func (f fakeClient) RolloutRestart(options RolloutOptions) error { +func (f fakeClient) RolloutRestart(options RolloutRestartOptions) error { return f.internalClient.RolloutRestart(options) } @@ -132,15 +132,15 @@ func (f fakeClient) DescribeCluster(options DescribeClusterOptions) (*tree.Objec return f.internalClient.DescribeCluster(options) } -func (f fakeClient) RolloutPause(options RolloutOptions) error { +func (f fakeClient) RolloutPause(options RolloutPauseOptions) error { return f.internalClient.RolloutPause(options) } -func (f fakeClient) RolloutResume(options RolloutOptions) error { +func (f fakeClient) RolloutResume(options RolloutResumeOptions) error { return f.internalClient.RolloutResume(options) } -func (f fakeClient) RolloutUndo(options RolloutOptions) error { +func (f fakeClient) RolloutUndo(options RolloutUndoOptions) error { return f.internalClient.RolloutUndo(options) } diff --git a/cmd/clusterctl/client/rollout.go b/cmd/clusterctl/client/rollout.go index 2a932f82e8ae..c69ef2edab4b 100644 --- a/cmd/clusterctl/client/rollout.go +++ b/cmd/clusterctl/client/rollout.go @@ -26,8 +26,50 @@ import ( "sigs.k8s.io/cluster-api/cmd/clusterctl/internal/util" ) -// RolloutOptions carries the base set of options supported by rollout command. -type RolloutOptions struct { +// RolloutRestartOptions carries the options supported by RolloutRestart. +type RolloutRestartOptions struct { + // Kubeconfig defines the kubeconfig to use for accessing the management cluster. If empty, + // default rules for kubeconfig discovery will be used. + Kubeconfig Kubeconfig + + // Resources for the rollout command + Resources []string + + // Namespace where the resource(s) live. If unspecified, the namespace name will be inferred + // from the current configuration. + Namespace string +} + +// RolloutPauseOptions carries the options supported by RolloutPause. +type RolloutPauseOptions struct { + // Kubeconfig defines the kubeconfig to use for accessing the management cluster. If empty, + // default rules for kubeconfig discovery will be used. + Kubeconfig Kubeconfig + + // Resources for the rollout command + Resources []string + + // Namespace where the resource(s) live. If unspecified, the namespace name will be inferred + // from the current configuration. + Namespace string +} + +// RolloutResumeOptions carries the options supported by RolloutResume. +type RolloutResumeOptions struct { + // Kubeconfig defines the kubeconfig to use for accessing the management cluster. If empty, + // default rules for kubeconfig discovery will be used. + Kubeconfig Kubeconfig + + // Resources for the rollout command + Resources []string + + // Namespace where the resource(s) live. If unspecified, the namespace name will be inferred + // from the current configuration. + Namespace string +} + +// RolloutUndoOptions carries the options supported by RolloutUndo. +type RolloutUndoOptions struct { // Kubeconfig defines the kubeconfig to use for accessing the management cluster. If empty, // default rules for kubeconfig discovery will be used. Kubeconfig Kubeconfig @@ -40,16 +82,15 @@ type RolloutOptions struct { Namespace string // Revision number to rollback to when issuing the undo command. - // Revision number of a specific revision when issuing the history command. ToRevision int64 } -func (c *clusterctlClient) RolloutRestart(options RolloutOptions) error { +func (c *clusterctlClient) RolloutRestart(options RolloutRestartOptions) error { clusterClient, err := c.clusterClientFactory(ClusterClientFactoryInput{Kubeconfig: options.Kubeconfig}) if err != nil { return err } - objRefs, err := getObjectRefs(clusterClient, options) + objRefs, err := getObjectRefs(clusterClient, options.Namespace, options.Resources) if err != nil { return err } @@ -61,12 +102,12 @@ func (c *clusterctlClient) RolloutRestart(options RolloutOptions) error { return nil } -func (c *clusterctlClient) RolloutPause(options RolloutOptions) error { +func (c *clusterctlClient) RolloutPause(options RolloutPauseOptions) error { clusterClient, err := c.clusterClientFactory(ClusterClientFactoryInput{Kubeconfig: options.Kubeconfig}) if err != nil { return err } - objRefs, err := getObjectRefs(clusterClient, options) + objRefs, err := getObjectRefs(clusterClient, options.Namespace, options.Resources) if err != nil { return err } @@ -78,12 +119,12 @@ func (c *clusterctlClient) RolloutPause(options RolloutOptions) error { return nil } -func (c *clusterctlClient) RolloutResume(options RolloutOptions) error { +func (c *clusterctlClient) RolloutResume(options RolloutResumeOptions) error { clusterClient, err := c.clusterClientFactory(ClusterClientFactoryInput{Kubeconfig: options.Kubeconfig}) if err != nil { return err } - objRefs, err := getObjectRefs(clusterClient, options) + objRefs, err := getObjectRefs(clusterClient, options.Namespace, options.Resources) if err != nil { return err } @@ -95,12 +136,12 @@ func (c *clusterctlClient) RolloutResume(options RolloutOptions) error { return nil } -func (c *clusterctlClient) RolloutUndo(options RolloutOptions) error { +func (c *clusterctlClient) RolloutUndo(options RolloutUndoOptions) error { clusterClient, err := c.clusterClientFactory(ClusterClientFactoryInput{Kubeconfig: options.Kubeconfig}) if err != nil { return err } - objRefs, err := getObjectRefs(clusterClient, options) + objRefs, err := getObjectRefs(clusterClient, options.Namespace, options.Resources) if err != nil { return err } @@ -112,21 +153,21 @@ func (c *clusterctlClient) RolloutUndo(options RolloutOptions) error { return nil } -func getObjectRefs(clusterClient cluster.Client, options RolloutOptions) ([]corev1.ObjectReference, error) { +func getObjectRefs(clusterClient cluster.Client, namespace string, resources []string) ([]corev1.ObjectReference, error) { // If the option specifying the Namespace is empty, try to detect it. - if options.Namespace == "" { + if namespace == "" { currentNamespace, err := clusterClient.Proxy().CurrentNamespace() if err != nil { return []corev1.ObjectReference{}, err } - options.Namespace = currentNamespace + namespace = currentNamespace } - if len(options.Resources) == 0 { + if len(resources) == 0 { return []corev1.ObjectReference{}, fmt.Errorf("required resource not specified") } - normalized := normalizeResources(options.Resources) - objRefs, err := util.GetObjectReferences(options.Namespace, normalized...) + normalized := normalizeResources(resources) + objRefs, err := util.GetObjectReferences(namespace, normalized...) if err != nil { return []corev1.ObjectReference{}, err } diff --git a/cmd/clusterctl/client/rollout_test.go b/cmd/clusterctl/client/rollout_test.go index d4c21926ec33..00a7a77d26e7 100644 --- a/cmd/clusterctl/client/rollout_test.go +++ b/cmd/clusterctl/client/rollout_test.go @@ -28,29 +28,65 @@ import ( "sigs.k8s.io/cluster-api/cmd/clusterctl/client/config" ) -type rolloutTest struct { - name string - fields fields - args args - wantErr bool -} -type fields struct { - client *fakeClient -} -type args struct { - options RolloutOptions +func fakeClientForRollout() *fakeClient { + core := config.NewProvider("cluster-api", "https://somewhere.com", clusterctlv1.CoreProviderType) + infra := config.NewProvider("infra", "https://somewhere.com", clusterctlv1.InfrastructureProviderType) + md1 := &clusterv1.MachineDeployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "MachineDeployment", + APIVersion: "cluster.x-k8s.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "md-1", + }, + } + md2 := &clusterv1.MachineDeployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "MachineDeployment", + APIVersion: "cluster.x-k8s.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "md-2", + }, + } + config1 := newFakeConfig(). + WithProvider(core). + WithProvider(infra) + + cluster1 := newFakeCluster(cluster.Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, config1). + WithProviderInventory(core.Name(), core.Type(), "v1.0.0", "cluster-api-system"). + WithProviderInventory(infra.Name(), infra.Type(), "v2.0.0", "infra-system"). + WithObjs(md1). + WithObjs(md2) + + client := newFakeClient(config1). + WithCluster(cluster1) + + return client } -// genericTestCases are test cases that can be passed to any of the rollout subcommands. -func genericTestCases() []rolloutTest { - return []rolloutTest{ +func Test_clusterctlClient_RolloutRestart(t *testing.T) { + type fields struct { + client *fakeClient + } + type args struct { + options RolloutRestartOptions + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ { name: "return an error if machinedeployment is not found", fields: fields{ client: fakeClientForRollout(), }, args: args{ - options: RolloutOptions{ + options: RolloutRestartOptions{ Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, Resources: []string{"machinedeployment/foo"}, Namespace: "default", @@ -64,7 +100,7 @@ func genericTestCases() []rolloutTest { client: fakeClientForRollout(), }, args: args{ - options: RolloutOptions{ + options: RolloutRestartOptions{ Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, Resources: []string{"machinedeployment/md-1", "machinedeployment/md-does-not-exist"}, Namespace: "default", @@ -78,7 +114,7 @@ func genericTestCases() []rolloutTest { client: fakeClientForRollout(), }, args: args{ - options: RolloutOptions{ + options: RolloutRestartOptions{ Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, Resources: []string{"foo/bar"}, Namespace: "default", @@ -92,65 +128,20 @@ func genericTestCases() []rolloutTest { client: fakeClientForRollout(), }, args: args{ - options: RolloutOptions{ + options: RolloutRestartOptions{ Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, Namespace: "default", }, }, wantErr: true, }, - } -} - -func fakeClientForRollout() *fakeClient { - core := config.NewProvider("cluster-api", "https://somewhere.com", clusterctlv1.CoreProviderType) - infra := config.NewProvider("infra", "https://somewhere.com", clusterctlv1.InfrastructureProviderType) - md1 := &clusterv1.MachineDeployment{ - TypeMeta: metav1.TypeMeta{ - Kind: "MachineDeployment", - APIVersion: "cluster.x-k8s.io/v1beta1", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "md-1", - }, - } - md2 := &clusterv1.MachineDeployment{ - TypeMeta: metav1.TypeMeta{ - Kind: "MachineDeployment", - APIVersion: "cluster.x-k8s.io/v1beta1", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "md-2", - }, - } - config1 := newFakeConfig(). - WithProvider(core). - WithProvider(infra) - - cluster1 := newFakeCluster(cluster.Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, config1). - WithProviderInventory(core.Name(), core.Type(), "v1.0.0", "cluster-api-system"). - WithProviderInventory(infra.Name(), infra.Type(), "v2.0.0", "infra-system"). - WithObjs(md1). - WithObjs(md2) - - client := newFakeClient(config1). - WithCluster(cluster1) - - return client -} - -func Test_clusterctlClient_RolloutRestart(t *testing.T) { - tests := genericTestCases() - additionalTests := []rolloutTest{ { name: "do not return error if machinedeployment found", fields: fields{ client: fakeClientForRollout(), }, args: args{ - options: RolloutOptions{ + options: RolloutRestartOptions{ Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, Resources: []string{"machinedeployment/md-1"}, Namespace: "default", @@ -164,7 +155,7 @@ func Test_clusterctlClient_RolloutRestart(t *testing.T) { client: fakeClientForRollout(), }, args: args{ - options: RolloutOptions{ + options: RolloutRestartOptions{ Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, Resources: []string{"machinedeployment/md-1", "machinedeployment/md-2"}, Namespace: "default", @@ -174,8 +165,6 @@ func Test_clusterctlClient_RolloutRestart(t *testing.T) { }, } - tests = append(tests, additionalTests...) - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) @@ -191,7 +180,74 @@ func Test_clusterctlClient_RolloutRestart(t *testing.T) { } func Test_clusterctlClient_RolloutPause(t *testing.T) { - tests := genericTestCases() + type fields struct { + client *fakeClient + } + type args struct { + options RolloutPauseOptions + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + name: "return an error if machinedeployment is not found", + fields: fields{ + client: fakeClientForRollout(), + }, + args: args{ + options: RolloutPauseOptions{ + Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, + Resources: []string{"machinedeployment/foo"}, + Namespace: "default", + }, + }, + wantErr: true, + }, + { + name: "return error if one of the machinedeployments is not found", + fields: fields{ + client: fakeClientForRollout(), + }, + args: args{ + options: RolloutPauseOptions{ + Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, + Resources: []string{"machinedeployment/md-1", "machinedeployment/md-does-not-exist"}, + Namespace: "default", + }, + }, + wantErr: true, + }, + { + name: "return error if unknown resource specified", + fields: fields{ + client: fakeClientForRollout(), + }, + args: args{ + options: RolloutPauseOptions{ + Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, + Resources: []string{"foo/bar"}, + Namespace: "default", + }, + }, + wantErr: true, + }, + { + name: "return error if no resource specified", + fields: fields{ + client: fakeClientForRollout(), + }, + args: args{ + options: RolloutPauseOptions{ + Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, + Namespace: "default", + }, + }, + wantErr: true, + }, + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) @@ -207,7 +263,74 @@ func Test_clusterctlClient_RolloutPause(t *testing.T) { } func Test_clusterctlClient_RolloutResume(t *testing.T) { - tests := genericTestCases() + type fields struct { + client *fakeClient + } + type args struct { + options RolloutResumeOptions + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + name: "return an error if machinedeployment is not found", + fields: fields{ + client: fakeClientForRollout(), + }, + args: args{ + options: RolloutResumeOptions{ + Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, + Resources: []string{"machinedeployment/foo"}, + Namespace: "default", + }, + }, + wantErr: true, + }, + { + name: "return error if one of the machinedeployments is not found", + fields: fields{ + client: fakeClientForRollout(), + }, + args: args{ + options: RolloutResumeOptions{ + Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, + Resources: []string{"machinedeployment/md-1", "machinedeployment/md-does-not-exist"}, + Namespace: "default", + }, + }, + wantErr: true, + }, + { + name: "return error if unknown resource specified", + fields: fields{ + client: fakeClientForRollout(), + }, + args: args{ + options: RolloutResumeOptions{ + Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, + Resources: []string{"foo/bar"}, + Namespace: "default", + }, + }, + wantErr: true, + }, + { + name: "return error if no resource specified", + fields: fields{ + client: fakeClientForRollout(), + }, + args: args{ + options: RolloutResumeOptions{ + Kubeconfig: Kubeconfig{Path: "kubeconfig", Context: "mgmt-context"}, + Namespace: "default", + }, + }, + wantErr: true, + }, + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) diff --git a/cmd/clusterctl/cmd/rollout/pause.go b/cmd/clusterctl/cmd/rollout/pause.go index 958fe494c41b..8e29ed200fec 100644 --- a/cmd/clusterctl/cmd/rollout/pause.go +++ b/cmd/clusterctl/cmd/rollout/pause.go @@ -77,7 +77,7 @@ func runPause(cfgFile string, args []string) error { return err } - return c.RolloutPause(client.RolloutOptions{ + return c.RolloutPause(client.RolloutPauseOptions{ Kubeconfig: client.Kubeconfig{Path: pauseOpt.kubeconfig, Context: pauseOpt.kubeconfigContext}, Namespace: pauseOpt.namespace, Resources: pauseOpt.resources, diff --git a/cmd/clusterctl/cmd/rollout/restart.go b/cmd/clusterctl/cmd/rollout/restart.go index e6d85cc4cdbf..7f46fac317fd 100644 --- a/cmd/clusterctl/cmd/rollout/restart.go +++ b/cmd/clusterctl/cmd/rollout/restart.go @@ -76,7 +76,7 @@ func runRestart(cfgFile string, _ *cobra.Command, args []string) error { return err } - return c.RolloutRestart(client.RolloutOptions{ + return c.RolloutRestart(client.RolloutRestartOptions{ Kubeconfig: client.Kubeconfig{Path: restartOpt.kubeconfig, Context: restartOpt.kubeconfigContext}, Namespace: restartOpt.namespace, Resources: restartOpt.resources, diff --git a/cmd/clusterctl/cmd/rollout/resume.go b/cmd/clusterctl/cmd/rollout/resume.go index ee5c803c908a..07dc4bc1fa5f 100644 --- a/cmd/clusterctl/cmd/rollout/resume.go +++ b/cmd/clusterctl/cmd/rollout/resume.go @@ -76,7 +76,7 @@ func runResume(cfgFile string, args []string) error { return err } - return c.RolloutResume(client.RolloutOptions{ + return c.RolloutResume(client.RolloutResumeOptions{ Kubeconfig: client.Kubeconfig{Path: resumeOpt.kubeconfig, Context: resumeOpt.kubeconfigContext}, Namespace: resumeOpt.namespace, Resources: resumeOpt.resources, diff --git a/cmd/clusterctl/cmd/rollout/undo.go b/cmd/clusterctl/cmd/rollout/undo.go index 7e40c4bb716f..cfa6603009b4 100644 --- a/cmd/clusterctl/cmd/rollout/undo.go +++ b/cmd/clusterctl/cmd/rollout/undo.go @@ -76,7 +76,7 @@ func runUndo(cfgFile string, args []string) error { return err } - return c.RolloutUndo(client.RolloutOptions{ + return c.RolloutUndo(client.RolloutUndoOptions{ Kubeconfig: client.Kubeconfig{Path: undoOpt.kubeconfig, Context: undoOpt.kubeconfigContext}, Namespace: undoOpt.namespace, Resources: undoOpt.resources, diff --git a/docs/book/src/developer/providers/v1.3-to-v1.4.md b/docs/book/src/developer/providers/v1.3-to-v1.4.md index c1125535f5de..bddbd7b8e299 100644 --- a/docs/book/src/developer/providers/v1.3-to-v1.4.md +++ b/docs/book/src/developer/providers/v1.3-to-v1.4.md @@ -36,6 +36,7 @@ maintainers of providers and consumers of our Go API. - `Backup(options BackupOptions) error` in the Client interface has been removed. - `Restore(options RestoreOptions) error` in the Client interface has been removed. +- `cmd/clusterctl/client.RolloutOptions` has been removed, `RolloutRestartOptions`, `RolloutPauseOptions` , `RolloutResumeOptions`, and `RolloutUndoOptions` have been added instead. ### Other