fix: #7

[remi-espie]

When the keep_registered flag was true, the VM stayed in HyperV manager, but its files were deleted anyway, making it impossible to use the VM as-is.

This PR makes the VM's files cleanup look for the keep_registered flag, and if set to true, will not delete the VM files, keeping it usable.

However, the VM files stays in the temp folder. We may want to move the files to another location beforehand if the flag is up.

This PR closes #7 .

[lbajolet-hashicorp]

Hey @remi-espie,

Thanks for this PR!

By curiosity, have you tested it in your setup since you hit the problem described in the issue? Did that fix it?

I'm not a Hyper-V expert at all, so I'll have to rely on your expertise here, but the code looks good to me, just want to confirm it does work at least in your env, and I left a quick suggestion regarding the removal of the image.

Let me know when you have time to address the suggestion I left, and I'll do a second pass after that.

[remi-espie]

Hello !

Thanks for the great suggestion, I just added it !

To answer your initial question :

By curiosity, have you tested it in your setup since you hit the problem described in the issue? Did that fix it?

I'm not a Hyper-V expert at all, so I'll have to rely on your expertise here, but the code looks good to me, just want to confirm it does work at least in your env, and I left a quick suggestion regarding the removal of the image.

I'm not a Hyper-V expert either, I just worked with it recently. I tested it one time and it worked flawlessly: the VM is still registered in Hyper-V and you can use it without issue afterward. However, as I said, the artifacts stay located in the build directory in the user's TEMP folder, which does not seems to be ideal.

[lbajolet-hashicorp]

This is a good point, I'm not sure what the artifact produced by Packer looks like for Hyper-V, but in this case it might be a good idea to export the files in a separate location then?
I don't know how temp files are managed on Windows, but if it's in any way similar to Linux, they might be memory-mapped, and flushed on shutdown, so in this case we lose them on each power-cycle.

This may signal we should have an output_directory (through this being Windows, output_folder may be more idiomatic) option for the template? This would supersede keep_registered in this case, and export the files in a persistent directory.
This option may even be the actual working directory rather than a temporary one, which will not be cleaned-up, unless there's a problem with the build as pointed out in my last review.

If you agree and decide to go with the output_directory route, feel free to take inspiration from the qemu plugin for example. Please note that the option is not mandatory here as we pick a default, but there always is a persistent directory for the output, and must not exist before running the build.

Let us know what you think, and thanks for the quick turnaround time!

[remi-espie]

For Hyper-V, the artifacts consists of a few folder with data about the VM (like network interface, CPU number, etc) in .vmxc files and the VM as a .vhdx file.

I believe Windows does not flush the TEMP folder at each shutdown, however I also think that an output_directory option would be a great addition.

I won't be able to work on this new option in the following weeks, so it may be better to create a new PR that would add the output_directory option. What do you think ?

[lbajolet-hashicorp]

I don't think we'll be able to work on this soon as well, so regarding this PR that's up to you, I don't mind leaving it open with the discussion readily available, and either you can pick it up when you have time, or someone else may piggyback off it and add the missing attribute.

If you prefer to start with a clean slate, feel free to close it and we'll review the output directory PR when available for sure.

Thanks!

[remi-espie]

Hello @lbajolet-hashicorp . I just noticed that the Hyper-V plugin has a temp_path parameter which default to %TEMP%. By changing it, you can have the VM still registered and working in the directory you want.
Also, there is already a output_directory parameter. It is not used when the keep_registered parameter is true though, as it only works for the VM exportation.

What do you think I should do ? I may supersede temp_path with output_directory when keep_registered is set to true, however it might break backward compatibility. Else, I can go with temp_path and call it a day.

[lbajolet-hashicorp]

Hi @remi-espie,

Regarding the temp_path attribute, if it already exists and makes it possible for users to have their files in the desired location, we can use it in conjunction with keep_registered then.
Let's continue with your PR, I would probably suggest adding something to documentation of the attribute so it is stated that the VM's files will reside in temp by default, unless the temp_path attribute is set to something else.

We could probably rename the attribute to something more explicit regarding how it's meant to be used (I'm thinking something like vm_dir, but others would work), but as you point out, doing so will break compatibility, and I don't think we need to do it now.

In the end then yes, let's continue with this PR so the directory doesn't get wiped, and let's point out that temp_path should be set for such cases in the docs, then we should be good to go.

Thanks!

[lbajolet-hashicorp]

LGTM, thanks for the rerolls @remi-espie !

[pmcevoy]

Something still off with the combination of parms to get this to work

I have the following params that I believe are relevant to delivering a working VM in Hyper-V manager:

  temp_path             = "E:/Hyper-V/"
  shutdown_command = "shutdown /s /t 5" 
  shutdown_timeout = "60s"
  keep_registered = true
  skip_export     = true 
  skip_compaction = true

I have no output_directory specfied according to hints mentioned in this thread.

HyperV "Virtual Machine" files appear to remain in the build folder below E:/Hyper-V/ - but there is no "Virtual DIsks" folder - that appears to be created in an output-xxx subfolder beside the HCL file.

The VM listed in HyperV manager will not start - complains about missing disk files. WHen I move the Virtual DIsk folder below the build folder, then complains about incorrect perms.

Ulitimately perhaps I'm abusing packer to create VM instances rather than images, but then perhaps keep_registered and skip_export should not be options?

packer 1.9.4 with hyperv plugin 1.1.3

[remi-espie]

Hello @pmcevoy, I've tested your configuration, and turns out, skip_export mess with the keep_registered parameter. I may make both parameter incompatible if used together, but for now, you can use the VMs in their temp path if you remove the skip_export parameter (which, is this case, does nothing else than moving the .vhdx file).