nixos/kubernetes: Patch kube-proxy for iptables 1.6.2 compatibility#36739
Closed
srhb wants to merge 7 commits intoNixOS:masterfrom
Closed
nixos/kubernetes: Patch kube-proxy for iptables 1.6.2 compatibility#36739srhb wants to merge 7 commits intoNixOS:masterfrom
srhb wants to merge 7 commits intoNixOS:masterfrom
Conversation
Member
|
@GrahamcOfBorg build kubernetes |
|
Failure on x86_64-darwin (full log) Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Partial log (click to expand)
|
|
Failure on x86_64-linux (full log) Partial log (click to expand)
|
Contributor
Author
|
@Mic92 Any idea how to make grahamcofborg run the tests as well? I can't seem to figure out the right command, something appears to be different for the kubernetes test set (and it's not running on Hydra either, as far as I can see.) |
Contributor
Author
|
Hm, I'll look into those failures. Didn't see those locally. |
Contributor
Author
|
Weird. |
Contributor
Author
|
@GrahamcOfBorg build kubernetes |
|
Failure on x86_64-darwin (full log) Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Partial log (click to expand)
|
Member
|
@GrahamcOfBorg test tests.kubernetes.dns.singlenode |
|
Failure on x86_64-linux (full log) Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Partial log (click to expand)
|
Mic92
reviewed
Mar 10, 2018
nixos/release.nix
Outdated
| tests.kubernetes.dns = callSubTestsOnTheseSystems ["x86_64-linux"] tests/kubernetes/dns.nix {}; | ||
| ## kubernetes.e2e should eventually replace kubernetes.rbac when it works | ||
| #tests.kubernetes.e2e = callSubTestsOnTheseSystems ["x86_64-linux"] tests/kubernetes/e2e.nix {}; | ||
| tests.kubernetes.rbac = callSubTestsOnTheseSystems ["x86_64-linux"] tests/kubernetes/rbac.nix {}; |
Member
There was a problem hiding this comment.
Let's remove some abstractions for our own sanity.
Member
|
@GrahamcOfBorg eval |
Mic92
reviewed
Mar 10, 2018
Contributor
Author
|
@Mic92 Great, thanks! Good to merge? |
Member
|
I only fixed running tests from |
|
Success on aarch64-linux (full log) Attempted: tests.kubernetes.dns, tests.kubernetes.rbac No log is available. |
|
Success on x86_64-linux (full log) Attempted: tests.kubernetes.dns, tests.kubernetes.rbac No log is available. |
Member
|
Also without tests it should be fine. Please also backport it to 18.03. |
Member
|
@grahamc is the log output of tests above legit? Maybe it points to wrong key? |
Member
|
This is so weird. Let's try again :o @GrahamcOfBorg test kubernetes.dns kubernetes.rbac |
|
Success on aarch64-linux (full log) Attempted: tests.kubernetes.dns, tests.kubernetes.rbac No log is available. |
|
Success on x86_64-linux (full log) Attempted: tests.kubernetes.dns, tests.kubernetes.rbac No log is available. |
grahamc
reviewed
Mar 15, 2018
nixos/release.nix
Outdated
| tests.kubernetes.dns = callSubTestsOnTheseSystems ["x86_64-linux"] tests/kubernetes/dns.nix {}; | ||
| ## kubernetes.e2e should eventually replace kubernetes.rbac when it works | ||
| #tests.kubernetes.e2e = callSubTestsOnTheseSystems ["x86_64-linux"] tests/kubernetes/e2e.nix {}; | ||
| tests.kubernetes.rbac = callSubTestsOnTheseSystems ["x86_64-linux"] tests/kubernetes/rbac.nix {}; |
Member
There was a problem hiding this comment.
Here is the problem, callSubTestsOnTheseSystems is expecting an attributeset of tests inside the file, but there is none. Thus this should probably be using callTestOnTheseSystems, but when I made the simple patch:
diff --git a/nixos/release.nix b/nixos/release.nix
index cc4c3e7c705..a26d4cee746 100644
--- a/nixos/release.nix
+++ b/nixos/release.nix
@@ -296,10 +296,10 @@ in rec {
tests.kernel-copperhead = callTest tests/kernel-copperhead.nix {};
tests.kernel-latest = callTest tests/kernel-latest.nix {};
tests.kernel-lts = callTest tests/kernel-lts.nix {};
- tests.kubernetes.dns = callSubTestsOnTheseSystems ["x86_64-linux"] tests/kubernetes/dns.nix {};
+ tests.kubernetes.dns = callTestOnTheseSystems ["x86_64-linux"] tests/kubernetes/dns.nix {};
## kubernetes.e2e should eventually replace kubernetes.rbac when it works
#tests.kubernetes.e2e = callSubTestsOnTheseSystems ["x86_64-linux"] tests/kubernetes/e2e.nix {};
- tests.kubernetes.rbac = callSubTestsOnTheseSystems ["x86_64-linux"] tests/kubernetes/rbac.nix {};
+ tests.kubernetes.rbac = callTestOnTheseSystems ["x86_64-linux"] tests/kubernetes/rbac.nix {};
tests.latestKernel.login = callTest tests/login.nix { latestKernel = true; };
tests.ldap = callTest tests/ldap.nix {};
#tests.lightdm = callTest tests/lightdm.nix {};I got:
grahamc@Morbo> HOME=/homeless-shelter NIX_PATH=nixpkgs=$(pwd) nix-instantiate ./nixos/release.nix -A tests.kubernetes.rbac --option restrict-eval true --option build-timeout 1800 --arg supportedSystems '["x86_64-linux"]' --show-trace
error: while evaluating anonymous function at /home/grahamc/projects/nixpkgs/nixos/release.nix:23:8, called from /home/grahamc/projects/nixpkgs/lib/attrsets.nix:282:43:
while evaluating ‘hydraJob’ at /home/grahamc/projects/nixpkgs/lib/customisation.nix:162:14, called from /home/grahamc/projects/nixpkgs/nixos/release.nix:23:16:
while evaluating the attribute ‘system’ at /home/grahamc/projects/nixpkgs/lib/customisation.nix:167:24:
attribute ‘system’ missing, at /home/grahamc/projects/nixpkgs/lib/customisation.nix:167:10
and I don't have the time at this moment to debug it. Maybe you can take a look?
Contributor
Author
|
@GrahamcOfBorg test kubernetes.dns.singlenode kubernetes.dns.multinode kubernetes.rbac.singlenode kubernetes.rbac.multinode |
|
No attempt on aarch64-linux The following builds were skipped because they don't evaluate on aarch64-linux: tests.kubernetes.dns.singlenode, tests.kubernetes.dns.multinode, tests.kubernetes.rbac.singlenode, tests.kubernetes.rbac.multinode No log is available. |
|
No attempt on x86_64-linux The following builds were skipped because they don't evaluate on x86_64-linux: tests.kubernetes.dns.singlenode, tests.kubernetes.dns.multinode, tests.kubernetes.rbac.singlenode, tests.kubernetes.rbac.multinode No log is available. |
Contributor
Author
Looks good to me. @Mic92 Should I keep in the changes you made to release.nix or not? It seems they do nothing for ofborg, not sure about Hydra. |
Member
|
These tests won't run on Hydra, just like they don't run on OfBorg. I need to publish logs as to why they don't evaluate obviously, but if you try them like I showed you'll see why: In other words, the test seems to be using IFD which is not permitted. |
Member
|
Specifically this is the problem, in certs.nix: the readFile is not permitted. |
Member
|
I'm almost done fixing the test to be able to run on Hydra and OfBorg. |
Member
|
The following patch seems to cause the tests to break, but is the right way to go to have this no longer violate IFD issues: diff --git a/nixos/tests/kubernetes/certs.nix b/nixos/tests/kubernetes/certs.nix
index d3eff910c46..097488c46e0 100644
--- a/nixos/tests/kubernetes/certs.nix
+++ b/nixos/tests/kubernetes/certs.nix
@@ -7,11 +7,26 @@
}:
let
runWithCFSSL = name: cmd:
- builtins.fromJSON (builtins.readFile (
- pkgs.runCommand "${name}-cfss.json" {
- buildInputs = [ pkgs.cfssl ];
- } "cfssl ${cmd} > $out"
- ));
+ let secrets = pkgs.runCommand "${name}-cfss.json" {
+ buildInputs = [ pkgs.cfssl pkgs.jq ];
+ outputs = [ "out" "cert" "key" "csr" ];
+ }
+ ''
+ (
+ echo "${cmd}"
+ cfssl ${cmd} > tmp
+ cat tmp | jq -r .key > $key
+ cat tmp | jq -r .cert > $cert
+ cat tmp | jq -r .csr > $csr
+
+ touch $out
+ ) 2>&1 | fold -w 80 -s
+ '';
+ in {
+ key = secrets.key;
+ cert = secrets.cert;
+ csr = secrets.csr;
+ };
writeCFSSL = content:
pkgs.runCommand content.name {
@@ -25,10 +40,10 @@ let
noCSR = content: pkgs.lib.filterAttrs (n: v: n != "csr") content;
noKey = content: pkgs.lib.filterAttrs (n: v: n != "key") content;
- writeFile = content: pkgs.writeText "content" (
- if pkgs.lib.isAttrs content then builtins.toJSON content
- else toString content
- );
+ writeFile = content:
+ if pkgs.lib.isDerivation content
+ then content
+ else pkgs.writeText "content" (builtins.toJSON content);
createServingCertKey = { ca, cn, hosts? [], size ? 2048, name ? cn }:
noCSR (
|
Member
|
@srhb can you add this? |
8 tasks
Contributor
Author
|
Honestly, this PR is a complete mess now. I'd much rather focus on getting the patch that unbreaks kube-proxy merged and have a separate PR or issue regarding tests being broken on Hydra and ofborg. Thanks for all your input. |
8 tasks
srhb
added a commit
to srhb/nixpkgs
that referenced
this pull request
Mar 17, 2018
Contributor
Author
|
OK, all the test changes are now in #37199 and removed from this PR. This PR should be good to merge in order to fix kubernetes networking. I doubt I'll be able to finish making the test non-IFD-compatible. I've spent a few hours trying to track down the (last?) stray issue(s) without luck. |
Contributor
|
Applied in e8cbd44 -- thanks! |
Contributor
|
Thanks a lot for the patch. Is there an ETA for this patch to get backported to 18.03? I am hitting this issue in a new install. |
Contributor
|
Hey @itorres, |
globin
pushed a commit
to mayflower/nixpkgs
that referenced
this pull request
May 24, 2018
Closes NixOS#36739 (cherry picked from commit e8cbd44)
globin
pushed a commit
to mayflower/nixpkgs
that referenced
this pull request
May 26, 2018
Closes NixOS#36739 (cherry picked from commit e8cbd44)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation for this change
Fixes #35544
Things done
Upstream patch to insert the missing space in the "-w 5" argument to iptables-restore included. This fixes kube-proxy.
This should be backported to 18.03
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)