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

Try to detect the guest IP address with Get-NetNeighbor #5993

Closed
wants to merge 1 commit into from

Conversation

tuxillo
Copy link

@tuxillo tuxillo commented Mar 8, 2018

For guests that don't have support for Hyper-V integration services
try to use a hack to find the IP Address from the neighbor cache entries.

Pointed-out-by: Matt Hickok (@matthew-hickok)

Tests:

C:\Users\Administrator\go\src\github.com\hashicorp\packer>go test
PASS
ok github.com/hashicorp/packer 1.133s

C:\Users\Administrator\go\src\github.com\hashicorp\packer>go fmt

C:\Users\Administrator\go\src\github.com\hashicorp\packer>

Related to #4404 , #4656 and #5049 , probably more.

For guests that don't have support for Hyper-V integration services
try to use a hack to find the IP Address from the neighbor cache entries.

Pointed-out-by: Matt Hickok (@matthew-hickok)
@tuxillo tuxillo requested a review from a team as a code owner March 8, 2018 16:03
@SwampDragons
Copy link
Contributor

Is this build sufficient for closing #4404, or does #4656 also need to be merged? I do not have an environment for testing that reproduces the problems stated in #4404 so I need some validation from the community on this. Perhaps @matthew-hickok or @w9n or @gh2k can weigh in, since they experienced the issue described in #4404?

@tuxillo
Copy link
Author

tuxillo commented Mar 13, 2018

Hi,

PR #4656 is definitely needed because otherwise it is not possible to use a legacy network adapter for the builder VM. However, that PR seems to need some conflict resolution for it to merge properly, according to Github.

Anyways, since I have the environment I can provide logs and/or evidences needed. But as you said, it would be good if those users could weigh in : -)

Regards,
Antonio Huete

@SwampDragons
Copy link
Contributor

If you'd be interested in taking on that PR and resolving conflicts, I can try to work on merging these for the next release.

@tuxillo
Copy link
Author

tuxillo commented Mar 15, 2018

@SwampDragons alright I'll try to resolve the conflicts, get it building and working. I'll include logs and all that.
I think it would be good to include in the documentation some sort of warning about legacy network adapters and their usage with vagrant, what do you think?

@SwampDragons
Copy link
Contributor

@tuxillo I'm always a fan of airing on the side of more documentation. If you want to write something up, I can help find a good home for it in the docs.

@tuxillo
Copy link
Author

tuxillo commented Mar 21, 2018

BTW, @gh2k 's solution looks cleaner than the one I tried right? (tuxillo@f67c265)

I'd say factoring out is good but not sure what your preference is for packer.

@SwampDragons
Copy link
Contributor

Yeah I think it's cleaner.

@SwampDragons
Copy link
Contributor

@tuxillo I just closed the older PR because it looked like nothing was happening with it. What do you want me to do with this one? My understanding is that it doesn't do the job by itself, so I'm inclined to close until we have one PR with all of the changes necessary to fix this problem. What do you think?

@tuxillo
Copy link
Author

tuxillo commented Apr 5, 2018

@SwampDragons yeah, that's correct. Ignore my comment in the other issue, I misread and I thought you actually closed this one.

Here's what I think it has to happen:

  • Rebase @gh2k 's branch and submit together with logs to verify it does what it is supposed to do.
  • Test my PR after and verify it picks up the IP in the cases it is supposed to do it.
  • Add proper documentation this hack and when it will work.

What do you think?

@SwampDragons
Copy link
Contributor

I think that sounds like a good course of action.

@withinboredom
Copy link

Is a legacy adapter required to use this pr @tuxillo?

@tuxillo
Copy link
Author

tuxillo commented Jun 8, 2018

Sorry, will try to get this done by this weekend!

@withinboredom yeah, it's for legacy adapters in OS that don't have support for the hyperv integration services.

@tuxillo
Copy link
Author

tuxillo commented Jun 24, 2018

When trying to build I get this error:

C:\Users\vagrant\go\src\github.com\hashicorp\packer>go build
go build github.com/hashicorp/packer/vendor/github.com/oracle/oci-go-sdk/core: 
C:\Go\pkg\tool\windows_amd64\compile.exe: fork/exec C:\Go\pkg\tool\windows_amd64\compile.exe: The filename or extension is too long.

After a quick google search I found: golang/go#18468 and golang/go#25292

I'm using Go 1.10.3, but it seems it'll be fixed in 1.11 and to get the fix you have cherry-pick the fix and build Go, or am I missing something?

@SwampDragons
Copy link
Contributor

That sounds about right. Sorry for that build error. It's a known issue with windows.

@SwampDragons
Copy link
Contributor

Go 1.11 has just been released so hopefully your build woes will be better. https://groups.google.com/forum/#!msg/golang-announce/O7POXMK3xbM/Ia8ObPyWDgAJ

@SwampDragons
Copy link
Contributor

I'm going to close this PR since it's been inactive for so long; if you end up working more on this issue, just open a new one when you're ready. :)

@ghost
Copy link

ghost commented Mar 30, 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 Mar 30, 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.

None yet

6 participants