kubernetes: 1.9.7 -> 1.10.3 #41073

johanot · 2018-05-25T09:17:52Z

Motivation for this change

Update kubernetes module and package to 1.10.X.

This PR is a cleaned and rebased version of latest cherry-picks to #38445

This PR also updates kube-dns and kube-dashboard addons to latest versions:

kube-dns: 1.14.4 -> 1.14.10
kube-dashboard: 1.8.2 -> 1.8.3

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
- 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 nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

As shipped with k8s 1.10.3. Also: - updated the definition jsons as they are distributed in k8s. - updated the image uris as they are renamed in k8s - added imageDigest param as per 7368487

VMs were starving, many of the daemons were unable to complete their tasks resulting in tests failures. Turned off verbose output from k8s components as it consumes even more resources, and useful error messages actually drown in debug-clutter

johanot · 2018-05-25T09:22:56Z

@steveej @cstrahan @azazel75 @srhb

I cherry-picked @azazel75 commits for updating dashboard and dns, since they contain the manifest changes. Also, 1.5 gig mem seems to be enough for all tests to run smoothly.

I left out the etcd bump, since I don't think it belongs in this PR.

Hope y'all agree.

srhb · 2018-05-25T09:27:00Z

pkgs/applications/networking/cluster/kubernetes/default.nix

  };

-  # go > 1.10 should be fixed by https://github.com/kubernetes/kubernetes/pull/60373
+  # go > 1.10 should be fixed by https://github.com/kubernetes/kubernetes/pull/60597


This looks like it's already merged, but I think we should follow whichever version kube is built with upstream, which appears to be 1.9.3. Is there any place where this information is located that we can link to instead? Maybe the changelog?

According to https://github.com/kubernetes/community/blob/master/contributors/devel/development.md, minimum Golang version required for 1.10 is v1.9.1. And the official version states in External Dependencies for kubernetes 1.10 is v1.9.3: https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG-1.10.md

Let's link to the External Dependencies instead then.

srhb · 2018-05-25T09:29:10Z

nixos/modules/services/cluster/kubernetes/default.nix

@@ -339,9 +370,9 @@ in {
        type = types.str;
      };

