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

Allow user to specify default address pools for docker networks #818

Merged
merged 1 commit into from
May 8, 2018

Conversation

selansen
Copy link
Contributor

@selansen selansen commented Jan 19, 2018

This is separate commit for CLI files to address PR moby/moby#36054 moby/moby#36396

Signed-off-by: selansen [email protected]

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah changed the title Allow user to specify default address pools for docker networks [do not merge] Allow user to specify default address pools for docker networks Jan 19, 2018
@codecov-io
Copy link

codecov-io commented Jan 19, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@0ff5f52). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #818   +/-   ##
=========================================
  Coverage          ?   51.67%           
=========================================
  Files             ?      244           
  Lines             ?    15823           
  Branches          ?        0           
=========================================
  Hits              ?     8177           
  Misses            ?     7101           
  Partials          ?      545

Copy link
Contributor

@mdlinville mdlinville left a comment

Choose a reason for hiding this comment

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

Docs LGTM

@selansen selansen changed the title [do not merge] Allow user to specify default address pools for docker networks Allow user to specify default address pools for docker networks May 1, 2018
@@ -40,6 +40,7 @@ Options:
-D, --debug Enable debug mode
--default-gateway ip Container default gateway IPv4 address
--default-gateway-v6 ip Container default gateway IPv6 address
--default-address-pools Set the default address pools for local node networks
Copy link
Member

Choose a reason for hiding this comment

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

This should be singular now

Also, I think this page has an example daemon.json; can you update that one as well (with some example values)

man/dockerd.8.md Outdated
@@ -22,6 +22,7 @@ dockerd - Enable daemon mode
[**-D**|**--debug**]
[**--default-gateway**[=*DEFAULT-GATEWAY*]]
[**--default-gateway-v6**[=*DEFAULT-GATEWAY-V6*]]
[**--default-address-pools**[=*DEFAULT-ADDRESS-POOLS*]]
Copy link
Member

Choose a reason for hiding this comment

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

same here

@selansen
Copy link
Contributor Author

selansen commented May 1, 2018 via email

@selansen selansen force-pushed the master branch 2 times, most recently from 43c360e to e949b50 Compare May 4, 2018 17:21
@selansen
Copy link
Contributor Author

selansen commented May 4, 2018

I thought I pushed my changes but looks like I forgot to push it. updated @thaJeztah review comments.

@@ -1349,6 +1350,8 @@ This is a full example of the allowed configuration options on Linux:
]
}
}
"default-address-pools":[{"base":"172.80.0.0/16","size":24},
Copy link
Member

Choose a reason for hiding this comment

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

Looks like there's a comma missing in the JSON (it shows as invalid)

This is separate commit for CLI files to address PR 36054

Signed-off-by: selansen <[email protected]>
@selansen
Copy link
Contributor Author

selansen commented May 5, 2018

corrected it.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

ping @gbarr01 for additional documentation changes in other sections of the documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants