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

Drop traffic from docker0 bridge to weave bridge #704

Merged
merged 3 commits into from
May 20, 2015

Conversation

bboreham
Copy link
Contributor

Fixes #620, as discussed in comments there. All smoke tests still pass.

@bboreham
Copy link
Contributor Author

Hang on, need to amend the smoke test to check that this actually works

@bboreham
Copy link
Contributor Author

OK, have extended the smoke test - had to kludge the exec_on thing a bit to get the negation in the right place.

@rade
Copy link
Member

rade commented May 19, 2015

Why is the kludge necessary? Would wrapping in a sh -c execution do the trick?

@rade
Copy link
Member

rade commented May 19, 2015

Alternatively, assert_raises "exec_on $HOST1 $PING $C1" 1 should work. No need to create a helper function for it, since there is just one call site.

@rade
Copy link
Member

rade commented May 19, 2015

Except once you've rebased from master there will be three call sites, so an exec_on1_fail would probably still be a good idea.

@rade
Copy link
Member

rade commented May 19, 2015

Hmm, there's a problem with both your existing exec_on1_n and my proposed exec_on1_fail - they can fail (and hence report success) for entirely the wrong reasons. That's why I went for the 'negate the result of the specific check we expect to fail' approach elsewhere. So perhaps a sh -c is the way to go after all.

bboreham added 2 commits May 20, 2015 10:27
Note that we have to wrap the '!' in 'sh -c' to get the negation to work as expected.
@bboreham
Copy link
Contributor Author

Changed to sh -c and rebased

# exec_on1 "c5 ! $PING $C3"
exec_on1 "c3 sh -c \"! $PING $C1\""
exec_on1 "c5 sh -c \"! $PING $C1\""
exec_on1 "c5 sh -c \"! $PING $C3\""

This comment was marked as abuse.

@bboreham
Copy link
Contributor Author

Annularized

@rade rade added this to the 0.11.0 milestone May 20, 2015
rade added a commit that referenced this pull request May 20, 2015
Drop traffic from docker0 bridge to weave bridge

Fixes #620.
@rade rade merged commit f642db0 into weaveworks:master May 20, 2015
@rade
Copy link
Member

rade commented May 21, 2015

I've just a) committed a change for simplicity and consistency.

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

Successfully merging this pull request may close these issues.

weave expose breaks subnet isolation
2 participants