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

Vsphere windows support #6087

Merged
merged 6 commits into from
Apr 14, 2016
Merged

Vsphere windows support #6087

merged 6 commits into from
Apr 14, 2016

Conversation

aheeren
Copy link
Contributor

@aheeren aheeren commented Apr 8, 2016

Initial support for cloning of windows machines in Vsphere. Not all Vsphere windows features are implemented. This checks if the vsphere OS name starts with "win", and makes a windows clone config if it does, and a linux clone config otherwise. Without this change, creating clones of windows machines fails. fixes #5490

@Preskton
Copy link

Preskton commented Apr 8, 2016

cc @rossedman

err = template.Properties(context.TODO(), template.Reference(), []string{"parent", "config.template", "config.guestId", "resourcePool", "snapshot", "guest.toolsVersionStatus2", "config.guestFullName"}, &template_mo)

var identity_options types.BaseCustomizationIdentitySettings
if strings.HasPrefix(template_mo.Config.GuestId, "win") {
Copy link

Choose a reason for hiding this comment

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

Dig it - this is the same path we were going down - yours is much cleaner. We may include the pieces to domain join in a second round once we can show that basic provisioning of a Win box works.

@ricardclau
Copy link
Contributor

Really interested in this one.
Will try to build this locally this week, test it against our vsphere and provide some feedback

@rossedman
Copy link

@aheeren I have not been able to make this work yet. Guest customizations are failing. Do you have a specific box you are using to test with

@ricardclau
Copy link
Contributor

@aheeren @rossedman I would appreciate some help in order to be able to help testing this :)

What I was trying to do is actually compiling the vsphere provider with:

go get github.com/aheeren/terraform
switch to branch Vsphere-windows
cd $GOPATH/src/github.com/aheeren/terraform/builtin/bins/provider-vsphere
Run go get there -> creates a $GOPATH/bin/provider-vsphere
Rename that bin as terraform-provider-vsphere

And I expected Terraform to use that newly built provider

It is complaining from a tf file including the windows_opt_config section so there is something wrong in my way of doing it

Any ideas?

@rossedman
Copy link

@ricardclau Reference the make instructions on the terraform home page. You need to use make dev or make plugin-dev PLUGIN=provider-vsphere

@ricardclau
Copy link
Contributor

That did not work, this is why I tried to do the steps manually

The problem with that approach is that as you can see in the Makefile, it tries to build from the hashicorp branch, not the one from @aheeren

@ricardclau
Copy link
Contributor

I double checked that I can build the terraform-provider-vsphere binary with my method and my particular build is the one in my path (just added some cheeky log at the beginning)

However when I run terraform it seems to not use this one as it cannot see the windows_opt_config bit or there is something else wrong

