Skip to content

Commit 2b69699

Browse files
author
Marek Counts
committed
updated phase runner to enable custom arg validation
currently sub phases cannot have custom arg validation and container commands can have args. This removes phase container commands from taking args and enables custom args on the leaf phases
1 parent bf79c7c commit 2b69699

File tree

6 files changed

+165
-15
lines changed

6 files changed

+165
-15
lines changed

cmd/kubeadm/app/cmd/phases/join/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ go_library(
3737
"//staging/src/k8s.io/client-go/util/cert:go_default_library",
3838
"//vendor/github.com/lithammer/dedent:go_default_library",
3939
"//vendor/github.com/pkg/errors:go_default_library",
40+
"//vendor/github.com/spf13/cobra:go_default_library",
4041
"//vendor/k8s.io/klog:go_default_library",
4142
"//vendor/k8s.io/utils/exec:go_default_library",
4243
],

cmd/kubeadm/app/cmd/phases/join/controlplanejoin.go

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"fmt"
2121

2222
"github.com/pkg/errors"
23+
"github.com/spf13/cobra"
24+
2325
"k8s.io/kubernetes/cmd/kubeadm/app/cmd/options"
2426
"k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow"
2527
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
@@ -58,6 +60,7 @@ func NewControlPlaneJoinPhase() workflow.Phase {
5860
Short: "Join a machine as a control plane instance",
5961
InheritFlags: getControlPlaneJoinPhaseFlags("all"),
6062
RunAllSiblings: true,
63+
ArgsValidator: cobra.NoArgs,
6164
},
6265
newEtcdLocalSubphase(),
6366
newUpdateStatusSubphase(),
@@ -68,10 +71,11 @@ func NewControlPlaneJoinPhase() workflow.Phase {
6871

6972
func newEtcdLocalSubphase() workflow.Phase {
7073
return workflow.Phase{
71-
Name: "etcd",
72-
Short: "Add a new local etcd member",
73-
Run: runEtcdPhase,
74-
InheritFlags: getControlPlaneJoinPhaseFlags("etcd"),
74+
Name: "etcd",
75+
Short: "Add a new local etcd member",
76+
Run: runEtcdPhase,
77+
InheritFlags: getControlPlaneJoinPhaseFlags("etcd"),
78+
ArgsValidator: cobra.NoArgs,
7579
}
7680
}
7781

@@ -83,17 +87,19 @@ func newUpdateStatusSubphase() workflow.Phase {
8387
kubeadmconstants.ClusterStatusConfigMapKey,
8488
kubeadmconstants.KubeadmConfigConfigMap,
8589
),
86-
Run: runUpdateStatusPhase,
87-
InheritFlags: getControlPlaneJoinPhaseFlags("update-status"),
90+
Run: runUpdateStatusPhase,
91+
InheritFlags: getControlPlaneJoinPhaseFlags("update-status"),
92+
ArgsValidator: cobra.NoArgs,
8893
}
8994
}
9095

9196
func newMarkControlPlaneSubphase() workflow.Phase {
9297
return workflow.Phase{
93-
Name: "mark-control-plane",
94-
Short: "Mark a node as a control-plane",
95-
Run: runMarkControlPlanePhase,
96-
InheritFlags: getControlPlaneJoinPhaseFlags("mark-control-plane"),
98+
Name: "mark-control-plane",
99+
Short: "Mark a node as a control-plane",
100+
Run: runMarkControlPlanePhase,
101+
InheritFlags: getControlPlaneJoinPhaseFlags("mark-control-plane"),
102+
ArgsValidator: cobra.NoArgs,
97103
}
98104
}
99105

cmd/kubeadm/app/cmd/phases/join/controlplaneprepare.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"fmt"
2121

2222
"github.com/pkg/errors"
23+
"github.com/spf13/cobra"
2324

