Skip to content

nixosTests.flannel: port to python, unbreak #85252

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 15, 2020
Merged

nixosTests.flannel: port to python, unbreak #85252

merged 1 commit into from
Apr 15, 2020
+19 −18

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Apr 14, 2020

Motivation for this change

The flannel test was broken and still written in Perl.

For reasons yet unknown, the vxlan backend doesn't work (at least inside
the qemu networking), so this is moved to the udp backend.

Note changing the backend apparently also changes the interface name,
it's now flannel0, not flannel.1

fixes #74941
Python VM Test Migration: #72828

@johanot do you have an idea why it's broken inside qemu's networking?

#55649 suggests it once worked, so I'm curious.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@tfc tfc mentioned this pull request Apr 14, 2020
Closed
@flokli
Copy link
Contributor Author

flokli commented Apr 14, 2020

This seems to be pretty trivial to backport to 20.03 as well.

ZHF: #80379

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Apr 14, 2020
@flokli
Copy link
Contributor Author

flokli commented Apr 14, 2020

@GrahamcOfBorg test flannel

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Apr 14, 2020
@flokli
nixosTests.flannel: port to python, unbreak
28ef438 
For reasons yet unknown, the vxlan backend doesn't work (at least inside
the qemu networking), so this is moved to the udp backend.

Note changing the backend apparently also changes the interface name,
it's now `flannel0`, not `flannel.1`

fixes NixOS#74941
@flokli flokli force-pushed the nixos-flannel-fix branch from e7cbe7d to 28ef438 Compare April 14, 2020 21:56
@ofborg ofborg bot removed the 8.has: module (update) This PR changes an existing module in `nixos/` label Apr 14, 2020
@flokli
Copy link
Contributor Author

flokli commented Apr 14, 2020

@GrahamcOfBorg test flannel

@flokli flokli requested a review from tfc April 15, 2020 11:59
tfc
tfc reviewed Apr 15, 2020
View reviewed changes
nixos/tests/flannel.nix
node1.wait_until_succeeds("ip l show dev flannel0")
ip1 = node1.succeed("ip -4 addr show flannel0 | grep -oP '(?<=inet).*(?=/)'")
node2.wait_until_succeeds("ip l show dev flannel0")
ip2 = node2.succeed("ip -4 addr show flannel0 | grep -oP '(?<=inet).*(?=/)'")
Copy link
Contributor

Choose a reason for hiding this comment

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

up until here both machines do not communicate with each other yet, right? But the code is 100% the same if i didn't misread anything. Can you deduplicate the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code is the same as before - I just added a step to wait for the interface to show up, as it sometimes wasn't there yet. We need to look up the other machines's IP address to do the pinging.

johanot
johanot approved these changes Apr 15, 2020
View reviewed changes
Copy link
Contributor

@johanot johanot left a comment

Choose a reason for hiding this comment

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

LGTM. Test run clean on my machine. Sounds like vxlan-over-qemu-fabric might be to blame for the flakiness. I image a ton of plausible race-conditions in that stack :)

@flokli flokli merged commit 7835641 into NixOS:master Apr 15, 2020
@flokli flokli deleted the nixos-flannel-fix branch April 15, 2020 18:01
@flokli
Copy link
Contributor Author

flokli commented Apr 15, 2020

Thanks for looking into it. I'd still love if we could somehow test vxlan (to ensure more production-grade deployments work), but I agree having a flannel test with another backend is preferrable over a totally broken one.

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

@tfc tfc

@arianvp arianvp

@johanot johanot

@worldofpeace worldofpeace

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

nixos/tests/flannel fails
4 participants
@flokli @tfc @arianvp @johanot