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

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
}
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
}
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

}
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

3 participants