2425
clientset "k8s.io/client-go/kubernetes"
2526
"k8s.io/klog"
@@ -157,10 +158,11 @@ func newControlPlanePrepareKubeconfigSubphase() workflow.Phase {
157158

158159
func newControlPlanePrepareControlPlaneSubphase() workflow.Phase {
159160
return workflow.Phase{
160-
Name: "control-plane",
161-
Short: "Generate the manifests for the new control plane components",
162-
Run: runControlPlanePrepareControlPlaneSubphase, //NB. eventually in future we would like to break down this in sub phases for each component
163-
InheritFlags: getControlPlanePreparePhaseFlags("control-plane"),
161+
Name: "control-plane",
162+
Short: "Generate the manifests for the new control plane components",
163+
Run: runControlPlanePrepareControlPlaneSubphase, //NB. eventually in future we would like to break down this in sub phases for each component
164+
InheritFlags: getControlPlanePreparePhaseFlags("control-plane"),
165+
ArgsValidator: cobra.NoArgs,
164166
}
165167
}
166168

cmd/kubeadm/app/cmd/phases/workflow/phase.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ limitations under the License.
1616

1717
package workflow
1818

19-
import "github.com/spf13/pflag"
19+
import (
20+
"github.com/spf13/cobra"
21+
"github.com/spf13/pflag"
22+
)
2023

2124
// Phase provides an implementation of a workflow phase that allows
2225
// creation of new phases by simply instantiating a variable of this type.
@@ -71,6 +74,10 @@ type Phase struct {
7174
// Nb. if two or phases have the same local flags, please consider using local flags in the parent command
7275
// or additional flags defined in the phase runner.
7376
LocalFlags *pflag.FlagSet
77+
78+
// ArgsValidator defines the positional arg function to be used for validating args for this phase
79+
// If not set a phase will adopt the args of the top level command.
80+
ArgsValidator cobra.PositionalArgs
7481
}
7582

7683
// AppendPhase adds the given phase to the nested, ordered sequence of phases.

cmd/kubeadm/app/cmd/phases/workflow/runner.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,12 @@ func (e *Runner) BindToCommand(cmd *cobra.Command) {
372372
// if this phase has children (not a leaf) it doesn't accept any args
373373
if len(p.Phases) > 0 {
374374
phaseCmd.Args = cobra.NoArgs
375+
} else {
376+
if p.ArgsValidator == nil {
377+
phaseCmd.Args = cmd.Args
378+
} else {
379+
phaseCmd.Args = p.ArgsValidator
380+
}
375381
}
376382

377383
// adds the command to parent

cmd/kubeadm/app/cmd/phases/workflow/runner_test.go

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,134 @@ func phaseBuilder5(name string, flags *pflag.FlagSet) Phase {
297297
}
298298
}
299299

300+
type argTest struct {
301+
args cobra.PositionalArgs
302+
pass []string
303+
fail []string
304+
}
305+
306+
func phaseBuilder6(name string, args cobra.PositionalArgs, phases ...Phase) Phase {
307+
return Phase{
308+
Name: name,
309+
Short: fmt.Sprintf("long description for %s ...", name),
310+
Phases: phases,
311+
ArgsValidator: args,
312+
}
313+
}
314+
315+
// customArgs is a custom cobra.PositionArgs function
316+
func customArgs(cmd *cobra.Command, args []string) error {
317+
for _, a := range args {
318+
if a != "qux" {
319+
return fmt.Errorf("arg %s does not equal qux", a)
320+
}
321+
}
322+
return nil
323+
}
324+
325+
func TestBindToCommandArgRequirements(t *testing.T) {
326+
327+
// because cobra.ExactArgs(1) == cobra.ExactArgs(3), it is needed
328+
// to run test argument sets that both pass and fail to ensure the correct function was set.
329+
var usecases = []struct {
330+
name string
331+
runner Runner
332+
testCases map[string]argTest
333+
cmd *cobra.Command
334+
}{
335+
{
336+
name: "leaf command, no defined args, follow parent",
337+
runner: Runner{
338+
Phases: []Phase{phaseBuilder("foo")},
339+
},
340+
testCases: map[string]argTest{
341+
"phase foo": {
342+
pass: []string{"one", "two", "three"},
343+
fail: []string{"one", "two"},
344+
args: cobra.ExactArgs(3),
345+
},
346+
},
347+
cmd: &cobra.Command{
348+
Use: "init",
349+
Args: cobra.ExactArgs(3),
350+
},
351+
},
352+
{
353+
name: "container cmd expect none, custom arg check for leaf",
354+
runner: Runner{
355+
Phases: []Phase{phaseBuilder6("foo", cobra.NoArgs,
356+
phaseBuilder6("bar", cobra.ExactArgs(1)),
357+
phaseBuilder6("baz", customArgs),
358+
)},
359+
},
360+
testCases: map[string]argTest{
361+
"phase foo": {
362+
pass: []string{},
363+
fail: []string{"one"},
364+
args: cobra.NoArgs,
365+
},
366+
"phase foo bar": {
367+
pass: []string{"one"},
368+
fail: []string{"one", "two"},
369+
args: cobra.ExactArgs(1),
370+
},
371+
"phase foo baz": {
372+
pass: []string{"qux"},
373+
fail: []string{"one"},
374+
args: customArgs,
375+
},
376+
},
377+
cmd: &cobra.Command{
378+
Use: "init",
379+
Args: cobra.NoArgs,
380+
},
381+
},
382+
}
383+
384+
for _, rt := range usecases {
385+
t.Run(rt.name, func(t *testing.T) {
386+
387+
rt.runner.BindToCommand(rt.cmd)
388+
389+
// Checks that cmd gets a new phase subcommand
390+
phaseCmd := getCmd(rt.cmd, "phase")
391+
if phaseCmd == nil {
392+
t.Error("cmd didn't have phase subcommand\n")
393+
return
394+
}
395+
396+
for c, args := range rt.testCases {
397+
398+
cCmd := getCmd(rt.cmd, c)
399+
if cCmd == nil {
400+
t.Errorf("cmd didn't have %s subcommand\n", c)
401+
continue
402+
}
403+
404+
// Ensure it is the expected function
405+
if reflect.ValueOf(cCmd.Args).Pointer() != reflect.ValueOf(args.args).Pointer() {
406+
t.Error("The function poiners where not equal.")
407+
}
408+
409+
// Test passing argument set
410+
err := cCmd.Args(cCmd, args.pass)
411+
412+
if err != nil {
413+
t.Errorf("command %s should validate the args: %v\n %v", cCmd.Name(), args.pass, err)
414+
}
415+
416+
// Test failing argument set
417+
err = cCmd.Args(cCmd, args.fail)
418+
419+
if err == nil {
420+
t.Errorf("command %s should fail to validate the args: %v\n %v", cCmd.Name(), args.pass, err)
421+
}
422+
}
423+
424+
})
425+
}
426+
}
427+
300428
func TestBindToCommand(t *testing.T) {
301429

302430
var dummy string

0 commit comments

Comments
 (0)