Remove code that sets lo to down prior ns deletion #104
Conversation
Signed-off-by: Michael Zappa <[email protected]>
|
@MikeZappa87: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
added
dco-signoff: yes
| if err := tearDownLoopback(podNetwork.NetNS); err != nil { | ||
| // ignore error | ||
| logrus.Warningf("Ignoring error tearing down loopback interface: %v", err) | ||
| } |
There was a problem hiding this comment.
@dcbw do you have more context on that and if it's safe to remove?
There was a problem hiding this comment.
This is doing effectively (including the ns setup)
Pod Setup:
ip netns add ns0
ip netns exec ns0 ip link set lo up
Pod Tear down:
ip netns exec ns0 ip link set lo down
ip netns del ns0
We are just skipping the 'ip link set lo down' piece
In the end, the network namespace is deleted and no frames are sent between the host/ns with the state of lo changing
There was a problem hiding this comment.
@saschagrunert I can't think of a reason to bother setting it down, since we're gonna tear the netns down immediately after anyway.
There was a problem hiding this comment.
Good, thank you for the explanation :)
dcbw
removed
the
do-not-merge/release-note-label-needed
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dcbw, MikeZappa87 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
Pull Request Test Coverage Report for Build 2236793634
💛 - Coveralls |
I am not sure this code is needed since deleting the network namespace takes care of lo. If this was done for a specific reason I am curious to hear about it :-) It looks like it was an attempt to duplicate the behavior of the loopback plugin.
I can apply a deprecation flag to tearDownLoopback func if that is preferred
Signed-off-by: Michael Zappa [email protected]