From 583fda5dfe5caf661910077d852786036944cf97 Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Tue, 29 Jun 2021 01:22:41 -0500 Subject: [PATCH] Add possibility to specify 0 timeout via CLI options Currently zero timeout won't be applied if specified via CLI options of commands plan run and plan run. This patch fixes it. Change-Id: Icad06ae8c71d78dc81821e2c5b0adb486d547fda Signed-off-by: Ruslan Aliev Relates-To: #566 Closes: #566 --- cmd/phase/run.go | 23 +++++++++++++------ cmd/plan/run.go | 23 +++++++++++++------ pkg/phase/command.go | 16 ++++++------- pkg/phase/command_test.go | 10 ++++---- pkg/phase/executors/baremetal_manager.go | 4 ++-- pkg/phase/executors/baremetal_manager_test.go | 4 +++- pkg/phase/executors/k8s_applier.go | 4 ++-- pkg/phase/ifc/executor.go | 6 ++--- 8 files changed, 53 insertions(+), 37 deletions(-) diff --git a/cmd/phase/run.go b/cmd/phase/run.go index eef974e71..0e7056c7a 100644 --- a/cmd/phase/run.go +++ b/cmd/phase/run.go @@ -16,6 +16,7 @@ package phase import ( "github.com/spf13/cobra" + "github.com/spf13/pflag" "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/phase" @@ -36,10 +37,8 @@ Run initinfra phase // NewRunCommand creates a command to run specific phase func NewRunCommand(cfgFactory config.Factory) *cobra.Command { - p := &phase.RunCommand{ - Factory: cfgFactory, - Options: phase.RunFlags{}, - } + p := &phase.RunCommand{Factory: cfgFactory} + f := &phase.RunFlags{} runCmd := &cobra.Command{ Use: "run PHASE_NAME", @@ -48,12 +47,22 @@ func NewRunCommand(cfgFactory config.Factory) *cobra.Command { Args: cobra.ExactArgs(1), Example: runExample, RunE: func(cmd *cobra.Command, args []string) error { - p.Options.PhaseID.Name = args[0] + p.PhaseID.Name = args[0] + // Go through all the flags that have been set + fn := func(flag *pflag.Flag) { + switch flag.Name { + case "dry-run": + p.Options.DryRun = f.DryRun + case "wait-timeout": + p.Options.Timeout = &f.Timeout + } + } + cmd.Flags().Visit(fn) return p.RunE() }, } flags := runCmd.Flags() - flags.BoolVar(&p.Options.DryRun, "dry-run", false, "simulate phase execution") - flags.DurationVar(&p.Options.Timeout, "wait-timeout", 0, "wait timeout") + flags.BoolVar(&f.DryRun, "dry-run", false, "simulate phase execution") + flags.DurationVar(&f.Timeout, "wait-timeout", 0, "wait timeout") return runCmd } diff --git a/cmd/plan/run.go b/cmd/plan/run.go index c0e027596..21c7c02c0 100644 --- a/cmd/plan/run.go +++ b/cmd/plan/run.go @@ -16,6 +16,7 @@ package plan import ( "github.com/spf13/cobra" + "github.com/spf13/pflag" "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/phase" @@ -37,10 +38,9 @@ Perform a dry run of a plan // NewRunCommand creates a command which execute a particular phase plan func NewRunCommand(cfgFactory config.Factory) *cobra.Command { - r := &phase.PlanRunCommand{ - Factory: cfgFactory, - Options: phase.PlanRunFlags{}, - } + r := &phase.PlanRunCommand{Factory: cfgFactory} + f := &phase.RunFlags{} + runCmd := &cobra.Command{ Use: "run PLAN_NAME", Short: "Airshipctl command to run plan", @@ -48,13 +48,22 @@ func NewRunCommand(cfgFactory config.Factory) *cobra.Command { Example: runExample, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - r.Options.PlanID.Name = args[0] + r.PlanID.Name = args[0] + fn := func(flag *pflag.Flag) { + switch flag.Name { + case "dry-run": + r.Options.DryRun = f.DryRun + case "wait-timeout": + r.Options.Timeout = &f.Timeout + } + } + cmd.Flags().Visit(fn) return r.RunE() }, } flags := runCmd.Flags() - flags.BoolVar(&r.Options.DryRun, "dry-run", false, "simulate phase execution") - flags.DurationVar(&r.Options.Timeout, "wait-timeout", 0, "wait timeout") + flags.BoolVar(&f.DryRun, "dry-run", false, "simulate phase execution") + flags.DurationVar(&f.Timeout, "wait-timeout", 0, "wait timeout") return runCmd } diff --git a/pkg/phase/command.go b/pkg/phase/command.go index e884d3ef9..f3ffe598c 100644 --- a/pkg/phase/command.go +++ b/pkg/phase/command.go @@ -45,12 +45,12 @@ type GenericRunFlags struct { // RunFlags options for phase run command type RunFlags struct { GenericRunFlags - PhaseID ifc.ID } // RunCommand phase run command type RunCommand struct { - Options RunFlags + PhaseID ifc.ID + Options ifc.RunOptions Factory config.Factory } @@ -68,11 +68,11 @@ func (c *RunCommand) RunE() error { client := NewClient(helper) - phase, err := client.PhaseByID(c.Options.PhaseID) + phase, err := client.PhaseByID(c.PhaseID) if err != nil { return err } - return phase.Run(ifc.RunOptions{DryRun: c.Options.DryRun, Timeout: c.Options.Timeout}) + return phase.Run(c.Options) } // ListCommand phase list command @@ -205,12 +205,12 @@ func (c *PlanListCommand) RunE() error { // PlanRunFlags options for phase run command type PlanRunFlags struct { GenericRunFlags - PlanID ifc.ID } // PlanRunCommand phase run command type PlanRunCommand struct { - Options PlanRunFlags + PlanID ifc.ID + Options ifc.RunOptions Factory config.Factory } @@ -228,11 +228,11 @@ func (c *PlanRunCommand) RunE() error { client := NewClient(helper) - plan, err := client.PlanByID(c.Options.PlanID) + plan, err := client.PlanByID(c.PlanID) if err != nil { return err } - return plan.Run(ifc.RunOptions{DryRun: c.Options.DryRun, Timeout: c.Options.Timeout}) + return plan.Run(c.Options) } // ClusterListCommand options for cluster list command diff --git a/pkg/phase/command_test.go b/pkg/phase/command_test.go index 5a35a2311..dcdeacaf2 100644 --- a/pkg/phase/command_test.go +++ b/pkg/phase/command_test.go @@ -43,7 +43,7 @@ func TestRunCommand(t *testing.T) { tests := []struct { name string errContains string - runFlags phase.RunFlags + runFlags ifc.RunOptions factory config.Factory }{ { @@ -492,13 +492,11 @@ func TestPlanRunCommand(t *testing.T) { tt := tc t.Run(tt.name, func(t *testing.T) { cmd := phase.PlanRunCommand{ - Options: phase.PlanRunFlags{ - GenericRunFlags: phase.GenericRunFlags{ - DryRun: true, - }, - PlanID: tt.planID, + Options: ifc.RunOptions{ + DryRun: true, }, Factory: tt.factory, + PlanID: tt.planID, } err := cmd.RunE() if tt.expectedErr != "" { diff --git a/pkg/phase/executors/baremetal_manager.go b/pkg/phase/executors/baremetal_manager.go index f7c624818..0d07c60d9 100644 --- a/pkg/phase/executors/baremetal_manager.go +++ b/pkg/phase/executors/baremetal_manager.go @@ -125,8 +125,8 @@ func toCommandOptions(i inventoryifc.Inventory, spec v1alpha1.BaremetalManagerSpec, opts ifc.RunOptions) *inventory.CommandOptions { timeout := time.Duration(spec.Timeout) * time.Second - if opts.Timeout != 0 { - timeout = opts.Timeout + if opts.Timeout != nil { + timeout = *opts.Timeout } return &inventory.CommandOptions{ diff --git a/pkg/phase/executors/baremetal_manager_test.go b/pkg/phase/executors/baremetal_manager_test.go index 2f31da9c1..aff7783fa 100644 --- a/pkg/phase/executors/baremetal_manager_test.go +++ b/pkg/phase/executors/baremetal_manager_test.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -89,6 +90,7 @@ func TestNewBMHExecutor(t *testing.T) { } func TestBMHExecutorRun(t *testing.T) { + timeout := time.Duration(40) tests := []struct { name string expectedErr string @@ -102,7 +104,7 @@ func TestBMHExecutorRun(t *testing.T) { runOptions: ifc.RunOptions{ DryRun: true, // any value but zero - Timeout: 40, + Timeout: &timeout, }, execDoc: executorDoc(t, fmt.Sprintf(bmhExecutorTemplate, "unknown", "")), inventory: testBaremetalInventory(), diff --git a/pkg/phase/executors/k8s_applier.go b/pkg/phase/executors/k8s_applier.go index 5b2d915a2..e0fc1b9de 100644 --- a/pkg/phase/executors/k8s_applier.go +++ b/pkg/phase/executors/k8s_applier.go @@ -92,8 +92,8 @@ func (e *KubeApplierExecutor) Run(ch chan events.Event, runOpts ifc.RunOptions) dryRunStrategy = common.DryRunClient } timeout := time.Second * time.Duration(e.apiObject.Config.WaitOptions.Timeout) - if int64(runOpts.Timeout/time.Second) != 0 { - timeout = runOpts.Timeout + if runOpts.Timeout != nil { + timeout = *runOpts.Timeout } log.Debugf("WaitTimeout: %v", timeout) diff --git a/pkg/phase/ifc/executor.go b/pkg/phase/ifc/executor.go index 0288d9edb..9f515bf03 100644 --- a/pkg/phase/ifc/executor.go +++ b/pkg/phase/ifc/executor.go @@ -38,10 +38,8 @@ type ExecutorStatus struct{} // RunOptions holds options for run method type RunOptions struct { - DryRun bool - Progress bool - - Timeout time.Duration + DryRun bool + Timeout *time.Duration } // RenderOptions holds options for render method