Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Change NO_MASQ_LOCAL option to default ON #3756

Merged
merged 3 commits into from
Aug 4, 2020
Merged

Change NO_MASQ_LOCAL option to default ON #3756

merged 3 commits into from
Aug 4, 2020

Conversation

murali-reddy
Copy link
Contributor

Fixes #3389

@bboreham bboreham changed the title add NO_MASQ_LOCAL environment variable to weave kube daemonset's add NO_MASQ_LOCAL=1 environment variable to weave kube daemonsets Jan 21, 2020
@bboreham bboreham mentioned this pull request Jan 21, 2020
Closed
bboreham
bboreham approved these changes Jan 21, 2020
View reviewed changes
Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

This is a change in behaviour, possibly breaking for some people, but on balance probably the right thing to do.

@bboreham
Copy link
Contributor

I just had a thought that we could make the script default ON unless it sees NO_MASQ_LOCAL=0.
This would simplify configuration and tie the change to a code release, rather than config.

@bboreham bboreham modified the milestones: 2.6.1, 2.7 Feb 26, 2020
@murali-reddy
Copy link
Contributor Author

This would simplify configuration and tie the change to a code release, rather than config.

updated PR to be code only change

@bboreham bboreham changed the title add NO_MASQ_LOCAL=1 environment variable to weave kube daemonsets Change NO_MASQ_LOCAL option to default ON Mar 18, 2020
bboreham
bboreham approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

Great!

@bboreham
Copy link
Contributor

I think we should change the docs

weave/site/kubernetes/kube-addon.md

Line 406 in 9f00f78

* `NO_MASQ_LOCAL` - set to 1 to preserve the client source IP address when

What about test 840, which is setting the variable?

@bboreham bboreham merged commit 5f27dbf into master Aug 4, 2020
@bboreham bboreham deleted the issue-3389 branch August 4, 2020 16:42
@bboreham bboreham mentioned this pull request Aug 27, 2020
Merged
@HeroCC HeroCC mentioned this pull request Jan 4, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bboreham bboreham bboreham approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.7
Development

Successfully merging this pull request may close these issues.

Feature request: Please make NO_MASQ_LOCAL=1 the default setting
2 participants
@murali-reddy @bboreham