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

qemu builder and vbox builders now need to explicitly set WinRMPort for StepConnect #4321

Merged
merged 2 commits into from
Dec 24, 2016

Conversation

DanHam
Copy link
Contributor

@DanHam DanHam commented Dec 23, 2016

#2576 introduced a bug that broke WinRM for Virtualbox builders since it made changes to the way WinRMPort was defined in helper/communicator/step_connect.go - see #4310. Hopefully this change aligns the Virtualbox builders to the new way of doing things.

I've tested the changes with the Virtualbox iso builder and communication over WinRM is now working again. I've also tested a Linux build to ensure the ssh communicator is still working as expected.

Note that the Virtualbox ovf builder should also be affected by the changes #2576 made. I haven't been able to perform any checks that this is actually the case as I don't have an ovf template to hand...

Closes #4310

Aligns vbox builders with changes to helper/communicator/step_connect.go
introduced by PR #2576

Fixes #4310
@rickard-von-essen
Copy link
Collaborator

This probably effects qemu too, ping @vtolstov

@DanHam
Copy link
Contributor Author

DanHam commented Dec 23, 2016

This probably effects qemu too

@rickard-von-essen I think you're right there! Perhaps

	if b.config.Comm.Type != "none" {
		steps = append(steps,
			&communicator.StepConnect{
				Config:    &b.config.Comm,
				Host:      commHost,
				SSHConfig: sshConfig,
				SSHPort:   commPort,
			},
		)
	}

from lines 394 - 403 of builder/qemu/builder.go should be changed in a similar way. e.g.

	if b.config.Comm.Type != "none" {
		steps = append(steps,
			&communicator.StepConnect{
				Config:    &b.config.Comm,
				Host:      commHost,
				SSHConfig: sshConfig,
				SSHPort:   commPort,
				WinRMPort: commPort,
			},
		)
	}

@rickard-von-essen @vtolstov Unfortunately, I don't have the setup to be able to test the qemu builder. However, if the qemu builder was broken by #2576 and that change actually fixes it, I would be happy to submit the change above in with this PR...

@vtolstov
Copy link
Contributor

Please, wait i'm recheck qemu builder now

@vtolstov
Copy link
Contributor

vtolstov commented Dec 23, 2016

Yes, under qemu winrm is broken in current master.

==> qemu: Starting HTTP server on port 8723
==> qemu: Found port for communicator (SSH, WinRM, etc): 4435.
==> qemu: Looking for available port between 5900 and 6000 on 127.0.0.1
==> qemu: Starting VM, booting from CD-ROM
    qemu: The VM will be run headless, without a GUI. If you want to
    qemu: view the screen of the VM, connect via VNC without a password to
    qemu: 127.0.0.1:5912

But packer use default port

2016/12/23 22:09:15 packer: 2016/12/23 22:09:15 [INFO] Attempting WinRM connection...
2016/12/23 22:09:15 packer: 2016/12/23 22:09:15 [DEBUG] connecting to remote shell using WinRM
2016/12/23 22:09:15 packer: 2016/12/23 22:09:15 [ERROR] connection error: unknown error Post http://127.0.0.1:5985/wsman: dial tcp 127.0.0.1:5985: getsockopt: connection refused
2016/12/23 22:09:15 packer: 2016/12/23 22:09:15 [ERROR] WinRM connection err: unknown error Post http://127.0.0.1:5985/wsman: dial tcp 127.0.0.1:5985: getsockopt: connection refused

@vtolstov
Copy link
Contributor

@DanHam please add fix for qemu builder

Same as vbox builders, aligns qemu with changes to
helper/communicator/step_connect.go introduced by PR #2576
@DanHam
Copy link
Contributor Author

DanHam commented Dec 23, 2016

@vtolstov Done. Hopefully that will fix. Please test again.

@DanHam DanHam changed the title vbox builders now need to explicitly set WinRMPort for StepConnect qemu builder and vbox builders now need to explicitly set WinRMPort for StepConnect Dec 23, 2016
@vtolstov
Copy link
Contributor

Thanks, i'm trying.

@vtolstov
Copy link
Contributor

i'm check windows build and not it works fine, so lgtm

@DanHam
Copy link
Contributor Author

DanHam commented Dec 24, 2016

OK. Good news. Thanks for providing the testing!

@rickard-von-essen
Copy link
Collaborator

Great work! 🎄

@rickard-von-essen rickard-von-essen merged commit 33f1808 into hashicorp:master Dec 24, 2016
@DanHam
Copy link
Contributor Author

DanHam commented Dec 24, 2016

@rickard-von-essen Thanks! Have a very Merry Christmas!!

@StefanScherer
Copy link
Contributor

How about a small bugfix release 0.12.2 which fixes this problem for VirtualBox?

@ghost ghost locked and limited conversation to collaborators Apr 5, 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.

packer 0.12.1 VirtualBox builder uses wrong WinRM port.
4 participants