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

Setup the bond interfaces with proper default routes #2479

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@TheOriginalAlex
Copy link
Contributor

TheOriginalAlex commented Sep 2, 2017

We need to add the addresses first and then set the routes, that way if the private ip gets added last, we're not setting the private ip as the default route
And using netlink.Bond to create the bondded interface is more readable, I think.

Signed-off-by: Alex Johnson hello@alex-johnson.net

Fixes #2446

- What I did
Cleaned up the packet metadata.

- How I did it
Created the bond0 interface using the netlink.Bond struct.
Add addresses and then setup routes in two discreet loops, that way the private IP doesn't end up being the last route that's added.

- How to verify it
Run on a Packet Type 0 server.

- Description for the changelog

Cleaned up and fixed Packet metadata/bonding

- A picture of a cute animal (not mandatory but encouraged)
Cute Dolphin

Setup the bond interfaces with proper default routes
We need to add the addresses first and then set the routes, that way if the private ip gets added last, we're not setting the private ip as the default route
And using netlink.Bond to create the bondded interface is more readable, I think.

Signed-off-by: Alex Johnson <hello@alex-johnson.net>
@justincormack

This comment has been minimized.

Copy link
Collaborator

justincormack commented Sep 2, 2017

re "- How to verify it - Run on a Packet Type 0 server." you need to run on type 2 as well. ANd in particular check /sys/class/net/bond0/bonding/mode is correct.

newBond.Mode = netlink.BondMode(ni.BondingMode())

if err := netlink.LinkAdd(newBond); err != nil {
if !strings.Contains(err.Error(), "file exists") {

This comment has been minimized.

@justincormack

justincormack Sep 2, 2017

Collaborator

you should be able to check if the error is EEXIST without using strings.Contains.

I still dont understand why netlink gives this error...

} else {
return fmt.Errorf("Could not get link by name for bond0: %s", err)
}
}

This comment has been minimized.

@justincormack

justincormack Sep 2, 2017

Collaborator

it would make sense to print a message saying "using existing bond0 interface" in this case.

@justincormack

This comment has been minimized.

Copy link
Collaborator

justincormack commented Oct 4, 2017

@TheOriginalAlex hi, have you had a chance to test this on type 2? I have been a bit busy so I haven't yet, will try to get to it in a while...

@justincormack

This comment has been minimized.

Copy link
Collaborator

justincormack commented Nov 14, 2017

I am working on an updated version of this.

@TheOriginalAlex

This comment has been minimized.

Copy link
Contributor

TheOriginalAlex commented Nov 14, 2017

Sure! Sorry, I haven't responded much. Been busy. Also, I got stuck on getting the default gateway to work for some reason. Running the binary directly on the host worked, but wouldn't work inside the metadata container even though the metadata container has CAP_NET_ADMIN and /dev is mounted in it.

@justincormack

This comment has been minimized.

Copy link
Collaborator

justincormack commented Nov 17, 2017

@TheOriginalAlex what problems were you seeing with the default gateway?

@TheOriginalAlex

This comment has been minimized.

Copy link
Contributor

TheOriginalAlex commented Nov 18, 2017

It just wasn't getting set. If I printed the output of nsenter.RouteList, I could see it, but it wasn't showing up if I ran ip routes show on the shell.

@justincormack

This comment has been minimized.

Copy link
Collaborator

justincormack commented Nov 20, 2017

Routes seemed to be working for me ok, but I need to rebase against the new metadata package again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment