Skip to content

fix #21745, preserve container connectivity when the bridge changes#22850

Merged
fpletz merged 1 commit intoNixOS:masterfrom
kampfschlaefer:feature/21745-fix_networking_restart
Mar 26, 2017
Merged

fix #21745, preserve container connectivity when the bridge changes#22850
fpletz merged 1 commit intoNixOS:masterfrom
kampfschlaefer:feature/21745-fix_networking_restart

Conversation

@kampfschlaefer
Copy link
Copy Markdown
Contributor

Motivation for this change

When a container has interfaces added to bridges on the host, there are changes to the host where the interface is removed from the bridge during switch-to-configuration as the bridge is stopped and restarted.

This tries to make the bridge only reload and preserve as much as possible.

I don't yet like the file in /run/.interfaces to remember which interfaces to remove when reloading. But we have to save this somewhere and not just remove all interfaces and re-add them as that would add removed devices. And we can't remove all enslaved devices and only add those configured as that would drop all containers from the bridge…

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • [-] Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • [-] Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@fpletz
Copy link
Copy Markdown
Member

fpletz commented Feb 21, 2017

👍 on the approach. I think something along these lines is the only viable solution.

@volth I also did not test it yet, but the code definitely fixes your first issue. Regarding the second, I'm not sure if the unit will be restarted or reloaded in that case.

We could also move @kampfschlaefer's code from reload to postStop and save all interfaces not part of the previous nixos config. When the unit is started again, those interfaces should be added along with the configured ones.

@fpletz fpletz added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Feb 21, 2017
@fpletz fpletz added this to the 17.03 milestone Feb 21, 2017
@fpletz fpletz self-assigned this Mar 25, 2017
@fpletz
Copy link
Copy Markdown
Member

fpletz commented Mar 26, 2017

Ok, as we didn't get any other feedback and I did some testing, I think we will go with this approach.

It's a bit sad that this won't work when changing the status of RSTP, but documenting this fact will suffice IMHO.

@kampfschlaefer
Copy link
Copy Markdown
Contributor Author

Actually, once you have one bridge with stp enabled, changing the stp state on the other interfaces should not be a problem anymore. Because then the rstp daemon is already running and all is well.

And adopt the tests to add an interface and remove it again.

It should work when deactivating rstp, it will not work when activating
rstp for the first bridge as then the userspace daemon is not yet
available. But once one bridge is active with stp, it should work with
the reload for any further bridge.

Fixes NixOS#21745. Also see NixOS#22547.
@fpletz fpletz force-pushed the feature/21745-fix_networking_restart branch from 8e21c99 to 6872995 Compare March 26, 2017 16:48
@fpletz
Copy link
Copy Markdown
Member

fpletz commented Mar 26, 2017

Thanks!

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

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants