Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Wrapper startup-script sets startup-script-status to "done" instead of "error" when a wrapped startup script fails. #45

Closed
supersmo opened this issue Sep 20, 2021 · 5 comments · Fixed by #135

Comments

@supersmo
Copy link

Overview of the Issue

The Wrapper startup-script defined in startup.go:

var StartupScriptLinux string = fmt.Sprintf(`#!/usr/bin/env bash

sets the metadata startup-script-status to "done" instead of "error" when the wrapped script fails.

To fix it change:

echo "Packer startup script done."
SetMetadata %s %s
exit $RETVAL
`, StartupWrappedScriptKey, StartupScriptStatusKey, StartupScriptStatusDone)

To something like:

if [ "$RETVAL" == "0" ]; then
  echo "Packer startup script done."
  SetMetadata %[2]s %[3]s
else
  echo "Packer startup script failed with return value: $RETVAL"
  SetMetadata %[2]s %[4]s
fi
exit $RETVAL
`, StartupWrappedScriptKey, StartupScriptStatusKey, StartupScriptStatusDone, StartupScriptStatusError)

Reproduction Steps

Create a startup-script that fails and let the builder wrap it.
Trigger a packer build and notice that the startup-script-status is set to "done".

Plugin and Packer version

plugin: googlecompute 1.05

@github-actions
Copy link

github-actions bot commented Dec 6, 2022

This issue has been synced to JIRA for planning.

JIRA ID: HPR-864

@nywilken
Copy link
Contributor

nywilken commented Dec 7, 2022

Hi @supersmo thanks for bubbling this up. Taking a look at the issue I can understand the confusion here, thanks for adding a potential fix to the script. Looking at the code it appears that the builder will continue to wait and eventually timeout if the status set in the Metadata is anything but "done". Which could be a reason why the status was never set correctly or it was truly a miss on our end.

That said, by updating the code to set the metadata status to "error", would you expect Packer to fail if the startup script resulted in a failure?

Seeing as the current behavior would have resulted in an a failed Packer build due to a timeout issue. I'm allowing Packer to fail if the wrapped startup script failed to prevent the user from thinking that everything executed without error. Please let me know if that is not what you think a user would expect.

@nywilken
Copy link
Contributor

nywilken commented Dec 7, 2022

Thanks again for reporting, this should be fixed by #135 ; some test binaries are available via the link below if you would like to test the fix.

https://app.circleci.com/pipelines/github/hashicorp/packer-plugin-googlecompute/256/workflows/980ca1fe-50d9-4188-9351-f3bbbb730a08/jobs/2859/artifacts

@supersmo
Copy link
Author

That said, by updating the code to set the metadata status to "error", would you expect Packer to fail if the startup script resulted in a failure?

Seeing as the current behavior would have resulted in an a failed Packer build due to a timeout issue. I'm allowing Packer to fail if the wrapped startup script failed to prevent the user from thinking that everything executed without error. Please let me know if that is not what you think a user would expect.

Yepp, I'd expect the Packer build to fail. 👍

We've switched over to os-login since this was reported so we're no longer using the statup metadata for our packing, but I trust that it now works 😊

@heyealex
Copy link

heyealex commented Jan 5, 2023

Is there an ETA on when a new version with this fix will be released?

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants