Skip to content

Cleanup VM in case of a failure - #120

Merged
fkorotkov merged 3 commits into
mainfrom
cleanup-vm
Mar 6, 2024
Merged

Cleanup VM in case of a failure#120
fkorotkov merged 3 commits into
mainfrom
cleanup-vm

Conversation

@fkorotkov

Copy link
Copy Markdown
Contributor

Fixes #115

@fkorotkov

Copy link
Copy Markdown
Contributor Author

@sparshev FYI

@fkorotkov
fkorotkov requested a review from edigaryev March 5, 2024 21:28
@sparshev

sparshev commented Mar 5, 2024

Copy link
Copy Markdown
Contributor

Sorry, see just one file is changed, but according to the builder you have 3 separated steps for this matter:

  • step_clone_vm.go
  • step_create_linux_vm.go
  • step_create_vm.go

But I'm not sure I really understand the codebase, so could be wrong.

@fkorotkov

Copy link
Copy Markdown
Contributor Author

Good catch, @sparshev! I've extracted cleanup in a separate step which is the first one. Cleanup functions are called in reverse order so it will be called the last.

@sparshev

sparshev commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

Yep, now looks better!) Do you think this PR will contain a fix for Ctrl-C issue as well, because in description it says Fixes #115. Or I can create a separated issue about Ctrl-C.

@fkorotkov

Copy link
Copy Markdown
Contributor Author

It seems it will be handled according to Packer documentation:

Cancels are most commonly triggered by external interrupts, such as the user pressing Ctrl-C. Packer will only exit once all the builders clean up...

@sparshev

sparshev commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

I'm talking about a bit different behavior: the issue with Ctrl-C that Tart VM is going down immediately after Ctrl-C, not after cleanup operation... So when you press Ctrl-C in packer with --on-error=ask the VM should continue to live until c and enter were pressed.

@fkorotkov

Copy link
Copy Markdown
Contributor Author

Oh. I think I misunderstood you but I still think that Ctrl+C will be handled since shutdown -h now is send in Cleanup of a run step.

@edigaryev is on PTO so I'm going to merge now so we can do a release. He'll review the change post-factum.

@fkorotkov
fkorotkov merged commit 95c1764 into main Mar 6, 2024
@fkorotkov
fkorotkov deleted the cleanup-vm branch March 6, 2024 16:14
@torarnv

torarnv commented Mar 9, 2024

Copy link
Copy Markdown
Contributor

Does this take the packer build -on-error= argument into account?

-on-error=[cleanup|abort|ask|run-cleanup-provisioner] If the build fails do: clean up (default), abort, ask, or run-cleanup-provisioner.

If I pass ask and press Ctrl+C, the VM is still deleted. I would have expected it to stay around, so I could debug what went wrong.

It seems to work fine if the cancellation is coming from a step, e.g. if the SSH times out. In that case the cleanup is not run when I choose so via the ask menu.

@torarnv

torarnv commented Mar 9, 2024

Copy link
Copy Markdown
Contributor

"Internal" failure. Choice respected:

==> macos.tart-cli.install-os: Creating virtual machine...
    macos.tart-cli.install-os: Installing OS...
    macos.tart-cli.install-os: 0%
    macos.tart-cli.install-os: Error: An error occurred during installation. Installation failed.
==> macos.tart-cli.install-os: Failed to create a VM: Bad exit status: 1
==> macos.tart-cli.install-os: Step "stepCreateVM" failed
==> macos.tart-cli.install-os: [c] Clean up and exit, [a] abort without cleanup, or [r] retry step (build may fail even if retry succeeds)? a
==> macos.tart-cli.install-os: Failed to create a VM: Bad exit status: 1
==> macos.tart-cli.install-os: Step "stepCreateVM" failed, aborting...
==> macos.tart-cli.install-os: aborted: skipping cleanup of step "stepCleanVM"
Build 'macos.tart-cli.install-os' errored after 2 minutes 10 seconds: Failed to create a VM: Bad exit status: 1

Ctrl+C. Choice ignored:

==> macos.tart-cli.install-os: Creating virtual machine...
    macos.tart-cli.install-os: Installing OS...
    macos.tart-cli.install-os: 0%
Cancelling build after receiving interrupt
==> macos.tart-cli.install-os: Failed to create a VM: Bad exit status: -1
==> macos.tart-cli.install-os: Step "stepCreateVM" failed
==> macos.tart-cli.install-os: [c] Clean up and exit, [a] abort without cleanup, or [r] retry step (build may fail even if retry succeeds)? a
==> macos.tart-cli.install-os: Cleaning up virtual machine...
    macos.tart-cli.install-os: failed to open lock file file:///Users/torarne/.tart/vms/macos-preinstall:13.6.0+22G120/config.json: No such file or directory
Build 'macos.tart-cli.install-os' errored after 1 second 874 milliseconds: Failed to create a VM: Bad exit status: -1

@edigaryev

Copy link
Copy Markdown
Contributor

Ctrl+C. Choice ignored:

This seems to be done intentionally by the Packer:

func ask(ui packersdk.Ui, name string, state multistep.StateBag) askResponse {
	ui.Say(fmt.Sprintf("Step %q failed", name))

	result := make(chan askResponse)
	go func() {
		result <- askPrompt(ui)
	}()

	for {
		select {
		case response := <-result:
			return response
		case <-time.After(100 * time.Millisecond):
			if _, ok := state.GetOk(multistep.StateCancelled); ok {
				return askCleanup
			}
		}
	}
}

I've created a hashicorp/packer-plugin-sdk#226 to address this.

@torarnv

torarnv commented Mar 14, 2024

Copy link
Copy Markdown
Contributor

Awesome @edigaryev !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No cleanup after failure

4 participants