-      admissionControl = mkOption {
+      enableAdmissionPlugins = mkOption {


What change does this require a user of admissionControl to make? If any, this should be documented in the release notes for 18.09

srhb · 2018-05-25T09:31:07Z

nixos/modules/services/cluster/kubernetes/dashboard.nix

@@ -113,8 +134,8 @@ in {
        };
        spec = {
          ports = [{
-            port = 80;
-            targetPort = 9090;
+            port = 443;


These port changes look like changes to user-facing behaviour, is that right? If so, we need a release notes for it at the very least.

Yes, the ports changed upstream, so I chose to update them in our manifest

srhb · 2018-05-25T09:52:18Z

I've built and run the tests on NixOS. Looks good! 👍

azazel75 · 2018-05-25T10:36:49Z

Well done @johanot, do you need any further help?

coretemp · 2018-05-25T12:45:50Z

I am mostly curious whether anyone is using kubernetes on NixOS in production, since the existence of a module is not the same as a production version module.

In general, some kind of meta-data on services like "experimental" or "just a toy" or "used in a national lab" would be useful.

johanot · 2018-05-25T13:13:50Z

@coretemp We are using the module for our production cluster. We have a couple of very minor modifications to the module in our setup, which I just haven't gotten around to upstream yet.

@azazel75 Thanks. I'll look into the review comments now and let you know if I need anything further.

johanot · 2018-05-25T14:50:26Z

@srhb Please review latest commit, when you have time :)

srhb · 2018-05-25T21:39:57Z

nixos/doc/manual/release-notes/rl-1809.xml

@@ -108,7 +108,7 @@ $ nix-instantiate -E '(import &lt;nixpkgsunstable&gt; {}).gitFull'
    <para>
     <literal>gnucash</literal> has changed from version 2.4 to 3.x.
     If you've been using <literal>gnucash</literal> (version 2.4) instead of
-     <literal>gnucash26</literal> (version 2.6) you must open your Gnucash 
+     <literal>gnucash26</literal> (version 2.6) you must open your Gnucash


No touchy the whitespace of other people! :P

@srhb now undone! :P

@johanot

- Added information regarding breaking changes to release note for 18.09 - Changed golang version comment in kubernetes package - Added @johanot to maintainers list

srhb · 2018-05-26T09:35:33Z

I think this is very good quality and quite sufficient for inclusion. :)

coretemp · 2018-05-26T11:26:20Z

nixos/modules/services/cluster/kubernetes/dashboard.nix

    } // (optionalAttrs cfg.enableRBAC {
      kubernetes-dashboard-crb = {
-        apiVersion = "rbac.authorization.k8s.io/v1beta1";
+        apiVersion = "rbac.authorization.k8s.io/v1";


v1 is repeated in at least two places. Please introduce a variable and use that instead of using the same constant twice.

coretemp · 2018-05-26T11:28:36Z

nixos/modules/services/cluster/kubernetes/dns.nix

@@ -59,7 +59,7 @@ in {

    services.kubernetes.addonManager.addons = {
      kubedns-deployment = {
-        apiVersion = "apps/v1beta1";
+        apiVersion = "extensions/v1beta1";


Same comment for a beta version.

coretemp · 2018-05-26T11:34:15Z

nixos/doc/manual/release-notes/rl-1809.xml

+   </listitem>
+   <listitem>
+    <para>
+     Recommented way to access the Kubernetes Dashboard is with HTTPS (TLS)


t -> d (must have)

with -> via (nice to have)

and perhaps specify a complete URL to access the kubernetes dashboard, if possible.(wish list)

Will fix the typo in an upcoming PR. Thanks :-)

coretemp · 2018-05-26T11:35:50Z

nixos/modules/services/cluster/kubernetes/dashboard.nix

              containers = [{
                name = "kubernetes-dashboard";
                image = "${name}:${version}";
                ports = [{
-                  containerPort = 9090;
+                  containerPort = 8443;


8443 is repeated in multiple places.

coretemp · 2018-05-26T11:39:18Z

nixos/modules/services/cluster/kubernetes/dns.nix

                  };
+                  args = [
+                    "--v=2"
+                    "--logtostderr"


Motivation for logging to stderr is missing. For production services, I almost never want to do that.

It's customnary in Docker containers: logs are then managed using logging drivers and aggregated using dedicated services. The alternative would be for the spawned process to write to files (as there's probably no logging daemon running in the container), but that approach brings up the issue of using a PersistentVolume just for the logs, which is far more invasive as a choice, in my opinion.

@coretemp No personal preferences involved here. The yaml manifest is a copy of the upstream kube-dns manifest at: https://github.com/kubernetes/kubernetes/blob/release-1.10/cluster/addons/dns/kube-dns.yaml.base. A possible future improvement to the dns submodule could be to add more options for making these things configurable. For now though, I think it's perfectly fine to ship the upstream defaults.

coretemp · 2018-05-26T11:40:13Z

nixos/modules/services/cluster/kubernetes/dns.nix

                  };
+                  args = [
+                    "--v=2"


If this is a verbosity level it should have a comment. The numeric value could instead be a named constant saying something about the verbosity level.

johanot · 2018-05-26T14:13:32Z

@coretemp Thank you for your comments in general. In a couple of weeks (probably) I'll be submitting a few minor improvements to the kubernetes modules in a new PR. Will bear your considerations in mind then.

NeQuissimus · 2018-05-26T22:48:22Z

@johanot ! Somehow I missed this PR completely... Thank you for this! 🎉

cstrahan and others added 5 commits May 25, 2018 10:50

kubernetes: update to 1.10

996849a

kubernetes: 1.10.0 -> 1.10.3

3e3917b

Kuberbetes DNS addon 1.14.4 -> 1.14.10

55fa98d

As shipped with k8s 1.10.3. Also: - updated the definition jsons as they are distributed in k8s. - updated the image uris as they are renamed in k8s - added imageDigest param as per 7368487

Kuberetes Dashboard addon 1.8.2 -> 1.8.3

29fd05f

As shipped with k8s 1.10.3. Also: - updated the definition jsons as they are distributed in k8s. - updated the image uris as they are renamed in k8s - added imageDigest param as per 7368487

srhb self-assigned this May 25, 2018

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) labels May 25, 2018

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels May 25, 2018

srhb reviewed May 25, 2018

View reviewed changes

srhb mentioned this pull request May 25, 2018

kubernetes: 1.9.1 -> 1.10 #38445

Closed

8 tasks

GrahamcOfBorg added 8.has: changelog 8.has: documentation labels May 25, 2018

srhb reviewed May 25, 2018

View reviewed changes

Kubernetes:

df54c25

- Added information regarding breaking changes to release note for 18.09 - Changed golang version comment in kubernetes package - Added @johanot to maintainers list

johanot force-pushed the kube-1.10 branch from 6493a1a to df54c25 Compare May 26, 2018 09:20

srhb merged commit 2052c16 into NixOS:master May 26, 2018

coretemp reviewed May 26, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubernetes: 1.9.7 -> 1.10.3 #41073

kubernetes: 1.9.7 -> 1.10.3 #41073

johanot commented May 25, 2018 •

edited by NeQuissimus

johanot commented May 25, 2018 •

edited

srhb May 25, 2018

johanot May 25, 2018

srhb May 25, 2018

srhb May 25, 2018

srhb May 25, 2018

azazel75 May 25, 2018

srhb commented May 25, 2018

azazel75 commented May 25, 2018

coretemp commented May 25, 2018

johanot commented May 25, 2018

johanot commented May 25, 2018

srhb May 25, 2018

johanot May 26, 2018

srhb commented May 26, 2018

coretemp May 26, 2018

coretemp May 26, 2018

coretemp May 26, 2018

johanot May 26, 2018

coretemp May 26, 2018

coretemp May 26, 2018

azazel75 May 26, 2018

johanot May 26, 2018

coretemp May 26, 2018

johanot commented May 26, 2018

NeQuissimus commented May 26, 2018

kubernetes: 1.9.7 -> 1.10.3 #41073

kubernetes: 1.9.7 -> 1.10.3 #41073

Conversation

johanot commented May 25, 2018 • edited by NeQuissimus

Motivation for this change

Things done

johanot commented May 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srhb commented May 25, 2018

azazel75 commented May 25, 2018

coretemp commented May 25, 2018

johanot commented May 25, 2018

johanot commented May 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srhb commented May 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanot commented May 26, 2018

NeQuissimus commented May 26, 2018

johanot commented May 25, 2018 •

edited by NeQuissimus

johanot commented May 25, 2018 •

edited