-
Notifications
You must be signed in to change notification settings - Fork 765
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
bridge: various fixes #62
Conversation
Is there any way we can add test coverage for these changes? I've been thinking about refactoring this plugin, but its scarier to attempt when there are important features that might break while the tests remain green. |
Yeah, good point. Most of this should be testable. |
* Don't set the MAC, send gratuitous arp instead * Set the bridge's MAC to itself * Only disable DAD when necessary
@rosenhouse added a test for the "stable mac" case. Not sure how to reasonably test the other changes. |
@squeed looks like it went red |
Once #56 is merged we should see fewer test failures. A lot of these tests are dancing around with namespaces, making them non-deterministic. |
Yeah, I don't see how to add functional tests either. (all the more reason to refactor to a more dependency-injected style where we can have unit tests of the application-object). LGTM |
…istency-openshift-4.11-ose-containernetworking-plugins Updating ose-containernetworking-plugins images to be consistent with ART
Fixes: #60 #59 #52