Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Don't fail when there are unsupported net interfaces in the container #146

Merged
merged 2 commits into from
Jul 12, 2019

Conversation

luxas
Copy link
Contributor

@luxas luxas commented Jul 12, 2019

Fixes #133
(I believe)

The problem was that when ignite tried to parse the networking for the following container...

$ docker run -it busybox ip addr
...
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
2: sit0@NONE: <NOARP> mtu 1480 qdisc noop qlen 1000
    link/sit 0.0.0.0 brd 0.0.0.0
8: eth0@if9: <BROADCAST,MULTICAST,UP,LOWER_UP,M-DOWN> mtu 1500 qdisc noqueue
    link/ether 02:42:ac:11:00:02 brd ff:ff:ff:ff:ff:ff
    inet 172.17.0.2/16 brd 172.17.255.255 scope global eth0
       valid_lft forever preferred_lft forever
    inet 192.168.252.10 peer 192.168.252.9/32 scope global tun0
       valid_lft forever preferred_lft forever
    inet6 fe80::5eae:1d4:2b4b:ee4/64 scope link stable-privacy
       valid_lft forever preferred_lft forever

...it tried to find the address of the first interface sit0@NONE, but that failed (obviously). The problem was that the networkSetup function did not try the other interfaces, just retry and do the same thing after one more second.

This PR fixes so that ignite tries all network interfaces.

/cc @twelho @danielcb

@luxas luxas added this to the v0.4.1 milestone Jul 12, 2019
@twelho
Copy link
Contributor

twelho commented Jul 12, 2019

This will work fine for the first "pass" of the interfaces, where it checks every one, skips invalid ones and adds valid ones, but if a valid interface is not ready yet, it will be skipped if some other valid interface happens to be ready (as interfacesCount becomes > 0).

I suggest if encountering an invalid interface, you add it to ignoreInterfaces and set the retry flag to retry the loop again, until a pass is encountered where no interfaces got added to ignored.

You should also check the boolean in takeAddress, which signals if an address can be retrieved from the interface. If not, that calls for a retry (without ignoring the interface). It's not perfect however, as that will get stuck if the interface has no address.

Was there an issue where ignite-spawn was fast enough to catch an interface before Docker gave it its address? Otherwise we could just use the no addresses flag to ignore the interface.

The correct way would be to retry the interfaces that are down, giving them time to go up first. All interfaces which are up and have an invalid address configuration should be ignored.

@luxas
Copy link
Contributor Author

luxas commented Jul 12, 2019

This will work fine for the first "pass" of the interfaces, where it checks every one, skips invalid ones and adds valid ones, but if a valid interface is not ready yet, it will be skipped if some other valid interface happens to be ready (as interfacesCount becomes > 0).

This means we would support multiple interfaces. IMO we don't at the moment. We should create an issue towards that end, and fix edge cases like this in all the places that might be relevant. Right now it's on a "best-effort" basis. Without this PR, one interface support is blocked (which is worse), hence I suggest merging this PR now, and following up on multiple interfaces once we're thought through that correctly and implemented tests to ensure it stays supported.

Was there an issue where ignite-spawn was fast enough to catch an interface before Docker gave it its address?

Yes.

@twelho
Copy link
Contributor

twelho commented Jul 12, 2019

OK, it works for now, but we need to come up with a better solution later to actually support multiple interfaces. I would still suggest checking interfaces without any addresses (the takeAddress boolean) and retrying them, as ignite-spawn might be fast enough to not detect any addresses for any interfaces without it, leaving the VM again without connection. If the interface has an address, but it's incorrect/invalid, just ignore it (by continuing and adding to ignoreInterfaces).

@twelho
Copy link
Contributor

twelho commented Jul 12, 2019

We'll worry about ongoing interface configs/proper multi-interface support later.

@luxas
Copy link
Contributor Author

luxas commented Jul 12, 2019

This all will be better with #85 fixed, too

@luxas
Copy link
Contributor Author

luxas commented Jul 12, 2019

We'll worry about ongoing interface configs/proper multi-interface support later.

Please create an issue with your thoughts about it and tag v0.5.0

@luxas luxas merged commit d2b2c6c into master Jul 12, 2019
@luxas luxas mentioned this pull request Jul 12, 2019
@twelho twelho deleted the log_network_issues branch July 12, 2019 16:58
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.

network setup failed: timed out waiting for the condition
2 participants