[HPR-864] Error on wrapped startup script errors

builder/googlecompute/builder_acc_test.go

@@ -56,3 +56,43 @@ func TestAccBuilder_DefaultTokenSource(t *testing.T) {
 	}
 	acctest.TestPlugin(t, testCase)
 }
+
+func TestAccBuilder_WrappedStartupScriptSuccess(t *testing.T) {
+	tmpl, err := testDataFs.ReadFile("testdata/wrapped-startup-scripts/successful.pkr.hcl")
+	if err != nil {
+		t.Fatalf("failed to read testdata file %s", err)
+	}
+	testCase := &acctest.PluginTestCase{
+		Name:     "googlecompute-packer-good-startup-metadata",
+		Template: string(tmpl),
+		Check: func(buildCommand *exec.Cmd, logfile string) error {
+			if buildCommand.ProcessState != nil {
+				if buildCommand.ProcessState.ExitCode() != 0 {
+					return fmt.Errorf("Bad exit code. Logfile: %s", logfile)
+				}
+			}
+			return nil
+		},
+	}
+	acctest.TestPlugin(t, testCase)
+}
+
+func TestAccBuilder_WrappedStartupScriptError(t *testing.T) {
+	tmpl, err := testDataFs.ReadFile("testdata/wrapped-startup-scripts/errored.pkr.hcl")
+	if err != nil {
+		t.Fatalf("failed to read testdata file %s", err)
+	}
+	testCase := &acctest.PluginTestCase{
+		Name:     "googlecompute-packer-bad-startup-metadata",
+		Template: string(tmpl),
+		Check: func(buildCommand *exec.Cmd, logfile string) error {
+			if buildCommand.ProcessState != nil {
+				if buildCommand.ProcessState.ExitCode() != 1 {
+					return fmt.Errorf("Bad exit code. Logfile: %s", logfile)
+				}
+			}
+			return nil
+		},
+	}
+	acctest.TestPlugin(t, testCase)
+}

builder/googlecompute/startup.go

@@ -28,7 +28,7 @@ SetMetadata () {
   gcloud compute instances add-metadata ${HOSTNAME} --metadata ${1}=${2} --zone ${ZONE}
 }
 
-STARTUPSCRIPT=$(GetMetadata attributes/%s)
+STARTUPSCRIPT=$(GetMetadata attributes/%[1]s)
 STARTUPSCRIPTPATH=/packer-wrapped-startup-script
 if [ -f "/var/log/startupscript.log" ]; then
   STARTUPSCRIPTLOGPATH=/var/log/startupscript.log
@@ -52,9 +52,15 @@ if [[ ! -z $STARTUPSCRIPT ]]; then
   rm ${STARTUPSCRIPTPATH}
 fi
 
-echo "Packer startup script done."
-SetMetadata %s %s
+if [ $RETVAL -ne 0  ]; then
+  echo "Packer startup script exited with exit code: ${RETVAL}"
+  SetMetadata %[2]s %[4]s
+else
+  echo "Packer startup script done."
+  SetMetadata %[2]s %[3]s
+fi
+
 exit $RETVAL
-`, StartupWrappedScriptKey, StartupScriptStatusKey, StartupScriptStatusDone)
+`, StartupWrappedScriptKey, StartupScriptStatusKey, StartupScriptStatusDone, StartupScriptStatusError)
 
 var StartupScriptWindows string = ""

builder/googlecompute/step_wait_startup_script.go

@@ -11,6 +11,12 @@ import (
 	"github.com/hashicorp/packer-plugin-sdk/retry"
 )
 
+// ErrStartupScriptMetadata means that the user provided startup script resulted in
+// setting the set-startup-script metadata status to error.
+var ErrStartupScriptMetadata = errors.New("Startup script exited with error.")
+
+// StepWaitStartupScript is a trivial implementation of a Packer multistep
+// It can be used for tracking the set-startup-script metadata status.
 type StepWaitStartupScript int
 
 // Run reads the instance metadata and looks for the log entry
