From 3a22e56cd6fc51e99a295cbf55b00d70223e2ae5 Mon Sep 17 00:00:00 2001 From: Ken Sipe Date: Thu, 16 Apr 2020 20:03:55 -0500 Subject: [PATCH] Better Wait Handling on Installs (#1469) Signed-off-by: Ken Sipe --- pkg/kudoctl/cmd/plan.go | 27 --------------- pkg/kudoctl/cmd/plan/plan_wait.go | 56 ------------------------------- pkg/kudoctl/util/kudo/install.go | 4 +-- pkg/kudoctl/util/kudo/kudo.go | 12 +++++-- 4 files changed, 11 insertions(+), 88 deletions(-) delete mode 100644 pkg/kudoctl/cmd/plan/plan_wait.go diff --git a/pkg/kudoctl/cmd/plan.go b/pkg/kudoctl/cmd/plan.go index bfe882923..178f140bd 100644 --- a/pkg/kudoctl/cmd/plan.go +++ b/pkg/kudoctl/cmd/plan.go @@ -16,9 +16,6 @@ const ( ` planStatusExample = ` # View plan status kubectl kudo plan status --instance= -` - planWaitExample = ` # Wait on the current plan status to finish - kubectl kudo plan wait --instance= ` planTriggerExample = ` # Trigger an instance plan kubectl kudo plan trigger --instance= @@ -35,7 +32,6 @@ func newPlanCmd(out io.Writer) *cobra.Command { cmd.AddCommand(NewPlanHistoryCmd()) cmd.AddCommand(NewPlanStatusCmd(out)) - cmd.AddCommand(NewPlanWaitCmd(out)) cmd.AddCommand(NewPlanTriggerCmd()) return cmd @@ -81,29 +77,6 @@ func NewPlanStatusCmd(out io.Writer) *cobra.Command { return cmd } -//NewPlanWaitCmd waits on the status of an instance to complete -func NewPlanWaitCmd(out io.Writer) *cobra.Command { - options := &plan.WaitOptions{Out: out, WaitTime: 300} - cmd := &cobra.Command{ - Use: "wait", - Short: "Waits on a plan to finish for a particular instance.", - Example: planWaitExample, - RunE: func(cmd *cobra.Command, args []string) error { - return plan.Wait(options, &Settings) - }, - } - - cmd.Flags().StringVar(&options.Instance, "instance", "", "The instance name available from 'kubectl get instances'") - cmd.Flags().Int64Var(&options.WaitTime, "wait-time", 300, "Specify the max wait time in seconds for CLI to wait for the current plan to complete (default \"300\")") - - if err := cmd.MarkFlagRequired("instance"); err != nil { - clog.Printf("Please choose the instance with '--instance=': %v", err) - os.Exit(1) - } - - return cmd -} - // NewPlanTriggerCmd creates a command that triggers a specific plan for an instance. func NewPlanTriggerCmd() *cobra.Command { options := &plan.TriggerOptions{} diff --git a/pkg/kudoctl/cmd/plan/plan_wait.go b/pkg/kudoctl/cmd/plan/plan_wait.go deleted file mode 100644 index 40478cd3a..000000000 --- a/pkg/kudoctl/cmd/plan/plan_wait.go +++ /dev/null @@ -1,56 +0,0 @@ -package plan - -import ( - "errors" - "fmt" - "io" - "time" - - pollwait "k8s.io/apimachinery/pkg/util/wait" - - "github.com/kudobuilder/kudo/pkg/kudoctl/env" - "github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo" -) - -// Options are the configurable options for plans -type WaitOptions struct { - Out io.Writer - Instance string - WaitTime int64 -} - -// Status runs the plan status command -func Wait(options *WaitOptions, settings *env.Settings) error { - kc, err := env.GetClient(settings) - if err != nil { - return err - } - //return status(kc, options, settings.Namespace) - return wait(kc, options, settings.Namespace) -} - -func wait(kc *kudo.Client, options *WaitOptions, ns string) error { - instance, err := kc.GetInstance(options.Instance, ns) - if err != nil { - return err - } - if instance == nil { - return fmt.Errorf("instance %s/%s does not exist", ns, options.Instance) - } - - planStatus := instance.GetLastExecutedPlanStatus() - if planStatus == nil { - return fmt.Errorf("instance %s/%s does not have an active plan", ns, options.Instance) - } - - fmt.Fprintf(options.Out, "waiting on instance %s/%s with plan %q\n", ns, options.Instance, planStatus.Name) - err = kc.WaitForInstance(options.Instance, ns, nil, time.Duration(options.WaitTime)*time.Second) - if errors.Is(err, pollwait.ErrWaitTimeout) { - _, _ = fmt.Fprintf(options.Out, "timeout waiting for instance %s/%s on plan %q\n", ns, options.Instance, planStatus.Name) - } - if err != nil { - return err - } - _, _ = fmt.Fprintf(options.Out, "instance %s/%s plan %q finished\n", ns, options.Instance, planStatus.Name) - return nil -} diff --git a/pkg/kudoctl/util/kudo/install.go b/pkg/kudoctl/util/kudo/install.go index 444533211..a49e35cf0 100644 --- a/pkg/kudoctl/util/kudo/install.go +++ b/pkg/kudoctl/util/kudo/install.go @@ -58,12 +58,12 @@ func InstallPackage(kc *Client, resources *packages.Resources, skipInstance bool } instanceName = resources.Instance.ObjectMeta.Name - instanceExists, err := kc.InstanceExistsInCluster(operatorName, namespace, resources.OperatorVersion.Spec.Version, instanceName) + instance, err := kc.GetInstance(instanceName, namespace) if err != nil { return fmt.Errorf("failed to verify existing instance: %v", err) } - if !instanceExists { + if instance == nil { if _, err := kc.InstallInstanceObjToCluster(resources.Instance, namespace); err != nil { return fmt.Errorf("failed to install instance %s: %v", instanceName, err) } diff --git a/pkg/kudoctl/util/kudo/kudo.go b/pkg/kudoctl/util/kudo/kudo.go index 59aa10899..859376a62 100644 --- a/pkg/kudoctl/util/kudo/kudo.go +++ b/pkg/kudoctl/util/kudo/kudo.go @@ -207,9 +207,15 @@ func (c *Client) UpdateInstance(instanceName, namespace string, operatorVersion return err } -// WaitForInstance waits for instance to be "finished". oldInstance is nil if there is no oldInstance. oldInstance -// should be provided if there was an update or upgrade. The wait will then initially wait for the "new" plan to activate -// then return when completed. The error is either an error in working with kubernetes or a wait.ErrWaitTimeout +// WaitForInstance waits for instance to be "complete". +// It uses controller-runtime `wait.PollImmediate`, the function passed to it returns done==false if it isn't done. +// For a situation where there is no previous state (like install), the "lastPlanStatus" will be nil until the manager +// sets it, then it's state will be watched (see InInstanceDone for more detail) +// For a situation where there is previous state (like update, upgrade, plan trigger) than it is important AND required +// that the "oldInstance" be provided. Without it, it is possible for this function to be "racy" and "flaky" meaning the +// "current" status could be the old "done" status or the new status... it's hard to know. If the oldInstance is provided +// the wait will then initially wait for the "new" plan to activate then return when completed. +// The error is either an error in working with kubernetes or a wait.ErrWaitTimeout func (c *Client) WaitForInstance(name, namespace string, oldInstance *v1beta1.Instance, timeout time.Duration) error { // polling interval 1 sec interval := 1 * time.Second