search

hetznercloud / packer-plugin-hcloud

Packer plugin for Hetzner Cloud Builder
https://developer.hashicorp.com/packer/integrations/hetznercloud/hcloud
Mozilla Public License 2.0
25 stars 21 forks source link

if snapshot name already exists and -force specified, safely overwrite it #18

Closed marco-m closed 2 years ago

marco-m commented 3 years ago

This is on top of #17, but unfortunately this GitHub UI doesn't show only the incremental work. Best reviewed commit-per-commit.

With previous PR #17 we detected if a snapshot name was already present and refused to proceed, unless packer build -force was specified. If it was specified, then we would add the new snapshot to the existing one.

This PR brings the behavior of this plugin completely on par with the expected behavior of packer build -force:

-force    Force a build to continue if artifacts exist, deletes existing artifacts.

The overwite is done safely: first a new image is built and a snapshot is taken. Only if the new snapshot is taken with success we then delete the old one.

As for PR #17, this PR comes with full test coverage.

NOTE: while developing this feature, I noticed two pre-existing bugs, fixed in commits efcc738 rebase: 930a8e9 and d9b9ba3 rebase: f083f10. Probably worthwile to involve somebody from Hetzener cloud to double-check.

Closes #14

EDIT: rebased over master, now this PR contains only commits related to itself.

marco-m commented 3 years ago

Hello @nywilken, would you (or anybody from the Packer team) have time to look at this ? Thanks!

nywilken commented 3 years ago

@marco-m apologies can you rebase on to the latest main now that #17 has been merged. And to confirm #17 adds the new snapshot but doesn't remove the old. So this PR fixes that issue, correct?

marco-m commented 3 years ago

hello @nywilken !

@marco-m apologies can you rebase on to the latest main now that #17 has been merged.

done

And to confirm #17 adds the new snapshot but doesn't remove the old. So this PR fixes that issue, correct?

This is correct.

Also, as mentioned in the PR description: while developing this feature, I noticed two pre-existing bugs, fixed in commits efcc738 rebase: 930a8e9 and d9b9ba3 rebase: f083f10. Probably worthwile to involve somebody from Hetzener cloud to double-check.

marco-m commented 2 years ago

Hello @nywilken friendly ping if you have time :-)

marco-m commented 2 years ago

hello @LKaemmerling could you please have a look (or propose somebody else from Hetzener) ? Please pay attention also at the commits that fix pre-existing bugs (see details in this PR description). thanks!

LKaemmerling commented 2 years ago

Hey @marco-m,

looks fine from my side. Initially the for loop it was used to show the progress of the action, but as packer never used this it should be save to remove it.

marco-m commented 2 years ago

thanks for the quick feedback @LKaemmerling :-)

marco-m commented 2 years ago

Thanks @nywilken ! :-)

nywilken commented 2 years ago

Thanks @nywilken ! :-)

I'll work on cutting a release of this plugin tomorrow. I would like to pull in the latest Packer SDK change (plumbing change).