@@ -28,7 +34,10 @@ func (s *StepWaitStartupScript) Run(ctx context.Context, state multistep.StateBa
 	ui.Say("Waiting for any running startup script to finish...")
 	// Keep checking the serial port output to see if the startup script is done.
 	err := retry.Config{
-		ShouldRetry: func(error) bool {
+		ShouldRetry: func(err error) bool {
+			if errors.Is(err, ErrStartupScriptMetadata) {
+				return false
+			}
 			return true
 		},
 		RetryDelay: (&retry.Backoff{InitialBackoff: 10 * time.Second, MaxBackoff: 60 * time.Second, Multiplier: 2}).Linear,
@@ -44,8 +53,8 @@ func (s *StepWaitStartupScript) Run(ctx context.Context, state multistep.StateBa
 
 		switch status {
 		case StartupScriptStatusError:
-			ui.Message("Startup script in error. Waiting...")
-			return errors.New("Startup script error.")
+			ui.Message("Startup script in error. Exiting...")
+			return ErrStartupScriptMetadata
 
 		case StartupScriptStatusDone:
 			ui.Message("Startup script successfully finished.")

builder/googlecompute/step_wait_startup_script_test.go

@@ -34,33 +34,45 @@ func TestStepWaitStartupScript(t *testing.T) {
 
 func TestStepWaitStartupScript_withWrapStartupScript(t *testing.T) {
 	tt := []struct {
-		WrapStartup                config.Trilean
-		Result, Zone, MetadataName string
+		Name                               string
+		WrapStartup                        config.Trilean
+		MetadataResult, Zone, MetadataName string
+		StepResult                         multistep.StepAction //Zero value for StepAction is StepContinue; this is expected for all passing test cases.
 	}{
-		{WrapStartup: config.TriTrue, Result: StartupScriptStatusDone, Zone: "test-zone", MetadataName: "test-instance-name"},
-		{WrapStartup: config.TriFalse},
+		{Name: "no- wrapped startup script", WrapStartup: config.TriFalse},
+		{Name: "good - wrapped startup script", WrapStartup: config.TriTrue, MetadataResult: StartupScriptStatusDone, Zone: "test-zone", MetadataName: "test-instance-name"},
+		{
+			Name:           "failed - wrapped startup script",
+			WrapStartup:    config.TriTrue,
+			MetadataResult: StartupScriptStatusError,
+			Zone:           "test-zone",
+			MetadataName:   "failed-instance-name",
+			StepResult:     multistep.ActionHalt,
+		},
 	}
 
 	for _, tc := range tt {
 		tc := tc
-		state := testState(t)
-		step := new(StepWaitStartupScript)
-		c := state.Get("config").(*Config)
-		d := state.Get("driver").(*DriverMock)
+		t.Run(tc.Name, func(t *testing.T) {
+			state := testState(t)
+			step := new(StepWaitStartupScript)
+			c := state.Get("config").(*Config)
+			d := state.Get("driver").(*DriverMock)
 
-		c.StartupScriptFile = "startup.sh"
-		c.WrapStartupScriptFile = tc.WrapStartup
-		c.Zone = "test-zone"
-		state.Put("instance_name", "test-instance-name")
+			c.StartupScriptFile = "startup.sh"
+			c.WrapStartupScriptFile = tc.WrapStartup
+			c.Zone = tc.Zone
+			state.Put("instance_name", tc.MetadataName)
 
-		// This step stops when it gets Done back from the metadata.
-		d.GetInstanceMetadataResult = tc.Result
+			// This step stops when it gets Done back from the metadata.
+			d.GetInstanceMetadataResult = tc.MetadataResult
 
-		// Run the step.
-		assert.Equal(t, step.Run(context.Background(), state), multistep.ActionContinue, "Step should have continued.")
+			// Run the step.
+			assert.Equal(t, step.Run(context.Background(), state), tc.StepResult, "Step should have continued.")
 
-		assert.Equal(t, d.GetInstanceMetadataResult, tc.Result, "MetadataResult was not the expected value.")
-		assert.Equal(t, d.GetInstanceMetadataZone, tc.Zone, "Zone was not the expected value.")
-		assert.Equal(t, d.GetInstanceMetadataName, tc.MetadataName, "Instance name was not the expected value.")
+			assert.Equal(t, d.GetInstanceMetadataResult, tc.MetadataResult, "MetadataResult was not the expected value.")
+			assert.Equal(t, d.GetInstanceMetadataZone, tc.Zone, "Zone was not the expected value.")
+			assert.Equal(t, d.GetInstanceMetadataName, tc.MetadataName, "Instance name was not the expected value.")
+		})
 	}
 }

builder/googlecompute/testdata/wrapped-startup-scripts/errored.pkr.hcl

@@ -0,0 +1,51 @@
+
+variable "project" {
+  type    = string
+  default = "${env("GOOGLE_PROJECT_ID")}"
+}
+
+variable "service_account_file" {
+  type    = string
+  default = "${env("GOOGLE_APPLICATION_CREDENTIALS")}"
+}
+
+variable "ssh_private_key" {
+  type    = string
+  default = ""
+}
+
+variable "ssh_username" {
+  type    = string
+  default = "packer"
+}
+
+variable "zone" {
+  type    = string
+  default = "us-central1-a"
+}
+
+locals { timestamp = regex_replace(timestamp(), "[- TZ:]", "") }
+
+source "googlecompute" "autogenerated_1" {
+  account_file        = var.service_account_file
+  image_name          = "packer-tester-${local.timestamp}"
+  project_id          = "${var.project}"
+  source_image_family = "centos-7"
+  ssh_username        = "${var.ssh_username}"
+  start_script_file   = "./errored.sh"
+  skip_create_image   = true
+  zone                = "${var.zone}"
+}
+
+build {
+  sources = ["source.googlecompute.autogenerated_1"]
+
+  provisioner "shell" {
+    execute_command = "sudo -E -S sh '{{ .Path }}'"
+    inline          = ["ls /var/log"]
+  }
+
+  provisioner "shell" {
+    inline          = ["echo hello from the other side"]
+  }
+}

builder/googlecompute/testdata/wrapped-startup-scripts/errored.sh

@@ -0,0 +1,4 @@
+echo "Executing some startup script"
+echo ".....pretend we did a lot of work here"
+echo "Uh oh we are starting to fail!!!!!"
+exit 1

builder/googlecompute/testdata/wrapped-startup-scripts/successful.pkr.hcl

@@ -0,0 +1,50 @@
+
+variable "project" {
+  type    = string
+  default = "${env("GOOGLE_PROJECT_ID")}"
+}
+
+variable "service_account_file" {
+  type    = string
+  default = "${env("GOOGLE_APPLICATION_CREDENTIALS")}"
+}
+
+variable "ssh_private_key" {
+  type    = string
+  default = ""
+}
+
+variable "ssh_username" {
+  type    = string
+  default = "packer"
+}
+
+variable "zone" {
+  type    = string
+  default = "us-central1-a"
+}
+
+locals { timestamp = regex_replace(timestamp(), "[- TZ:]", "") }
+
+source "googlecompute" "autogenerated_1" {
+  account_file        = var.service_account_file
+  image_name          = "packer-tester-${local.timestamp}"
+  project_id          = "${var.project}"
+  source_image_family = "centos-7"
+  ssh_username        = "${var.ssh_username}"
+  skip_create_image   = true
+  zone                = "${var.zone}"
+}
+
+build {
+  sources = ["source.googlecompute.autogenerated_1"]
+
+  provisioner "shell" {
+    execute_command = "sudo -E -S sh '{{ .Path }}'"
+    inline          = ["ls /var/log"]
+  }
+
+  provisioner "shell" {
+    inline          = ["echo hello from the other side"]
+  }
+}

builder/googlecompute/testdata/wrapped-startup-scripts/successful.sh

@@ -0,0 +1,4 @@
+echo "Executing some startup script"
+echo ".....pretend we did a lot of work here"
+echo "we are all done"
+exit 0