From b4a1e300f39b26773250b4af9abcf40ee3b33290 Mon Sep 17 00:00:00 2001
From: George Angel <george-angel@users.noreply.github.com>
Date: Thu, 13 Feb 2020 08:22:14 +0000
Subject: [PATCH] Move the cert fetch and restart calls into separate unit
 (#105)

* Move the cert fetch and restart calls into separate unit

Decouples kubelet service from certificate refreshing. Failing to fetch
new certificates doesn't restart kubelet or other components.

Restarting kubelet no longer refreshes certificates or restarts other
components.
---
 cfssl.tf                                      | 90 +++++++++----------
 common.tf                                     |  7 --
 etcd.tf                                       |  2 +-
 master.tf                                     |  9 +-
 .../cert-fetcher-etcd}/main.tf                |  4 +-
 modules/cert-refresh-master/main.tf           | 53 +++++++++++
 modules/cert-refresh-node/main.tf             | 45 ++++++++++
 .../systemd_service_restarter}/main.tf        |  4 +-
 node-common.tf                                |  5 ++
 resources/master-kubelet.service              | 12 ---
 resources/node-kubelet.service                |  2 -
 storage-node.tf                               |  2 +-
 worker.tf                                     |  2 +-
 13 files changed, 162 insertions(+), 75 deletions(-)
 rename {cert-fetcher => modules/cert-fetcher-etcd}/main.tf (86%)
 create mode 100644 modules/cert-refresh-master/main.tf
 create mode 100644 modules/cert-refresh-node/main.tf
 rename {systemd_service_restarter => modules/systemd_service_restarter}/main.tf (85%)

