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

handle gracefully add/delete of vethwedu dummy interface #3459

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

murali-reddy
Copy link
Contributor

@murali-reddy murali-reddy commented Nov 23, 2018

use LinkAddIfNotExist to add dummy interface, and delete dummy interface in defer block

fixes #3414

net/bridge.go Outdated Show resolved Hide resolved
net/bridge.go Outdated
return errors.Wrap(err, "creating dummy interface")
}
defer func() {
var dummyIf netlink.Link
dummyIf, err = netlink.LinkByName(WeaveDummyIfName)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we just use dummy here ?

net/bridge.go Outdated
return errors.Wrap(err, "creating dummy interface")
}
defer func() {
var dummy netlink.Link
dummy, err = netlink.LinkByName(WeaveDummyIfName)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we just re-use the dummy variable that was initialized earlier, instead of creating a new one?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you don't even have to assign to it - the initialization earlier leaves dummy in a state where you can just use it. Unless I'm missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. I changed the logic to re-use dummy. PTAL

return errors.Wrap(err, "creating dummy interface")
}
defer func() {
if err = netlink.LinkDel(dummy); err != nil {
err = errors.Wrap(err, "deleting dummy interface")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think assigning to the local variable err at this point has no effect.
If err is made a named return value from initPrep it would work, but suppose both LinkSetMTU and LinkDel fail, we'd only see one error.
Maybe use a different err to record the LinkDel() result and use Wrap() to add the previous error (if any).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, Wrap() can only wrap one error. Maybe just add the text of this one to the previous one, if it exists.

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 Net Daemonset fails to restart pod due to existing dummy interface
2 participants