Skip to content

Add test for net=container and links#11686

Merged
icecrime merged 2 commits intomoby:masterfrom
willhf:9340_test
Apr 6, 2015
Merged

Add test for net=container and links#11686
icecrime merged 2 commits intomoby:masterfrom
willhf:9340_test

Conversation

@willhf
Copy link
Contributor

@willhf willhf commented Mar 24, 2015

closes #9340
In writing a test for this we discovered that the check was wrong, hence the prefix change.

Signed-off-by: willhf <willhf@gmail.com>
@cpuguy83
Copy link
Member

LGTM

@duglin
Copy link
Contributor

duglin commented Mar 24, 2015

Do we need to modify the code at:
https://github.com/willhf/docker/blob/9340_test/runconfig/parse.go#L136

@estesp
Copy link
Contributor

estesp commented Mar 24, 2015

we should consider the merged #11369 and the proposed #11516 before taking on a test for something we may decide is a supported config now

@icecrime
Copy link
Contributor

icecrime commented Apr 2, 2015

@willhf Can you please update your PR addressing @duglin's comment? Thanks!

Signed-off-by: willhf <willhf@gmail.com>
@willhf
Copy link
Contributor Author

willhf commented Apr 6, 2015

@icecrime done!
Sorry for the delay.

@icecrime
Copy link
Contributor

icecrime commented Apr 6, 2015

@estesp True, but until we figure this out, don't you think this is a valid patch?

I'm thinking LGTM for now.

@estesp
Copy link
Contributor

estesp commented Apr 6, 2015

@icecrime yes, actually, let's merge this as the other work isn't really complete, and it is correct that even with #11369, it is still correct that both flags together do not work (see the @cpuguy83 comment here: #9340 (comment))

Given that, I'm good with merging this now and then understanding how we appropriately finish out the updated capability that you can link to another container started with --net=container:something.

LGTM

@icecrime
Copy link
Contributor

icecrime commented Apr 6, 2015

No docs needs as far as I can tell.

icecrime pushed a commit that referenced this pull request Apr 6, 2015
Add test for net=container and links
@icecrime icecrime merged commit 8a50746 into moby:master Apr 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--link should be disallowed with --net=container

6 participants