diff --git a/cfssl.tf b/cfssl.tf
index 039cb7d..79bc784 100644
--- a/cfssl.tf
+++ b/cfssl.tf
@@ -67,9 +67,9 @@ EOS
 }
 
 data "ignition_file" "cfssl-init-ca" {
-  mode = 493
+  mode       = 493
   filesystem = "root"
-  path = "/opt/bin/cfssl-init-ca"
+  path       = "/opt/bin/cfssl-init-ca"
 
   content {
     content = file("${path.module}/resources/cfssl-init-ca.sh")
@@ -77,9 +77,9 @@ data "ignition_file" "cfssl-init-ca" {
 }
 
 data "ignition_file" "cfssl-init-proxy-pki" {
-  mode = 493
+  mode       = 493
   filesystem = "root"
-  path = "/opt/bin/cfssl-init-proxy-pki"
+  path       = "/opt/bin/cfssl-init-proxy-pki"
 
   content {
     content = file("${path.module}/resources/cfssl-init-proxy-pki")
@@ -87,9 +87,9 @@ data "ignition_file" "cfssl-init-proxy-pki" {
 }
 
 data "ignition_file" "cfssl-proxy-ca-csr-json" {
-  mode = 420
+  mode       = 420
   filesystem = "root"
-  path = "/etc/cfssl/proxy-ca-csr.json"
+  path       = "/etc/cfssl/proxy-ca-csr.json"
 
   content {
     content = file("${path.module}/resources/cfssl-proxy-ca-csr.json")
@@ -97,9 +97,9 @@ data "ignition_file" "cfssl-proxy-ca-csr-json" {
 }
 
 data "ignition_file" "cfssl-proxy-csr-json" {
-  mode = 420
+  mode       = 420
   filesystem = "root"
-  path = "/etc/cfssl/proxy-csr.json"
+  path       = "/etc/cfssl/proxy-csr.json"
 
   content {
     content = file("${path.module}/resources/cfssl-proxy-csr.json")
@@ -110,16 +110,16 @@ data "template_file" "cfssl-server-config" {
   template = file("${path.module}/resources/cfssl-server-config.json")
 
   vars = {
-    expiry_hours = var.cfssl_node_expiry_hours
+    expiry_hours     = var.cfssl_node_expiry_hours
     cfssl_unused_key = random_id.cfssl-auth-key-unused.hex
-    cfssl_auth_key = random_id.cfssl-auth-key-client.hex
+    cfssl_auth_key   = random_id.cfssl-auth-key-client.hex
   }
 }
 
 data "ignition_file" "cfssl-server-config" {
-  mode = 384
+  mode       = 384
   filesystem = "root"
-  path = "/etc/cfssl/config.json"
+  path       = "/etc/cfssl/config.json"
 
   content {
     content = data.template_file.cfssl-server-config.rendered
@@ -127,14 +127,14 @@ data "ignition_file" "cfssl-server-config" {
 }
 
 data "ignition_systemd_unit" "cfssl" {
-  name = "cfssl.service"
+  name    = "cfssl.service"
   content = file("${path.module}/resources/cfssl.service")
 }
 
 data "ignition_file" "cfssl-sk-csr" {
-  mode = 420
+  mode       = 420
   filesystem = "root"
-  path = "/etc/cfssl/sk-csr.json"
+  path       = "/etc/cfssl/sk-csr.json"
 
   content {
     content = <<EOS
@@ -170,7 +170,7 @@ data "template_file" "cfssl-nginx" {
 
   vars = {
     nginx_image_url = "nginx"
-    nginx_image_tag = "1.15-alpine"
+    nginx_image_tag = "1.17-alpine"
   }
 }
 
@@ -181,7 +181,7 @@ data "ignition_systemd_unit" "cfssl-nginx" {
 }
 
 module "cfssl-restarter" {
-  source = "./systemd_service_restarter"
+  source = "./modules/systemd_service_restarter"
 
   service_name = "cfssl"
   on_calendar  = "*-*-* 00:00:00"
@@ -189,34 +189,34 @@ module "cfssl-restarter" {
 
 data "ignition_config" "cfssl" {
   files = concat(
-      [
-        data.ignition_file.cfssl.id,
-        data.ignition_file.cfssljson.id,
-        data.ignition_file.cfssl-server-config.id,
-        data.ignition_file.cfssl-ca-csr.id,
-        data.ignition_file.cfssl-init-ca.id,
-        data.ignition_file.cfssl-sk-csr.id,
-        data.ignition_file.cfssl-init-proxy-pki.id,
-        data.ignition_file.cfssl-proxy-ca-csr-json.id,
-        data.ignition_file.cfssl-proxy-csr-json.id,
-        data.ignition_file.cfssl-nginx-conf.id,
-        data.ignition_file.cfssl-nginx-auth.id,
-        data.ignition_file.format-and-mount.id,
-      ],
-      var.cfssl_additional_files
-    )
+    [
+      data.ignition_file.cfssl.id,
+      data.ignition_file.cfssljson.id,
+      data.ignition_file.cfssl-server-config.id,
+      data.ignition_file.cfssl-ca-csr.id,
+      data.ignition_file.cfssl-init-ca.id,
+      data.ignition_file.cfssl-sk-csr.id,
+      data.ignition_file.cfssl-init-proxy-pki.id,
+      data.ignition_file.cfssl-proxy-ca-csr-json.id,
+      data.ignition_file.cfssl-proxy-csr-json.id,
+      data.ignition_file.cfssl-nginx-conf.id,
+      data.ignition_file.cfssl-nginx-auth.id,
+      data.ignition_file.format-and-mount.id,
+    ],
+    var.cfssl_additional_files
+  )
 
   systemd = concat(
-      [
-        data.ignition_systemd_unit.update-engine.id,
-        data.ignition_systemd_unit.locksmithd_cfssl.id,
-        data.ignition_systemd_unit.docker-opts-dropin.id,
-        data.ignition_systemd_unit.node-exporter.id,
-        data.ignition_systemd_unit.cfssl.id,
-        data.ignition_systemd_unit.cfssl-nginx.id,
-        data.ignition_systemd_unit.cfssl-disk-mounter.id,
-      ],
-      module.cfssl-restarter.systemd_units,
-      var.cfssl_additional_systemd_units
-    )
+    [
+      data.ignition_systemd_unit.update-engine.id,
+      data.ignition_systemd_unit.locksmithd_cfssl.id,
+      data.ignition_systemd_unit.docker-opts-dropin.id,
+      data.ignition_systemd_unit.node-exporter.id,
+      data.ignition_systemd_unit.cfssl.id,
+      data.ignition_systemd_unit.cfssl-nginx.id,
+      data.ignition_systemd_unit.cfssl-disk-mounter.id,
+    ],
+    module.cfssl-restarter.systemd_units,
+    var.cfssl_additional_systemd_units
+  )
 }
diff --git a/common.tf b/common.tf
index d418bbf..4fb5878 100644
--- a/common.tf
+++ b/common.tf
@@ -25,13 +25,6 @@ data "ignition_file" "cfssljson" {
   }
 }
 
-module "kubelet-restarter" {
-  source = "./systemd_service_restarter"
-
-  service_name = "kubelet"
-  on_calendar  = var.cfssl_node_renew_timer
-}
-
 data "ignition_systemd_unit" "docker-opts-dropin" {
   name = "docker.service"
 
diff --git a/etcd.tf b/etcd.tf
index 0b21408..9f554d3 100644
--- a/etcd.tf
+++ b/etcd.tf
@@ -125,7 +125,7 @@ data "ignition_systemd_unit" "etcd-member-dropin" {
 }
 
 module "etcd-cert-fetcher" {
-  source = "./cert-fetcher"
+  source = "./modules/cert-fetcher-etcd"
 
   on_calendar = var.cfssl_node_renew_timer
 }
diff --git a/master.tf b/master.tf
index 9586e50..30aaf70 100644
--- a/master.tf
+++ b/master.tf
@@ -3,6 +3,11 @@ data "ignition_systemd_unit" "locksmithd_master" {
   mask = false == var.enable_container_linux_locksmithd_master
 }
 
+module "cert-refresh-master" {
+  source      = "./modules/cert-refresh-master"
+  on_calendar = var.cfssl_node_renew_timer
+}
+
 // Node certificate for kubelet to use as part of system:master-nodes. We need
 // ClusterRoleBindings to allow kube components creation and bind the group
 // with system:node role. In order to be authorized by the Node authorizer,
@@ -456,7 +461,7 @@ data "ignition_config" "master" {
       data.ignition_systemd_unit.docker-opts-dropin.id,
       data.ignition_systemd_unit.master-kubelet.id,
     ],
-    module.kubelet-restarter.systemd_units,
-    var.master_additional_systemd_units
+    module.cert-refresh-master.systemd_units,
+    var.master_additional_systemd_units,
   )
 }
diff --git a/cert-fetcher/main.tf b/modules/cert-fetcher-etcd/main.tf
similarity index 86%
rename from cert-fetcher/main.tf
rename to modules/cert-fetcher-etcd/main.tf
index a3a0290..32ba79c 100644
--- a/cert-fetcher/main.tf
+++ b/modules/cert-fetcher-etcd/main.tf
@@ -32,7 +32,7 @@ EOS
 
 output "systemd_units" {
   value = [
-    "${data.ignition_systemd_unit.cert-fetch-service.id}",
-    "${data.ignition_systemd_unit.cert-fetch-timer.id}",
+    data.ignition_systemd_unit.cert-fetch-service.id,
+    data.ignition_systemd_unit.cert-fetch-timer.id,
   ]
 }
diff --git a/modules/cert-refresh-master/main.tf b/modules/cert-refresh-master/main.tf
new file mode 100644
index 0000000..109442f
--- /dev/null
+++ b/modules/cert-refresh-master/main.tf
@@ -0,0 +1,53 @@
+variable "on_calendar" {
+  type = string
+}
+
+data "ignition_systemd_unit" "cert-refresh" {
+  name = "cert-refresh.service"
+
+  content = <<EOS
+[Unit]
+Description=Fetch new certificates from cfssl server and restart components to reload certs
+Requires=docker.service
+After=network-online.target
+[Service]
+Type=oneshot
+ExecStart=/opt/bin/cfssl-keys-and-certs-get
+ExecStart=/opt/bin/cfssl-new-node-cert
+ExecStart=/opt/bin/cfssl-new-kubelet-cert
+ExecStart=/opt/bin/cfssl-new-apiserver-cert
+ExecStart=/opt/bin/cfssl-new-apiserver-kubelet-client-cert
+ExecStart=/opt/bin/cfssl-new-scheduler-cert
+ExecStart=/opt/bin/cfssl-new-controller-manager-cert
+# Hack to reload certs on control plane tier
+#  https://github.com/kubernetes/kubernetes/issues/46287
+ExecStart=/usr/bin/systemctl try-restart kubelet.service
+ExecStart=-/bin/sh -c "docker restart $(docker ps -q -f name=k8s_kube-controller-manager)"
+ExecStart=-/bin/sh -c "docker restart $(docker ps -q -f name=k8s_kube-apiserver)"
+ExecStart=-/bin/sh -c "docker restart $(docker ps -q -f name=k8s_kube-scheduler)"
+[Install]
+WantedBy=multi-user.target
+EOS
+}
+
+data "ignition_systemd_unit" "cert-refresh-timer" {
+  name = "cert-refresh.timer"
+
+  content = <<EOS
+[Unit]
+Description=Fetch new certificates from cfssl server and restart components to reload certs
+[Timer]
+OnCalendar=${var.on_calendar}
+AccuracySec=1s
+RandomizedDelaySec=60min
+[Install]
+WantedBy=timers.target
+EOS
+}
+
+output "systemd_units" {
+  value = [
+    data.ignition_systemd_unit.cert-refresh.id,
+    data.ignition_systemd_unit.cert-refresh-timer.id,
+  ]
+}
diff --git a/modules/cert-refresh-node/main.tf b/modules/cert-refresh-node/main.tf
new file mode 100644
index 0000000..6decb92
--- /dev/null
+++ b/modules/cert-refresh-node/main.tf
@@ -0,0 +1,45 @@
+variable "on_calendar" {
+  type = string
+}
+
+data "ignition_systemd_unit" "cert-refresh" {
+  name = "cert-refresh.service"
+
+  content = <<EOS
+[Unit]
+Description=Fetch new certificates from cfssl server and restart components to reload certs
+Requires=docker.service
+After=network-online.target
+[Service]
+Type=oneshot
+ExecStart=/opt/bin/cfssl-new-cert
+ExecStart=/opt/bin/cfssl-new-kubelet-cert
+# Hack to reload certs on control plane tier
+#  https://github.com/kubernetes/kubernetes/issues/46287
+ExecStart=/usr/bin/systemctl try-restart kubelet.service
+[Install]
+WantedBy=multi-user.target
+EOS
+}
+
+data "ignition_systemd_unit" "cert-refresh-timer" {
+  name = "cert-refresh.timer"
+
+  content = <<EOS
+[Unit]
+Description=Fetch new certificates from cfssl server and restart components to reload certs
+[Timer]
+OnCalendar=${var.on_calendar}
+AccuracySec=1s
+RandomizedDelaySec=60min
+[Install]
+WantedBy=timers.target
+EOS
+}
+
+output "systemd_units" {
+  value = [
+    data.ignition_systemd_unit.cert-refresh.id,
+    data.ignition_systemd_unit.cert-refresh-timer.id,
+  ]
+}
diff --git a/systemd_service_restarter/main.tf b/modules/systemd_service_restarter/main.tf
similarity index 85%
rename from systemd_service_restarter/main.tf
rename to modules/systemd_service_restarter/main.tf
index e1d1b78..7b3ecf5 100644
--- a/systemd_service_restarter/main.tf
+++ b/modules/systemd_service_restarter/main.tf
@@ -30,7 +30,7 @@ EOS
 
 output "systemd_units" {
   value = [
-    "${data.ignition_systemd_unit.service-restart.id}",
-    "${data.ignition_systemd_unit.service-restart-timer.id}",
+    data.ignition_systemd_unit.service-restart.id,
+    data.ignition_systemd_unit.service-restart-timer.id,
   ]
 }
diff --git a/node-common.tf b/node-common.tf
index 22c596c..ee44962 100644
--- a/node-common.tf
+++ b/node-common.tf
@@ -142,3 +142,8 @@ data "ignition_file" "prometheus-ro-rootfs" {
     content = file("${path.module}/resources/prometheus-ro-rootfs")
   }
 }
+
+module "cert-refresh-node" {
+  source      = "./modules/cert-refresh-node"
+  on_calendar = var.cfssl_node_renew_timer
+}
diff --git a/resources/master-kubelet.service b/resources/master-kubelet.service
index ac27864..90e7a0e 100644
--- a/resources/master-kubelet.service
+++ b/resources/master-kubelet.service
@@ -16,18 +16,6 @@ ExecStartPre=/sbin/sysctl -w net.ipv4.tcp_retries2=8
 # This is a temporal workaround this upstream Kubernetes issue:
 #  https://github.com/kubernetes/kubernetes/issues/69015
 ExecStartPre=/sbin/sysctl -w fs.inotify.max_user_watches=524288
-ExecStartPre=/opt/bin/cfssl-keys-and-certs-get
-ExecStartPre=/opt/bin/cfssl-new-node-cert
-ExecStartPre=/opt/bin/cfssl-new-kubelet-cert
-ExecStartPre=/opt/bin/cfssl-new-apiserver-cert
-ExecStartPre=/opt/bin/cfssl-new-apiserver-kubelet-client-cert
-ExecStartPre=/opt/bin/cfssl-new-scheduler-cert
-ExecStartPre=/opt/bin/cfssl-new-controller-manager-cert
-# Hack to reload certs on control plane tier
-#  https://github.com/kubernetes/kubernetes/issues/46287
-ExecStartPre=-/bin/sh -c "docker restart $(docker ps -q -f name=k8s_kube-controller-manager)"
-ExecStartPre=-/bin/sh -c "docker restart $(docker ps -q -f name=k8s_kube-apiserver)"
-ExecStartPre=-/bin/sh -c "docker restart $(docker ps -q -f name=k8s_kube-scheduler)"
 ExecStart=${kubelet_binary_path} \
   --config=/etc/kubernetes/config/master-kubelet-conf.yaml \
   --kubeconfig=/var/lib/kubelet/kubeconfig \
diff --git a/resources/node-kubelet.service b/resources/node-kubelet.service
index 24b6411..614d817 100644
--- a/resources/node-kubelet.service
+++ b/resources/node-kubelet.service
@@ -16,8 +16,6 @@ ExecStartPre=/sbin/sysctl -w net.ipv4.tcp_retries2=8
 # This is a temporal workaround this upstream Kubernetes issue:
 #  https://github.com/kubernetes/kubernetes/issues/69015
 ExecStartPre=/sbin/sysctl -w fs.inotify.max_user_watches=524288
-ExecStartPre=/opt/bin/cfssl-new-cert
-ExecStartPre=/opt/bin/cfssl-new-kubelet-cert
 ExecStart=${kubelet_binary_path} \
 %{ if cloud_provider != "" }  --cloud-provider=${cloud_provider} \
 %{ endif ~}
diff --git a/storage-node.tf b/storage-node.tf
index b5bfcb7..a9b15ae 100644
--- a/storage-node.tf
+++ b/storage-node.tf
@@ -81,7 +81,7 @@ data "ignition_config" "storage-node" {
       data.ignition_systemd_unit.prometheus-ro-rootfs-timer.id,
       data.ignition_systemd_unit.storage-node-disk-mounter.id,
     ],
-    module.kubelet-restarter.systemd_units,
+    module.cert-refresh-node.systemd_units,
     var.storage_node_additional_systemd_units
   )
 }
diff --git a/worker.tf b/worker.tf
index a7b7550..7616898 100644
--- a/worker.tf
+++ b/worker.tf
@@ -63,7 +63,7 @@ data "ignition_config" "worker" {
       data.ignition_systemd_unit.prometheus-ro-rootfs.id,
       data.ignition_systemd_unit.prometheus-ro-rootfs-timer.id,
     ],
-    module.kubelet-restarter.systemd_units,
+    module.cert-refresh-node.systemd_units,
     var.worker_additional_systemd_units
   )
 }