Skip to content

Conversation

@YuriyAM
Copy link
Contributor

@YuriyAM YuriyAM commented Nov 13, 2024

Summary

This PR addresses an issue in the vsphere-clone builder where using the -force flag can lead to unintentional deletion of the target VM, which may result in the loss of critical data if the target VM is the only copy of a virtual machine.

Issue description

Currently, when the -force flag is specified, the vsphere-clone builder first deletes the target VM before verifying that the source VM to be cloned exists. If the source VM is missing, this process leaves users without a VM to clone and potentially results in data loss.

Use case

In our environment, Packer is used to perform regular updates on legacy virtual machine templates. These templates are unique, lack proper build configurations, and cannot be easily replicated. If, for any reason, human intervention results in running packer build with the -force flag, there is a significant risk of losing irreplaceable data.

Testing

Testing was performed to confirm:

  1. Source VM existence is verified before any deletion action on the target VM.
  2. With the -force flag, the target VM is only deleted if a source VM exists.

Related issues

@YuriyAM YuriyAM requested a review from a team as a code owner November 13, 2024 09:46
@hashicorp-cla-app
Copy link

hashicorp-cla-app bot commented Nov 13, 2024

CLA assistant check
All committers have signed the CLA.

@tenthirtyam
Copy link
Collaborator

Could you please link this pull request to an issue?

@YuriyAM
Copy link
Contributor Author

YuriyAM commented Nov 13, 2024

@tenthirtyam I couldn’t find any open or closed issue that this merge request directly relates to. If it’s required by your procedures, I’d be happy to create one 😊

@tenthirtyam
Copy link
Collaborator

yes, please. This is mentioned in the contributed guidelines. 😉

@YuriyAM
Copy link
Contributor Author

YuriyAM commented Nov 13, 2024

I might have overlooked that 😅 Thanks for guiding me! Here’s the link to the issue—#490

@tenthirtyam tenthirtyam added this to the Backlog milestone Nov 14, 2024
Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @YuriyAM,

At first glance this looks good to me, I'll defer to @tenthirtyam to give his opinion on the code before we can consider merging this PR, but on my end LGTM!

Copy link
Collaborator

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tenthirtyam
Copy link
Collaborator

@lbajolet-hashicorp - ready for your review again.

Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reroll and the pings @tenthirtyam @YuriyAM, the code looks good to me in the current state, and since the tests are all green, we can merge this one now!

@lbajolet-hashicorp lbajolet-hashicorp merged commit a5fbe40 into hashicorp:main Jan 27, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vsphere-clone: Unwanted behavior using -force flag when source virtual machine doesn't exist

3 participants