Any ideas? :(

@aheeren
Copy link
Contributor Author

aheeren commented Apr 11, 2016

Sorry for the difficulty guys. Verbatim error messages would be helpful so I can debug and update my code.

@rossedman I was using a windows server 2008 machine out of the box. I was using .tf.json, but that shouldn't make a difference. Here's my windows-opt-config:

"windows_opt_config" : {
    admin_password" : "password",
    "product_key" : "xxxxx-xxxxx-xxxxx-xxxxx-xxxxx"
},
...

Having an invalid product key will allow vsphere to create the clone, apply some of the configs (password, network configs i believe) but fail during customization before you can log in. This is what I was doing for most of my testing because legit product keys are expensive.

@ricardclau It sounds like the you're building it correctly, most likely this is a code bug that I'll have to trace and fix.

@aheeren aheeren changed the title Vsphere windows support WIP-Vsphere windows support Apr 11, 2016
@ricardclau
Copy link
Contributor

@hi @aheeren

My windows_opt_config is:

  windows_opt_config {
     domain = "OUR DOMAIN"
     domain_user = "AN EXISTING USER"
     domain_user_password = "A PASSWORD"
  }

Does it need the : ? That's not terraform syntax right?

@aheeren
Copy link
Contributor Author

aheeren commented Apr 11, 2016

@ricardclau The format of the config looks good, but "product_key" is non-optional. I believe this would work instead:

windows_opt_config {
     domain = "OUR DOMAIN"
     domain_user = "AN EXISTING USER"
     domain_user_password = "A PASSWORD"
     product_key = "xxxxx-xxxxx-xxxxx-xxxxx-xxxxx"
  }

until it fails because the product key is invalid.

@ricardclau
Copy link
Contributor

I tried adding the product_key and it did not work either :(

Errors:

  * vsphere_virtual_machine.windows_test: : invalid or unknown key: windows_opt_config

If I run it without the windows_opt_config it starts correctly but I get this error eventually:

Error applying plan:

2016/04/11 17:38:14 [DEBUG] waiting for all plugin processes to complete...
1 error(s) occurred:

* vsphere_virtual_machine.windows_test: A specified parameter was not correct. 
spec.identity

Not sure if this can prove if I am running your version of the provider or the stable one

@rossedman
Copy link

@aheeren I just had this work with vSphere 5.5 / Windows 2012 R2. I am trying some different variations but so far it is good.

@ricardclau
Copy link
Contributor

So, more fun stuff :)

The only way I managed to make it work was:

  • rsync all the $GOPATH/src/github.com/aheeren to $GOPATH/src/github.com/hashicorp
  • rebuild the thing

Now the windows_opt_config is picked... @aheeren do you guys need to do this all the time or is there a better way to make it work?

@aheeren
Copy link
Contributor Author

aheeren commented Apr 11, 2016

@ricardclau Glad you got it working. I have one git repo at go/src/github.com/hashicorp/terraform with 2 remotes, origin and hashicorp:
origin: git@github.com:aheeren/terraform.git
hashicorp: git@github.com:hashicorp/terraform.git

@rossedman
Copy link

@aheeren I don't know a ton about windows but we are manually setting IP and DNS and the machine boots on with a different IP and it seems to take awhile to reconfigure. Is this common? We are not using DHCP.

@aheeren
Copy link
Contributor Author

aheeren commented Apr 11, 2016

@rossedman Sorry, that's at the edge of my knowledge. Vsphere uses the same network specifications for Windows and Linux, so I assume the behavior should be the same. Do you have a network_interface in your .tf? If not, it should inherit all network configs from the template.

@ricardclau
Copy link
Contributor

Right @aheeren that's more or less what I emulated by rsyncing, although your way is FAR CLEANER and I will follow your advice :)

@rossedman
Copy link

@aheeren Its okay. I feel like everyone's vSphere knowledge drops off at some point. Ha. I will keep trying some things.

@ricardclau
Copy link
Contributor

Hi @aheeren the box is booting here, good stuff. It is fairly slow even if we are supplying an IP address so there might be some issue similar to what @rossedman is pointing out. My Vsphere knowledge is very limited so not able to help there :)

I used a template we have with Windows 2012 R2 Datacenter and I am waiting now in the after Terraform phase when it is trying to apply the autounattend files and so on. I am heading home as it takes a while to apply all that crap but again, really nice stuff! Thanks!

Maybe a nice addition would be to add a Windows timezone default (I guess "085" would be the closest to the Linux UTC one)?

Other than that... anything in particular you need testing we may be able to help with?

@aheeren
Copy link
Contributor Author

aheeren commented Apr 11, 2016

@ricardclau Thanks for offering! I'm going to put in the default windows timezone this evening. If you (and @rossedman if he's up for it) could either take a look at the systprep logs or put them in a pastebin somewhere (after removing sensitive data), I/we can try and find out if there's a loop that's bugging out or if Windows is just slow at setting IPs. Other than that... Do either of you know the best way to get Hashicorp folks to take a look at this?

@ricardclau
Copy link
Contributor

