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.

Sign up for GitHub

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

Jump to bottom

Documented CALICO_NAT_OUTGOING Parameter #775

Merged
merged 1 commit into from
Jun 21, 2017
Merged

Documented CALICO_NAT_OUTGOING Parameter #775

merged 1 commit into from
Jun 21, 2017

Conversation

VincentS
Copy link
Contributor

@VincentS VincentS commented May 10, 2017 •
edited by ozdanborne
Loading

Added documentation for CALICO_NAT_OUTGOING added by pull request #1640.

Fixes #516

@CLAassistant
Copy link

CLAassistant commented May 10, 2017 •
edited
Loading

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@VincentS VincentS mentioned this pull request May 10, 2017
Closed
@caseydavenport caseydavenport requested a review from tmjd May 10, 2017 17:22
@tmjd
Copy link
Member

tmjd commented May 10, 2017

This looks good but will need some updates after projectcalico/calicoctl#1640 is finalized.

@VincentS VincentS force-pushed the Document_CALICO_NAT_OUTGOING branch 2 times, most recently from 578bc92 to 712fefe Compare May 22, 2017 20:14
@tmjd tmjd mentioned this pull request May 26, 2017
Closed
tmjd
tmjd reviewed May 26, 2017
View reviewed changes
master/reference/node/configuration.md Outdated
@@ -27,6 +27,8 @@ The `calico/node` container is primarily configured through environment variable
| CALICO_IPV4POOL_CIDR | The IPv4 Pool to create if none exists at start up. It is invalid to define this variable and NO_DEFAULT_POOLS. | IPv4 CIDR | 192.168.0.0/16 |
| CALICO_IPV6POOL_CIDR | The IPv6 Pool to create if none exists at start up. It is invalid to define this variable and NO_DEFAULT_POOLS. | IPv6 CIDR | fd80:24e2:f998:72d6::/64 |
| CALICO_IPV4POOL_IPIP | IPIP Mode to use for the IPv4 POOL created at start up. | off, always, cross-subnet | off |
| CALICO_IPV4POOL_NAT_OUTGOING | Controls NAT Outgoing `nat-outgoing` for the IPv4 IPPOOL created at start up. | boolean | true |
Copy link
Member

Choose a reason for hiding this comment

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

We haven't referenced the specific flag in the Resource in other descriptions so I think you should remove the nat-outgoing from both of these entries.
nit: IPv4 IPPOOL should be IPv4 Pool

tmjd
tmjd reviewed May 26, 2017
View reviewed changes
master/reference/node/configuration.md
@@ -27,6 +27,8 @@ The `calico/node` container is primarily configured through environment variable
| CALICO_IPV4POOL_CIDR | The IPv4 Pool to create if none exists at start up. It is invalid to define this variable and NO_DEFAULT_POOLS. | IPv4 CIDR | 192.168.0.0/16 |
| CALICO_IPV6POOL_CIDR | The IPv6 Pool to create if none exists at start up. It is invalid to define this variable and NO_DEFAULT_POOLS. | IPv6 CIDR | fd80:24e2:f998:72d6::/64 |
| CALICO_IPV4POOL_IPIP | IPIP Mode to use for the IPv4 POOL created at start up. | off, always, cross-subnet | off |
Copy link
Member

Choose a reason for hiding this comment

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

While you're in here would you change POOL to Pool. Thanks

tmjd
tmjd reviewed May 26, 2017
View reviewed changes
master/reference/node/configuration.md Outdated
@@ -27,6 +27,8 @@ The `calico/node` container is primarily configured through environment variable
| CALICO_IPV4POOL_CIDR | The IPv4 Pool to create if none exists at start up. It is invalid to define this variable and NO_DEFAULT_POOLS. | IPv4 CIDR | 192.168.0.0/16 |
| CALICO_IPV6POOL_CIDR | The IPv6 Pool to create if none exists at start up. It is invalid to define this variable and NO_DEFAULT_POOLS. | IPv6 CIDR | fd80:24e2:f998:72d6::/64 |
| CALICO_IPV4POOL_IPIP | IPIP Mode to use for the IPv4 POOL created at start up. | off, always, cross-subnet | off |
| CALICO_IPV4POOL_NAT_OUTGOING | Controls NAT Outgoing `nat-outgoing` for the IPv4 IPPOOL created at start up. | boolean | true |
| CALICO_IPV6POOL_NAT_OUTGOING | Controls NAT Outgoing `nat-outgoing` for the IPv6 IPPOOL created at start up. | boolean | false |
Copy link
Member

Choose a reason for hiding this comment

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

same as above

tmjd
tmjd requested changes May 26, 2017
View reviewed changes
Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

A few changes please.

@caseydavenport caseydavenport added this to the Calico v2.4.0? milestone Jun 7, 2017
@VincentS VincentS force-pushed the Document_CALICO_NAT_OUTGOING branch 3 times, most recently from 80927d3 to 6fa64d1 Compare June 21, 2017 06:12
@VincentS VincentS mentioned this pull request Jun 21, 2017
Merged
2 tasks
@tmjd
Copy link
Member

tmjd commented Jun 21, 2017

@VincentS Could you please rebase this and get rid of that merge commit from me? I hit the appealing "Update branch" button and it created that merge commit, sorry I'd never used it before and need to remember to never use it again.
After the rebase then I think this is good.

@VincentS VincentS force-pushed the Document_CALICO_NAT_OUTGOING branch from 5121ac7 to cdba62e Compare June 21, 2017 21:01
@VincentS
Copy link
Contributor Author

Done!

@tmjd
Copy link
Member

tmjd commented Jun 21, 2017

LGTM
@robbrockbank What do you think?

@robbrockbank
Copy link
Contributor

LGTM

@robbrockbank robbrockbank merged commit aa07b6d into projectcalico:master Jun 21, 2017
song-jiang pushed a commit that referenced this pull request Jan 4, 2019
@tmjd
Merge pull request #775 from tigera/ets-aws-sg-fixes
9811847 
AWS SG: Fixes needed from testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmjd tmjd tmjd approved these changes

@robbrockbank robbrockbank robbrockbank approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Calico v2.4.0
Development

Successfully merging this pull request may close these issues.
5 participants
@VincentS @CLAassistant @tmjd @robbrockbank @caseydavenport