From 501db1587947c8d59fec151b0d88f9b226ae3a3c Mon Sep 17 00:00:00 2001 From: Johannes Liebermann Date: Mon, 10 Aug 2020 20:33:57 +0200 Subject: [PATCH] Refactor Terraform executor - Define a new type terraform.ExecutionStep which allows describing arbitrary Terraform operations declaratively. - Allow passing one or more ExecutionSteps to the Terraform executor. This allows making pkg/platform generic as now every platform implementation can define its steps for deploying infrastructure in a declarative way, thus making the code which interacts with Terraform generic. Thanks to the PreExecutionHook concept, the same is true even when arbitrary logic needs to be executed before some Terraform operation: A platform implementation can include a callback function in the PreExecutionHook field of an ExecutionStep and the generic Terraform code will invoke it at the right time. --- pkg/dns/dns.go | 52 ++++++++++--------- pkg/platform/packet/packet.go | 58 +++++++++++----------- pkg/terraform/executor.go | 91 ++++++++++++++++++++++++++-------- pkg/terraform/executor_test.go | 2 +- 4 files changed, 128 insertions(+), 75 deletions(-) diff --git a/pkg/dns/dns.go b/pkg/dns/dns.go index 886031915..5692a0c16 100644 --- a/pkg/dns/dns.go +++ b/pkg/dns/dns.go @@ -61,37 +61,43 @@ func (c *Config) Validate() error { return fmt.Errorf("invalid DNS provider %q", c.Provider) } -// AskToConfigure reads the required DNS entries from a Terraform output, -// asks the user to configure them and checks if the configuration is correct. -func (c *Config) AskToConfigure(ex *terraform.Executor) error { - dnsEntries, err := readDNSEntries(ex) - if err != nil { - return err - } +// ManualConfigPrompt returns a function which prompts the user to configure DNS entries manually +// and verifies the entries were created successfully. +func ManualConfigPrompt(c *Config) func(*terraform.Executor) error { + return func(ex *terraform.Executor) error { + dnsEntries, err := readDNSEntries(ex) + if err != nil { + return err + } - fmt.Printf("Please configure the following DNS entries at the DNS provider which hosts %q:\n", c.Zone) - prettyPrintDNSEntries(dnsEntries) + fmt.Printf("Please configure the following DNS entries at the DNS provider which hosts %q:\n", c.Zone) + prettyPrintDNSEntries(dnsEntries) - for { - fmt.Printf("Press Enter to check the entries or type \"skip\" to continue the installation: ") + for { + fmt.Printf("Press Enter to check the entries or type \"skip\" to continue the installation: ") - var input string - fmt.Scanln(&input) + var input string - if input == "skip" { - break - } else if input != "" { - continue - } + _, err := fmt.Scanln(&input) + if err != nil { + return fmt.Errorf("reading user input: %w", err) + } - if checkDNSEntries(dnsEntries) { - break + if input == "skip" { + break + } else if input != "" { + continue + } + + if checkDNSEntries(dnsEntries) { + break + } + + fmt.Println("Entries are not correctly configured, please verify.") } - fmt.Println("Entries are not correctly configured, please verify.") + return nil } - - return nil } func readDNSEntries(ex *terraform.Executor) ([]dnsEntry, error) { diff --git a/pkg/platform/packet/packet.go b/pkg/platform/packet/packet.go index fa14b1be2..321ed3a3e 100644 --- a/pkg/platform/packet/packet.go +++ b/pkg/platform/packet/packet.go @@ -263,35 +263,35 @@ func (c *config) terraformSmartApply(ex *terraform.Executor, dc dns.Config) erro return ex.Apply() } - arguments := []string{"apply", "-auto-approve"} - - // Create controllers. We need the controllers' IP addresses before we can - // apply the 'dns' module. - arguments = append(arguments, fmt.Sprintf("-target=module.packet-%s.packet_device.controllers", c.ClusterName)) - if err := ex.Execute(arguments...); err != nil { - return errors.Wrap(err, "creating controllers") - } - - // Apply 'dns' module. - arguments = append(arguments, "-target=module.dns") - if err := ex.Execute(arguments...); err != nil { - return errors.Wrap(err, "applying 'dns' module") - } - - // Run `terraform refresh`. This is required in order to make the outputs from the previous - // apply operations available. - // TODO: Likely caused by https://github.com/hashicorp/terraform/issues/23158. - if err := ex.Execute("refresh"); err != nil { - return errors.Wrap(err, "refreshing") - } - - // Prompt user to configure DNS. - if err := dc.AskToConfigure(ex); err != nil { - return errors.Wrap(err, "prompting for manual DNS configuration") - } - - // Finish deployment. - return ex.Apply() + steps := []terraform.ExecutionStep{ + // We need the controllers' IP addresses before we can apply the 'dns' module. + { + Description: "create controllers", + Args: []string{ + "apply", + "-auto-approve", + fmt.Sprintf("-target=module.packet-%s.packet_device.controllers", c.ClusterName), + }, + }, + { + Description: "create DNS records", + Args: []string{"apply", "-auto-approve", "-target=module.dns"}, + }, + // Run `terraform refresh`. This is required in order to make the outputs from the previous + // apply operations available. + // TODO: Likely caused by https://github.com/hashicorp/terraform/issues/23158. + { + Description: "refresh Terraform state", + Args: []string{"refresh"}, + }, + { + Description: "complete infrastructure creation", + Args: []string{"apply", "-auto-approve"}, + PreExecutionHook: dns.ManualConfigPrompt(&c.DNS), + }, + } + + return ex.Execute(steps...) } // terraformAddDeps adds explicit dependencies to cluster nodes so nodes diff --git a/pkg/terraform/executor.go b/pkg/terraform/executor.go index 3b5591139..918066885 100644 --- a/pkg/terraform/executor.go +++ b/pkg/terraform/executor.go @@ -70,6 +70,30 @@ const ( ExecutionStatusFailure ExecutionStatus = "Failure" ) +// ExecutionStep represents a single Terraform operation. +type ExecutionStep struct { + // A short string describing the step in a way that is meaningful to the user. The string + // should begin with a lowercase letter, be in the imperative tense and have no period at the + // end. + // + // Examples: + // - "create DNS resources" + // - "deploy virtual machines" + Description string + // A list of arguments to be passed to the `terraform` command. Note that for "apply" commands + // the "-auto-approve" argument should always be included to avoid halting the Terraform + // execution with interactive prompts. + // + // Examples: + // - []string{"apply", "-target=module.foo", "-auto-approve"} + // - []string{"refresh"} + // - []string{"apply", "-auto-approve"} + Args []string + // A function which should be run prior to executing the Terraform command. If specified and + // the function returns an error, execution is halted. + PreExecutionHook func(*Executor) error +} + // Executor enables calling Terraform from Go, across platforms, with any // additional providers/provisioners that the currently executing binary // exposes. @@ -126,25 +150,28 @@ func NewExecutor(conf Config) (*Executor, error) { return ex, nil } -// Init() is a wrapper function that runs -// `terraform init`. +// Init is a wrapper function that runs `terraform init`. func (ex *Executor) Init() error { - ex.logger.Println("Initializing Terraform working directory") - return ex.Execute("init") + return ex.Execute(ExecutionStep{ + Description: "initialize Terraform", + Args: []string{"init"}, + }) } -// Apply() is a wrapper function that runs -// `terraform apply -auto-approve`. +// Apply is a wrapper function that runs `terraform apply -auto-approve`. func (ex *Executor) Apply() error { - ex.logger.Println("Applying Terraform configuration. This creates infrastructure so it might take a long time...") - return ex.Execute("apply", "-auto-approve") + return ex.Execute(ExecutionStep{ + Description: "create infrastructure", + Args: []string{"apply", "-auto-approve"}, + }) } -// Destroy() is a wrapper function that runs -// `terraform destroy -auto-approve`. +// Destroy is a wrapper function that runs `terraform destroy -auto-approve`. func (ex *Executor) Destroy() error { - ex.logger.Println("Destroying Terraform-managed infrastructure") - return ex.Execute("destroy", "-auto-approve") + return ex.Execute(ExecutionStep{ + Description: "destroy infrastructure", + Args: []string{"destroy", "-auto-approve"}, + }) } // tailFile will indefinitely tail logs from the given file path, until @@ -176,14 +203,27 @@ func tailFile(path string, done chan struct{}, wg *sync.WaitGroup) { wg.Done() } -// Execute runs the given command and arguments against Terraform, and returns -// any errors that occur during the execution. -// -// An error is returned if the Terraform binary could not be found, or if the -// Terraform call itself failed, in which case, details can be found in the -// output. -func (ex *Executor) Execute(args ...string) error { - return ex.execute(ex.verbose, args...) +// Execute accepts one or more ExecutionSteps and executes them sequentially in the order they were +// provided. If a step has a PreExecutionHook defined, the hook is run prior to executing the step. +// If any error is encountered, the error is returned and the execution is halted. +func (ex *Executor) Execute(steps ...ExecutionStep) error { + for _, s := range steps { + if s.PreExecutionHook != nil { + ex.logger.Printf("Running pre-execution hook for step %q", s.Description) + + if err := s.PreExecutionHook(ex); err != nil { + return fmt.Errorf("pre-execution hook failed: %w", err) + } + } + + ex.logger.Printf("Executing step %q", s.Description) + + if err := ex.execute(ex.verbose, s.Args...); err != nil { + return err + } + } + + return nil } func (ex *Executor) executeVerbose(args ...string) error { @@ -193,7 +233,10 @@ func (ex *Executor) executeVerbose(args ...string) error { func (ex *Executor) execute(verbose bool, args ...string) error { pid, done, err := ex.ExecuteAsync(args...) if err != nil { - return fmt.Errorf("failed executing Terraform command with arguments '%s' in directory %s: %w", strings.Join(args, " "), ex.WorkingDirectory(), err) + return fmt.Errorf( + "executing Terraform with arguments '%s' in directory %s: %w", + strings.Join(args, " "), ex.WorkingDirectory(), err, + ) } var wg sync.WaitGroup @@ -351,7 +394,11 @@ func (ex *Executor) ExecuteSync(args ...string) ([]byte, error) { func (ex *Executor) Plan() error { ex.logger.Println("Generating Terraform execution plan") - if err := ex.Execute("refresh"); err != nil { + s := ExecutionStep{ + Description: "refresh Terraform state", + Args: []string{"refresh"}, + } + if err := ex.Execute(s); err != nil { return err } diff --git a/pkg/terraform/executor_test.go b/pkg/terraform/executor_test.go index ef1879fa2..1baad2c66 100644 --- a/pkg/terraform/executor_test.go +++ b/pkg/terraform/executor_test.go @@ -33,7 +33,7 @@ func executor(t *testing.T) *Executor { func TestExecuteCheckErrors(t *testing.T) { ex := executor(t) - if err := ex.Execute("apply"); err == nil { + if err := ex.Apply(); err == nil { t.Fatalf("Applying on empty directory should fail") } }