There is some problem with the box not being added to our domain (will check logs tomorrow and also check with the guys that have further experience with vsphere) but the box is definitely up and stable. Nice stuff!

Will try to share some sysprep logs tomorrow and see if we can make it a bit better

In terms of pinging Hashicorp... I guess we can ping @stack72 who merged a PR I did last Friday (and it was the one causing yours not being able to be merged, sorry about that)

@Preskton
Copy link

@aheeren in terms of HashiCorp support, we are unlikely to get much help in the way of debugging specific interactions between vSphere and Windows while using Terraform. For this provider, they are pretty much committed to doing the PR merge and running tests - not much more. Once we're all feeling good about the PR - and thank you all so far for your help in getting it going/tested - if we all push to get it merged in, hopefully, it'll be quick.

@rossedman
Copy link

@aheeren This works for me. I think there are a couple adjustments to make that the Terraform team will ask but I can do a full Windows creation and provision. 👍 I think we should definitely push for merge on this.

@@ -124,6 +136,12 @@ func resourceVSphereVirtualMachine() *schema.Resource {
ForceNew: true,
},

"linkedClone": &schema.Schema{

Choose a reason for hiding this comment

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

@aheeren I think this will be asked to change since it doesn't meet naming convention standards. Probably asked for linked_clone and this may conflict with other PRs that are currently out there.

@Preskton
Copy link

@rossedman @aheeren Reached out to HashiCorp in parallel to bump the priority on this PR.

@phinze
Copy link
Contributor

phinze commented Apr 13, 2016

Hi folks, sounds like there are multiple corroborating reports that this PR works to enable windows support. That's good news!

@rossedman's prediction on the attribute name format was correct - if we can get linkedClone switched to use linked_clone then I'm happy to pull this in. 👍

@rossedman
Copy link

@phinze Will do. 👍

@aheeren aheeren changed the title WIP-Vsphere windows support Vsphere windows support Apr 13, 2016
@aheeren
Copy link
Contributor Author

aheeren commented Apr 13, 2016

@phinze Rename is done f04298f

@ricardclau
Copy link
Contributor

Would it be possible to add the default windows timezone we talked about as well? Or at least a bit of a better error message? (not a big deal and it can certainly be added after this is merged)

@rossedman
Copy link

@aheeren 👍 Amazing work. Thank you.

@chrislovecnm
Copy link
Contributor

@phinze are you pleased with the unit test code coverage? I will walk through it tomorrow, but I am not seeing unit tests ;)

Depending on how the code is laid out it may be low risk to not have tests, but we will want them in the near future.

Will look at the PR tomorrow.

@ricardclau
Copy link
Contributor

Excellent work @aheeren. Thank you.

@phinze
Copy link
Contributor

phinze commented Apr 14, 2016

@chrislovecnm It'd be great to get some acceptance test coverage around this, but based on the manual testing reports and the highly requested nature of the functionality, I'm going to merge as-is so we can get this out there.

Thanks @aheeren and everybody who helped out!

@phinze phinze merged commit eded8bb into hashicorp:master Apr 14, 2016
@chrislovecnm
Copy link
Contributor

@phinze I understand, and yes I am super happy to see t in, and that is why I said it was low risk ;)

@aheeren I know you have done a lot, might I ask one more favor, which I am happy to help via answering any questions that you have, or anything else I can do. Might we please get some unit tests. Pretty please ;) More unit and functional tests will help us develop quicker and crest a much more stable product!!! Better toys for everyone.

@aheeren
Copy link
Contributor Author

aheeren commented Apr 14, 2016

@chrislovecnm Thanks, i'll get on that. I'll put some time in tomorrow, but probably won't have anything working/useful before mid next-week. Thanks everyone for building and testing!

@chrislovecnm
Copy link
Contributor

@aheeren 👍 and thanks!!!

@phinze you want another PR for the unit tests?

@ghost
Copy link

ghost commented Apr 26, 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 26, 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.

Terraform vSphere provider Windows support
7 participants