From 4fb8adb9e47ac3094906c5005f3f052f710aee79 Mon Sep 17 00:00:00 2001
From: Wilmar den Ouden <wilmardo@users.noreply.github.com>
Date: Tue, 8 Jan 2019 21:36:44 +0100
Subject: [PATCH] More dynamic local-storage-provisioner approach (#3472)

* Makes local volume provisioner more dynamic

* Correct variable name in local storage provisioner defaults

* Updates external-provisioner readme

* Updates variable naming to be more clear, more documentation, fixes sample inventory

* Variable refactor, untangled some jinja2 loops

* Corrects variable name

* No variable substitution in dict keys, replaced with anchor

* Fixes default storage_classes dict, inline docs

* Fixes spelling in inline docs

* Addresses comments in review

* Updates all the defaults

* Fix failing CI task

* Fixes external provisioner daemonset
---
 .../sample/group_vars/k8s-cluster/addons.yml  | 23 +++++----
 .../local_volume_provisioner/README.md        | 48 +++++++++++++++++--
 .../defaults/main.yml                         | 13 +++--
 .../local_volume_provisioner/tasks/main.yml   |  5 +-
 .../local-volume-provisioner-cm.yml.j2        | 22 +++++++--
 .../local-volume-provisioner-ds.yml.j2        | 12 ++---
 .../local-volume-provisioner-psp.yml.j2       |  4 +-
 .../local-volume-provisioner-sc.yml.j2        |  6 +--
 .../tasks/0050-create_directories.yml         |  6 ++-
 roles/kubespray-defaults/defaults/main.yaml   | 13 +++--
 10 files changed, 107 insertions(+), 45 deletions(-)

diff --git a/inventory/sample/group_vars/k8s-cluster/addons.yml b/inventory/sample/group_vars/k8s-cluster/addons.yml
index a73f590b8..ebe12e9c9 100644
--- a/inventory/sample/group_vars/k8s-cluster/addons.yml
+++ b/inventory/sample/group_vars/k8s-cluster/addons.yml
@@ -21,18 +21,17 @@ metrics_server_enabled: false
 local_volume_provisioner_enabled: false
 # local_volume_provisioner_namespace: kube-system
 # local_volume_provisioner_storage_classes:
-#   - name: "{{ local_volume_provisioner_storage_class | default('local-storage') }}"
-#     host_dir: "{{ local_volume_provisioner_base_dir | default ('/mnt/disks') }}"
-#     mount_dir: "{{ local_volume_provisioner_mount_dir | default('/mnt/disks') }}"
-#   - name: "local-ssd"
-#     host_dir: "/mnt/local-storage/ssd"
-#     mount_dir: "/mnt/local-storage/ssd"
-#   - name: "local-hdd"
-#     host_dir: "/mnt/local-storage/hdd"
-#     mount_dir: "/mnt/local-storage/hdd"
-#   - name: "local-shared"
-#     host_dir: "/mnt/local-storage/shared"
-#     mount_dir: "/mnt/local-storage/shared"
+#   - local-storage:
+#       host_dir: /mnt/disks
+#       mount_dir: /mnt/disks
+#   - fast-disks:
+#       host_dir: /mnt/fast-disks
+#       mount_dir: /mnt/fast-disks
+#       block_cleaner_command:
+#          - "/scripts/shred.sh"
+#          - "2"
+#       volume_mode: Filesystem
+#       fs_type: ext4
 
 # CephFS provisioner deployment
 cephfs_provisioner_enabled: false
diff --git a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/README.md b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/README.md
index cbc468f13..46528bb47 100644
--- a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/README.md
+++ b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/README.md
@@ -1,11 +1,51 @@
 Local Storage Provisioner
 =========================
 
-The local storage provisioner is NOT a dynamic storage provisioner as you would
+The [local storage provisioner](https://github.com/kubernetes-incubator/external-storage/tree/master/local-volume)
+is NOT a dynamic storage provisioner as you would
 expect from a cloud provider. Instead, it simply creates PersistentVolumes for
-all manually created volumes located in the directories specified in the `local_volume_provisioner_storage_classes.host_dir` entries.
-The default path is /mnt/disks and the rest of this doc will use that path as
-an example.
+all mounts under the host_dir of the specified storage class.
+These storage classes are specified in the `local_volume_provisioner_storage_classes` list.
+An example this list:
+
+```yaml
+local_volume_provisioner_storage_classes:
+  - local-storage:
+      host_dir: /mnt/disks
+      mount_dir: /mnt/disks
+  - fast-disks:
+      host_dir: /mnt/fast-disks
+      mount_dir: /mnt/fast-disks
+      block_cleaner_command:
+         - "/scripts/shred.sh"
+         - "2"
+      volume_mode: Filesystem
+      fs_type: ext4
+```
+
+For each dictionary in `local_volume_provisioner_storage_classes` a storageClass with the
+same name is created. The keys of this dictionary are converted to camelCase and added
+as attributes to the storageClass.
+The result of the above example is:
+
+```yaml
+data:
+  storageClassMap: |
+    local-storage:
+       hostDir: /mnt/disks
+       mountDir: /mnt/disks
+    fast-disks:
+       hostDir: /mnt/fast-disks
+       mountDir:  /mnt/fast-disks
+       blockCleanerCommand:
+         - "/scripts/shred.sh"
+         - "2"
+       volumeMode: Filesystem
+       fsType: ext4
+```
+
+The default StorageClass is local-storage on /mnt/disks,
+the rest of this doc will use that path as an example.
 
 Examples to create local storage volumes
 ----------------------------------------
diff --git a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/defaults/main.yml b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/defaults/main.yml
index 2a703cbf2..d0a5a9846 100644
--- a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/defaults/main.yml
+++ b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/defaults/main.yml
@@ -1,6 +1,11 @@
 ---
 local_volume_provisioner_namespace: "kube-system"
-local_volume_provisioner_storage_classes:
-  - name: "{{ local_volume_provisioner_storage_class | default('local-storage') }}"
-    host_dir: "{{ local_volume_provisioner_base_dir | default ('/mnt/disks') }}"
-    mount_dir: "{{ local_volume_provisioner_mount_dir | default('/mnt/disks') }}"
\ No newline at end of file
+# Levarages Ansibles string to Python datatype casting. Otherwise the dict_key isn't substituted
+# see https://github.com/ansible/ansible/issues/17324
+local_volume_provisioner_storage_classes: |
+  {
+    "{{ local_volume_provisioner_storage_class | default('local-storage') }}": {
+     "host_dir": "{{ local_volume_provisioner_base_dir | default ('/mnt/disks') }}",
+      "mount_dir": "{{ local_volume_provisioner_mount_dir | default('/mnt/disks') }}"
+    }
+  }
diff --git a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/tasks/main.yml b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/tasks/main.yml
index 12f259147..7ce95fb72 100644
--- a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/tasks/main.yml
+++ b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/tasks/main.yml
@@ -1,8 +1,7 @@
 ---
-
 - name: Local Volume Provisioner | Ensure base dir is created on all hosts
   file:
-    path: "{{ item[1].host_dir }}"
+    path: "{{ local_volume_provisioner_storage_classes[item.1].host_dir }}"
     state: directory
     owner: root
     group: root
@@ -10,7 +9,7 @@
   delegate_to: "{{ item[0] }}"
   with_nested:
     - "{{ groups['k8s-cluster'] }}"
-    - "{{ local_volume_provisioner_storage_classes }}"
+    - "{{ local_volume_provisioner_storage_classes.keys() }}"
   failed_when: false
 
 - name: Local Volume Provisioner | Create addon dir
diff --git a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-cm.yml.j2 b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-cm.yml.j2
index 8c5078eb6..3ca1a0cf2 100644
--- a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-cm.yml.j2
+++ b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-cm.yml.j2
@@ -1,3 +1,14 @@
+# Macro to convert camelCase dictionary keys to snake_case keys
+{%- macro convert_keys(mydict) %}
+  {% for key in mydict.keys() -%}
+    {% set key_split = key.split('_') -%}
+    {% set new_key = key_split[0] + key_split[1:]|map('capitalize')|join -%}
+    {% set value = mydict.pop(key) -%}
+    {{ mydict.__setitem__(new_key, value) -}}
+    {{ convert_keys(value) if value is mapping else None -}}
+  {% endfor -%}
+{% endmacro -%}
+
 ---
 apiVersion: v1
 kind: ConfigMap
@@ -6,8 +17,9 @@ metadata:
   namespace: {{ local_volume_provisioner_namespace }}
 data:
   storageClassMap: |
-{% for class in local_volume_provisioner_storage_classes %}
-    {{ class.name }}:
-      hostDir: {{ class.host_dir }}
-      mountDir: {{ class.mount_dir }}
-{% endfor %}
\ No newline at end of file
+{% for class_name, storage_class in local_volume_provisioner_storage_classes.iteritems() %}
+    {{ class_name }}:
+      {{- convert_keys(storage_class) }}
+      {{ storage_class | to_nice_yaml(indent=2) | indent(6) }}
+{%- endfor %}
+
diff --git a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-ds.yml.j2 b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-ds.yml.j2
index 22b2f8b22..28062aae3 100644
--- a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-ds.yml.j2
+++ b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-ds.yml.j2
@@ -44,17 +44,17 @@ spec:
             - name: local-volume-provisioner
               mountPath: /etc/provisioner/config
               readOnly: true
-{% for class in local_volume_provisioner_storage_classes %}
-            - name: {{ class.name }}
-              mountPath: {{ class.mount_dir }}
+{% for class_name, class_config in local_volume_provisioner_storage_classes.iteritems() %}
+            - name: local-volume-provisioner-hostpath-{{ class_name }}
+              mountPath: {{ class_config.mount_dir }}
               mountPropagation: "HostToContainer"
 {% endfor %}
       volumes:
         - name: local-volume-provisioner
           configMap:
             name: local-volume-provisioner
-{% for class in local_volume_provisioner_storage_classes %}
-        - name: {{ class.name }}
+{% for class_name, class_config in local_volume_provisioner_storage_classes.iteritems() %}
+        - name: local-volume-provisioner-hostpath-{{ class_name }}
           hostPath:
-            path: {{ class.host_dir }}
+            path: {{ class_config.host_dir }}
 {% endfor %}
diff --git a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-psp.yml.j2 b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-psp.yml.j2
index 0358d8dda..a7442f123 100644
--- a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-psp.yml.j2
+++ b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-psp.yml.j2
@@ -25,8 +25,8 @@ spec:
     - 'downwardAPI'
     - 'hostPath'
   allowedHostPaths:
-{% for class in local_volume_provisioner_storage_classes %}
-    - pathPrefix: "{{ class.host_dir }}"
+{% for class_name, class_config in local_volume_provisioner_storage_classes.iteritems() %}
+    - pathPrefix: "{{ class_config.host_dir }}"
       readOnly: false
 {% endfor %}
   hostNetwork: false
diff --git a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-sc.yml.j2 b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-sc.yml.j2
index 2a5ad13d4..4a4afae3a 100644
--- a/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-sc.yml.j2
+++ b/roles/kubernetes-apps/external_provisioner/local_volume_provisioner/templates/local-volume-provisioner-sc.yml.j2
@@ -1,9 +1,9 @@
-{% for class in local_volume_provisioner_storage_classes %}
+{% for class_name in local_volume_provisioner_storage_classes.keys() %}
 ---
 apiVersion: storage.k8s.io/v1
 kind: StorageClass
 metadata:
-  name: {{ class.name }}
+  name: {{ class_name }}
 provisioner: kubernetes.io/no-provisioner
 volumeBindingMode: WaitForFirstConsumer
-{% endfor %}
\ No newline at end of file
+{% endfor %}
diff --git a/roles/kubernetes/preinstall/tasks/0050-create_directories.yml b/roles/kubernetes/preinstall/tasks/0050-create_directories.yml
index f27bda3fe..43cb1c60f 100644
--- a/roles/kubernetes/preinstall/tasks/0050-create_directories.yml
+++ b/roles/kubernetes/preinstall/tasks/0050-create_directories.yml
@@ -47,12 +47,14 @@
 
 - name: Create local volume provisioner directories
   file:
-    path: "{{ item.host_dir }}"
+    path: "{{ local_volume_provisioner_storage_classes[item.1].host_dir }}"
     state: directory
     owner: root
     group: root
     mode: 0700
-  with_items: "{{ local_volume_provisioner_storage_classes }}"
+  with_nested:
+    - "{{ groups['k8s-cluster'] }}"
+    - "{{ local_volume_provisioner_storage_classes.keys() }}"
   when:
     - inventory_hostname in groups['k8s-cluster']
     - local_volume_provisioner_enabled
diff --git a/roles/kubespray-defaults/defaults/main.yaml b/roles/kubespray-defaults/defaults/main.yaml
index c5e8f55f6..78aa83d5f 100644
--- a/roles/kubespray-defaults/defaults/main.yaml
+++ b/roles/kubespray-defaults/defaults/main.yaml
@@ -314,10 +314,15 @@ kube_feature_gates: |-
   {%- endif %}
 
 # Local volume provisioner storage classes
-local_volume_provisioner_storage_classes:
-  - name: "{{ local_volume_provisioner_storage_class | default('local-storage') }}"
-    host_dir: "{{ local_volume_provisioner_base_dir | default ('/mnt/disks') }}"
-    mount_dir: "{{ local_volume_provisioner_mount_dir | default('/mnt/disks') }}"
+# Levarages Ansibles string to Python datatype casting. Otherwise the dict_key isn't substituted
+# see https://github.com/ansible/ansible/issues/17324
+local_volume_provisioner_storage_classes: |
+  {
+    "{{ local_volume_provisioner_storage_class | default('local-storage') }}": {
+     "host_dir": "{{ local_volume_provisioner_base_dir | default ('/mnt/disks') }}",
+      "mount_dir": "{{ local_volume_provisioner_mount_dir | default('/mnt/disks') }}"
+    }
+  }
 
 # weave's network password for encryption
 # if null then no network encryption
-- 
GitLab