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

add networking_config to container create fixes #51723 #51724

Merged
merged 4 commits into from Mar 21, 2019

Conversation

@clinta
Copy link
Contributor

commented Feb 20, 2019

What does this PR do?

This sets the networking options in the docker_container.running state

What issues does this PR fix or reference?

#51723

Tests written?

Yes

Commits signed with GPG?

Yes

Copy link
Contributor

left a comment

@clinta It looks like a few docker related tests failed, please have a look.

@clinta

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2019

Fixed the tests by making sure this option is only set for for the network specified by network_mode.

If this looks good I can squash the commits.

@clinta clinta force-pushed the clinta:docker-create-with-ip branch 2 times, most recently from 3d3e074 to 3041782 Feb 23, 2019
@dwoz

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2019

@clinta Looks good now but it would be ideal if you could also add a test for this issue to ensure there are no regressions in the future.

@clinta

This comment has been minimized.

Copy link
Contributor Author

commented Feb 24, 2019

I will try, I've been having a heck of a time getting the tests to even run locally though. I discovered that these docs are no longer relevant. Is there an updated guide to salt tests somewhere else?

@clinta clinta force-pushed the clinta:docker-create-with-ip branch 6 times, most recently from f534c03 to 092922d Mar 1, 2019
@clinta

This comment has been minimized.

Copy link
Contributor Author

commented Mar 1, 2019

I never could get the tests running locally, but after abusing the jenkins server I've got a test demonstrating the issue in 092922d and fixed in 408d167.

@dwoz dwoz self-requested a review Mar 5, 2019
clinta added 2 commits Mar 1, 2019
@clinta clinta force-pushed the clinta:docker-create-with-ip branch from 408d167 to c6e553c Mar 21, 2019
@clinta

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2019

This is freshly rebased with all CI checks passing.

twangboy added 2 commits Mar 21, 2019
@twangboy twangboy merged commit d2fd84f into saltstack:2018.3 Mar 21, 2019
10 checks passed
10 checks passed
WIP Ready for review
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint Python lint test has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py2-windows-2016 The py2-windows-2016 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.