Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Hyperv vmcx builder and allow vhd/vhdx instead of ISO #4944

Closed
wants to merge 16 commits into from

Conversation

taliesins
Copy link
Collaborator

  • Import virtual machine that has been exported to a folder
  • Clone an existing virtual machine
  • Can specify a vhd or vhdx file for iso_url, which will be mounted as hdd instead of creating a new one. This works for both hyperv builders
  • Add a few more unit tests to hyperv builders

@taliesins
Copy link
Collaborator Author

Testers can download binaries from: downloads of my local builds

@mwhooker mwhooker added this to the v1.1.0 milestone May 30, 2017
@pvandervelde
Copy link
Contributor

@taliesins Is there any chance you could rebase this one to grab the changes made in #4687? I tried building my own copy yesterday by locally merging your change to master but I only got the Linux binaries, not the windows ones :(

@pvandervelde
Copy link
Contributor

Managed to build a copy thanks to the wildly improved build scripts in the Packer repository. So far I've used that version of Packer to build Linux (Ubuntu 16.04) and Windows (2016) images from a previously generated base image. Haven't found any problems so far! All looking good here.

@taliesins
Copy link
Collaborator Author

@pvandervelde I assume this branch is working for you. I am going to rebase and merge it.

@BSick7
Copy link

BSick7 commented Aug 16, 2017

I find it confusing that a .vhd could be set as the iso.
These are different constructs and seems to go against the consistency of other providers (vmware, virtualbox, etc.).

@pvandervelde
Copy link
Contributor

@taliesins Yeah I did a rebase on master for myself and that was working just fine, so I'd say go for it.

@taliesins
Copy link
Collaborator Author

@BSick7 I agree with you on the confusion around iso and vhd. What would your suggestion be?

I would like to specify a remote location for a vhd. So I would like it to go through the same caching that iso would go through.

There was some awkwardness I was avoiding by reusing iso.

@BSick7
Copy link

BSick7 commented Aug 17, 2017

I see 2 ways of bringing clarity to this work.

  1. Each VM source would have its own packer builder (hyperv-iso, hyperv-vhd, etc.).
  2. There would be different source fields (iso_url, source_path_vhd, etc.).

I vote for (1) which prompted my PR #5253.

On a side note, I've always thought that it's weird that the builder includes the logic for sourcing the VM.
I always thought that constructed be abstracted into its own section.

)

// Run the steps.
if b.config.PackerDebug {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be changed to b.runner = packerCommon.NewRunner(steps, b.config.PackerConfig, ui). See #5297

@mwhooker
Copy link
Contributor

I agree with you on the confusion around iso and vhd. What would your suggestion be?

I think it's too bad, but we already overload the ISO prefix to mean any kind of disk image (hence the iso_target_extension field). Any change to this I think would have to hit anything that uses ISOConfig.

Maybe at the bare minimum, documenting that the iso_url etc are not just for isos, but can be used for VHDs. Maybe an example templating showing such.

This looks pretty good to me. @taliesins Please feel free to merge in when you think it's good to go. Please merge it by 9/5 so it's in the 1.1 release.

Needs a make fmt, too

@@ -314,6 +319,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
&common.StepCreateFloppy{
Files: b.config.FloppyConfig.FloppyFiles,
Directories: b.config.FloppyConfig.FloppyDirectories,
Directories: b.config.FloppyConfig.FloppyDirectories,
Copy link
Contributor

Choose a reason for hiding this comment

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

These two lines are duplicated.

@nebffa
Copy link

nebffa commented Sep 9, 2017

Any news on this? It would be really nice to have.

@vijayinvites
Copy link
Contributor

Yeah, I am also waiting for this to be merged in to master.

@mwhooker
Copy link
Contributor

hmm, had to change a few things. would feel more comfortable if this builder incubated for a bit before merging. can anyone report if they've had a successful build with it?

@mwhooker mwhooker modified the milestones: v1.2.0, v1.1.0 Sep 11, 2017
@pvandervelde
Copy link
Contributor

@mwhooker I've done a local merge to master and then build that a while ago. I've been building both Linux and Windows images with it for nearly 3 months now without any issues. Works fine on both my home machine and several work machines.

@pvandervelde
Copy link
Contributor

@mwhooker Did you want me to try and do a local merge and build it in the current state to test it? Probably won't be till tonight NZ local time earliest.

@mwhooker
Copy link
Contributor

@pvandervelde Any testing would be awesome, thanks!

taliesins and others added 14 commits October 4, 2017 15:26
Fix debug messages for cloning

Add hyperv-vmcx as a builder from command line
…it is used as the hard drive for spinning up a new machine, importing an exported virtual machine or cloning a virtual machine.

Can import a virtual machine from a folder
Can clone an existing virtual machine
…existing hard drive which means that we don't need to specify hard drive size

Filepath.ext includes the dot
@mwhooker
Copy link
Contributor

mwhooker commented Oct 9, 2017

unfortunately there are some conflicts from #5206

I will do my best to resolve them, but would like @sandersaares and @taliesins to take a look to make sure I didn't break anything

@pvandervelde
Copy link
Contributor

@mwhooker Let me know when you have done it and I do a local merge, build and test if you want me to

@pvandervelde
Copy link
Contributor

@taliesins Any chance you can have a look at building a VM from just the VHDX? From the documentation I got the impression that it should work but the mandatory parameters make it impossible

@mwhooker
Copy link
Contributor

mwhooker commented Oct 9, 2017

I'm sorry, it's beyond my ability right now to resolve the conflicts.

@vijayinvites
Copy link
Contributor

I can try resolving the conflicts. Whats the process to follow. Do I need permissions?

@rickard-von-essen
Copy link
Collaborator

@vijayinvites just pull these changes and rebase and open a new PR referencing this.

@vijayinvites
Copy link
Contributor

vijayinvites commented Oct 11, 2017

@rickard-von-essen @mwhooker , I have resolved the merge conflicts and created a new PR as per your suggestion.

@vijayinvites
Copy link
Contributor

I would like to see this gets included in 1.1.1 release.

@mwhooker
Copy link
Contributor

Merged in #5444

@mwhooker mwhooker closed this Oct 13, 2017
@gurry
Copy link

gurry commented Jan 16, 2018

Hi guys. Any idea when this feature will be available in a public release of Packer?

@gurry
Copy link

gurry commented Jan 16, 2018

Ignore that. I just saw that the feature is available in the current release and is documented here: https://www.packer.io/docs/builders/hyperv-vmcx.html. Don't know why Google didn't throw up this page when I searched for creating vagrant boxes from existing VHDs using Packer. Or may be it was too far down in the results.

@ghost
Copy link

ghost commented Apr 2, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants