From 8b5b27bb512dfc5186eb07ee64561890ea75b0d1 Mon Sep 17 00:00:00 2001
From: Chad Swenson <chadswen@gmail.com>
Date: Fri, 4 Nov 2016 16:40:14 -0500
Subject: [PATCH] Docker Options Refactor

---
 inventory/group_vars/all.yml                  |  5 +-
 roles/docker/tasks/main.yml                   | 21 +++----
 roles/docker/tasks/non-systemd.yml            | 61 +++++++++++++++++++
 roles/docker/tasks/systemd-proxies.yml        |  9 ---
 roles/docker/tasks/systemd.yml                | 24 ++++++++
 roles/docker/templates/docker-options.conf.j2 |  2 +
 ...md-docker.service.j2 => docker.service.j2} | 13 +---
 roles/docker/templates/http-proxy.conf.j2     |  1 -
 .../node/templates/kubelet-container.j2       |  2 +-
 roles/network_plugin/calico/tasks/main.yml    | 13 ----
 roles/network_plugin/calico/templates/docker  |  2 -
 roles/network_plugin/flannel/tasks/main.yml   | 41 +++++++++----
 roles/network_plugin/flannel/templates/docker |  6 --
 .../flannel/templates/docker-systemd          |  2 -
 .../flannel/templates/flannel-options.conf.j2 |  2 +
 roles/network_plugin/weave/tasks/main.yml     | 10 ---
 roles/network_plugin/weave/templates/docker   |  2 -
 17 files changed, 131 insertions(+), 85 deletions(-)
 create mode 100644 roles/docker/tasks/non-systemd.yml
 delete mode 100644 roles/docker/tasks/systemd-proxies.yml
 create mode 100644 roles/docker/tasks/systemd.yml
 create mode 100644 roles/docker/templates/docker-options.conf.j2
 rename roles/docker/templates/{systemd-docker.service.j2 => docker.service.j2} (65%)
 delete mode 100644 roles/network_plugin/calico/templates/docker
 delete mode 100644 roles/network_plugin/flannel/templates/docker
 delete mode 100644 roles/network_plugin/flannel/templates/docker-systemd
 create mode 100644 roles/network_plugin/flannel/templates/flannel-options.conf.j2
 delete mode 100644 roles/network_plugin/weave/templates/docker

diff --git a/inventory/group_vars/all.yml b/inventory/group_vars/all.yml
index 229a0d314..f64cab5aa 100644
--- a/inventory/group_vars/all.yml
+++ b/inventory/group_vars/all.yml
@@ -148,11 +148,14 @@ dns_server: "{{ kube_service_addresses|ipaddr('net')|ipaddr(2)|ipaddr('address')
 # https_proxy: ""
 # no_proxy: ""
 
+# Path used to store Docker data
+docker_daemon_graph: "/var/lib/docker"
+
 ## A string of extra options to pass to the docker daemon.
 ## This string should be exactly as you wish it to appear.
 ## An obvious use case is allowing insecure-registry access
 ## to self hosted registries like so:
-docker_options: "--insecure-registry={{ kube_service_addresses }}"
+docker_options: "--insecure-registry={{ kube_service_addresses }} --graph={{ docker_daemon_graph }}"
 
 # K8s image pull policy (imagePullPolicy)
 k8s_image_pull_policy: IfNotPresent
diff --git a/roles/docker/tasks/main.yml b/roles/docker/tasks/main.yml
index 1d237f5e9..c3bd842f3 100644
--- a/roles/docker/tasks/main.yml
+++ b/roles/docker/tasks/main.yml
@@ -62,20 +62,13 @@
   with_items: "{{ docker_package_info.pkgs }}"
   when: (ansible_os_family != "CoreOS") and (docker_package_info.pkgs|length > 0)
 
-- name: allow for proxies on systems using systemd
-  include: systemd-proxies.yml
-  when: ansible_service_mgr == "systemd" and
-        (http_proxy is defined or https_proxy is defined or no_proxy is defined)
+- name: Set docker upstart and sysvinit config
+  include: non-systemd.yml
+  when: ansible_service_mgr in ["sysvinit","upstart"]
 
-- name: Write docker.service systemd file
-  template:
-    src: systemd-docker.service.j2
-    dest: /etc/systemd/system/docker.service
-  register: docker_service_file
-  notify: restart docker
-  when: ansible_service_mgr == "systemd" and ansible_os_family != "CoreOS"
-
-- meta: flush_handlers
+- name: Set docker systemd config
+  include: systemd.yml
+  when: ansible_service_mgr == "systemd"
 
 - name: ensure docker service is started and enabled
   service:
@@ -83,4 +76,4 @@
     enabled: yes
     state: started
   with_items:
-    - docker
+    - docker
\ No newline at end of file
diff --git a/roles/docker/tasks/non-systemd.yml b/roles/docker/tasks/non-systemd.yml
new file mode 100644
index 000000000..48139e625
--- /dev/null
+++ b/roles/docker/tasks/non-systemd.yml
@@ -0,0 +1,61 @@
+---
+# This uses lineinfile instead of templates for idempotency in files that may be modified by different roles
+- name: Set docker options config file path
+  set_fact:
+    docker_options_file: >-
+      {%- if ansible_os_family == "Debian" -%}/etc/default/docker{%- elif ansible_os_family == "RedHat" -%}/etc/sysconfig/docker{%- endif -%}
+
+- name: Set docker options config variable name
+  set_fact:
+    docker_options_name: >-
+      {%- if ansible_os_family == "Debian" -%}DOCKER_OPTS{%- elif ansible_os_family == "RedHat" -%}other_args{%- endif -%}
+
+- name: Set docker options config value to be written
+  set_fact:
+    docker_options_value: '"{{ docker_options }} $DOCKER_NETWORK_OPTIONS $DOCKER_STORAGE_OPTIONS $INSECURE_REGISTRY"'
+
+- name: Set docker options config line to be written
+  set_fact:
+    docker_options_line: "{{ docker_options_name }}={{ docker_options_value }}"
+
+- name: Set docker proxy lines to be written
+  set_fact:
+    docker_proxy_lines:
+     - { name: "HTTP_PROXY", value: '"{{ http_proxy }}"' }
+     - { name: "HTTPS_PROXY", value: '"{{ https_proxy }}"' }
+     - { name: "NO_PROXY", value: '"{{ no_proxy }}"' }
+
+- name: Remove docker daemon proxy config lines that don't match desired lines
+  lineinfile:
+    dest: "{{ docker_options_file }}"
+    regexp: "^{{ item.name }}=(?!{{ item.value|regex_escape() }})"
+    state: absent
+  with_items: "{{ docker_proxy_lines|default([]) }}"
+  when: item.value is defined and (item.value | trim != '')
+
+- name: Write docker daemon proxy config lines
+  lineinfile:
+    dest: "{{ docker_options_file }}"
+    line: "{{ item.name }}={{ item.value }}"
+    owner: root
+    group: root
+    mode: 0644
+  with_items: "{{ docker_proxy_lines|default([]) }}"
+  when: item.value is defined and (item.value | trim != '')
+
+- name: Remove docker daemon options lines that don't match desired line
+  lineinfile:
+    dest: "{{ docker_options_file }}"
+    regexp: "^(DOCKER_OPTS|OPTIONS|other_args)=(?!{{ docker_options_value|regex_escape() }})"
+    state: absent
+
+- name: Write docker daemon options line
+  lineinfile:
+    dest: "{{ docker_options_file }}"
+    line: "{{ docker_options_line }}"
+    owner: root
+    group: root
+    mode: 0644
+  notify: restart docker
+
+- meta: flush_handlers
\ No newline at end of file
diff --git a/roles/docker/tasks/systemd-proxies.yml b/roles/docker/tasks/systemd-proxies.yml
deleted file mode 100644
index 4bbc423c9..000000000
--- a/roles/docker/tasks/systemd-proxies.yml
+++ /dev/null
@@ -1,9 +0,0 @@
----
-- name: create docker service directory for systemd
-  file: path=/etc/systemd/system/docker.service.d state=directory
-
-- name: drop docker environment conf to enable proxy usage
-  template:
-    src: http-proxy.conf.j2
-    dest: /etc/systemd/system/docker.service.d/http-proxy.conf
-  notify: restart docker
diff --git a/roles/docker/tasks/systemd.yml b/roles/docker/tasks/systemd.yml
new file mode 100644
index 000000000..e998979a6
--- /dev/null
+++ b/roles/docker/tasks/systemd.yml
@@ -0,0 +1,24 @@
+---
+- name: Create docker service systemd directory if it doesn't exist
+  file: path=/etc/systemd/system/docker.service.d state=directory
+
+- name: Write docker proxy drop-in
+  template:
+    src: http-proxy.conf.j2
+    dest: /etc/systemd/system/docker.service.d/http-proxy.conf
+  when: http_proxy is defined or https_proxy is defined or no_proxy is defined
+
+- name: Write docker.service systemd file
+  template:
+    src: docker.service.j2
+    dest: /etc/systemd/system/docker.service
+  register: docker_service_file
+  when: ansible_os_family != "CoreOS"
+
+- name: Write docker options systemd drop-in
+  template:
+    src: docker-options.conf.j2
+    dest: "/etc/systemd/system/docker.service.d/docker-options.conf"
+  notify: restart docker
+
+- meta: flush_handlers
\ No newline at end of file
diff --git a/roles/docker/templates/docker-options.conf.j2 b/roles/docker/templates/docker-options.conf.j2
new file mode 100644
index 000000000..50356a9f4
--- /dev/null
+++ b/roles/docker/templates/docker-options.conf.j2
@@ -0,0 +1,2 @@
+[Service]
+Environment="DOCKER_OPTS={% if docker_options is defined %}{{ docker_options }}{% endif %}"
diff --git a/roles/docker/templates/systemd-docker.service.j2 b/roles/docker/templates/docker.service.j2
similarity index 65%
rename from roles/docker/templates/systemd-docker.service.j2
rename to roles/docker/templates/docker.service.j2
index b19b1caaf..60ab22194 100644
--- a/roles/docker/templates/systemd-docker.service.j2
+++ b/roles/docker/templates/docker.service.j2
@@ -11,24 +11,15 @@ Wants=docker.socket
 
 [Service]
 Type=notify
-{% if ansible_os_family == "RedHat" %}
-EnvironmentFile=-/etc/default/docker
-EnvironmentFile=-/etc/sysconfig/docker
-EnvironmentFile=-/etc/sysconfig/docker-network
-EnvironmentFile=-/etc/sysconfig/docker-storage
-{% elif ansible_os_family == "Debian" %}
-EnvironmentFile=-/etc/default/docker
-{% endif %}
 Environment=GOTRACEBACK=crash
 ExecReload=/bin/kill -s HUP $MAINPID
 Delegate=yes
 KillMode=process
 ExecStart=/usr/bin/docker daemon \
-          $OPTIONS \
+          $DOCKER_OPTS \
           $DOCKER_STORAGE_OPTIONS \
           $DOCKER_NETWORK_OPTIONS \
-          $INSECURE_REGISTRY \
-          $DOCKER_OPTS
+          $INSECURE_REGISTRY
 TasksMax=infinity
 LimitNOFILE=1048576
 LimitNPROC=1048576
diff --git a/roles/docker/templates/http-proxy.conf.j2 b/roles/docker/templates/http-proxy.conf.j2
index 7e558837c..e79047771 100644
--- a/roles/docker/templates/http-proxy.conf.j2
+++ b/roles/docker/templates/http-proxy.conf.j2
@@ -1,3 +1,2 @@
 [Service]
-
 Environment={% if http_proxy %}"HTTP_PROXY={{ http_proxy }}"{% endif %} {% if https_proxy %}"HTTPS_PROXY={{ https_proxy }}"{% endif %} {% if no_proxy %}"NO_PROXY={{ no_proxy }}"{% endif %}
diff --git a/roles/kubernetes/node/templates/kubelet-container.j2 b/roles/kubernetes/node/templates/kubelet-container.j2
index 2fcc7307f..9be8795a9 100644
--- a/roles/kubernetes/node/templates/kubelet-container.j2
+++ b/roles/kubernetes/node/templates/kubelet-container.j2
@@ -6,7 +6,7 @@
 -v /etc/kubernetes:/etc/kubernetes \
 -v /sys:/sys \
 -v /dev:/dev \
--v /var/lib/docker:/var/lib/docker \
+-v {{ docker_daemon_graph }}:/var/lib/docker \
 -v /var/run:/var/run \
 -v /var/lib/kubelet:/var/lib/kubelet \
 {{ hyperkube_image_repo }}:{{ hyperkube_image_tag}} \
diff --git a/roles/network_plugin/calico/tasks/main.yml b/roles/network_plugin/calico/tasks/main.yml
index 189c6f370..c67313208 100644
--- a/roles/network_plugin/calico/tasks/main.yml
+++ b/roles/network_plugin/calico/tasks/main.yml
@@ -5,19 +5,6 @@
     dest: "/etc/cni/net.d/10-calico.conf"
     owner: kube
 
-- name: Calico | Set docker daemon options
-  template:
-    src: docker
-    dest: "/etc/default/docker"
-    owner: root
-    group: root
-    mode: 0644
-  notify:
-    - restart docker
-  when: ansible_os_family != "CoreOS"
-
-- meta: flush_handlers
-
 - name: Calico | Create calico certs directory
   file:
     dest: "{{ calico_cert_dir }}"
diff --git a/roles/network_plugin/calico/templates/docker b/roles/network_plugin/calico/templates/docker
deleted file mode 100644
index 05d6f9535..000000000
--- a/roles/network_plugin/calico/templates/docker
+++ /dev/null
@@ -1,2 +0,0 @@
-# Deployed by Ansible
-DOCKER_OPTS="{% if docker_options is defined %}{{ docker_options }}{% endif %}"
diff --git a/roles/network_plugin/flannel/tasks/main.yml b/roles/network_plugin/flannel/tasks/main.yml
index 8581d2ce7..ebb2b693d 100644
--- a/roles/network_plugin/flannel/tasks/main.yml
+++ b/roles/network_plugin/flannel/tasks/main.yml
@@ -35,27 +35,42 @@
 - set_fact:
     flannel_mtu: "{{ flannel_mtu_output.stdout }}"
 
-- name: Flannel | Set docker daemon options
-  template:
-    src: docker
-    dest: "/etc/default/docker"
+- set_fact:
+    docker_options_file: >-
+      {%- if ansible_os_family == "Debian" -%}/etc/default/docker{%- elif ansible_os_family == "RedHat" -%}/etc/sysconfig/docker{%- endif -%}
+
+- set_fact:
+    docker_options_name: >-
+      {%- if ansible_os_family == "Debian" -%}DOCKER_OPTS{%- elif ansible_os_family == "RedHat" -%}other_args{%- endif -%}
+
+- set_fact:
+    docker_network_options: '"--bip={{ flannel_subnet }} --mtu={{ flannel_mtu }}"'
+
+- name: Flannel | Remove non-systemd docker daemon network options that don't match desired line
+  lineinfile:
+    dest: "{{ docker_options_file }}"
+    regexp: "^DOCKER_NETWORK_OPTIONS=(?!{{ docker_network_options|regex_escape() }})"
+    state: absent
+  when: ansible_service_mgr in ["sysvinit","upstart"]
+
+- name: Flannel | Set non-systemd docker daemon network options
+  lineinfile:
+    dest: "{{ docker_options_file }}"
+    line: DOCKER_NETWORK_OPTIONS={{ docker_network_options }}
+    insertbefore: ^{{ docker_options_name }}=
     owner: root
     group: root
     mode: 0644
   notify:
     - restart docker
-  when: ansible_os_family != "CoreOS"
-
-- name: Flannel | Create docker service path for CoreOS
-  file: path=/etc/systemd/system/docker.service.d state=directory
-  when: ansible_os_family == "CoreOS"
+  when: ansible_service_mgr in ["sysvinit","upstart"]
 
-- name: Flannel | Create docker dropin for CoreOS
+- name: Flannel | Create docker network systemd drop-in
   template:
-    src: docker-systemd
+    src: flannel-options.conf.j2
     dest: "/etc/systemd/system/docker.service.d/flannel-options.conf"
   notify:
     - restart docker
-  when: ansible_os_family == "CoreOS"
+  when: ansible_service_mgr == "systemd"
 
-- meta: flush_handlers
+- meta: flush_handlers
\ No newline at end of file
diff --git a/roles/network_plugin/flannel/templates/docker b/roles/network_plugin/flannel/templates/docker
deleted file mode 100644
index a0e2b052b..000000000
--- a/roles/network_plugin/flannel/templates/docker
+++ /dev/null
@@ -1,6 +0,0 @@
-# Deployed by Ansible
-{% if (ansible_service_mgr in ["sysvinit","upstart"] and ansible_os_family == "Debian") or (ansible_os_family == "CoreOS") %}
-DOCKER_OPTS="--bip={{ flannel_subnet }} --mtu={{ flannel_mtu }} {% if docker_options is defined %}{{ docker_options }}{% endif %}"
-{% else %}
-OPTIONS="--bip={{ flannel_subnet }} --mtu={{ flannel_mtu }} {% if docker_options is defined %}{{ docker_options }}{% endif %}"
-{% endif %}
diff --git a/roles/network_plugin/flannel/templates/docker-systemd b/roles/network_plugin/flannel/templates/docker-systemd
deleted file mode 100644
index 8d7d6ad83..000000000
--- a/roles/network_plugin/flannel/templates/docker-systemd
+++ /dev/null
@@ -1,2 +0,0 @@
-[Service]
-Environment="DOCKER_OPTS=--bip={{ flannel_subnet }} --mtu={{ flannel_mtu }} {% if docker_options is defined %}{{ docker_options }}{% endif %}"
diff --git a/roles/network_plugin/flannel/templates/flannel-options.conf.j2 b/roles/network_plugin/flannel/templates/flannel-options.conf.j2
new file mode 100644
index 000000000..62dcb1e41
--- /dev/null
+++ b/roles/network_plugin/flannel/templates/flannel-options.conf.j2
@@ -0,0 +1,2 @@
+[Service]
+Environment="DOCKER_NETWORK_OPTIONS=--bip={{ flannel_subnet }} --mtu={{ flannel_mtu }}"
diff --git a/roles/network_plugin/weave/tasks/main.yml b/roles/network_plugin/weave/tasks/main.yml
index 59cc1bf37..b9c00cd33 100644
--- a/roles/network_plugin/weave/tasks/main.yml
+++ b/roles/network_plugin/weave/tasks/main.yml
@@ -1,14 +1,4 @@
 ---
-- name: Set docker daemon options
-  template:
-    src: docker
-    dest: "/etc/default/docker"
-    owner: root
-    group: root
-    mode: 0644
-  notify:
-    - restart docker
-
 - name: Weave | Copy cni plugins from hyperkube
   command: "/usr/bin/docker run --rm -v /opt/cni/bin:/cnibindir {{ hyperkube_image_repo }}:{{ hyperkube_image_tag }} /bin/cp -r /opt/cni/bin/. /cnibindir/"
   register: cni_task_result
diff --git a/roles/network_plugin/weave/templates/docker b/roles/network_plugin/weave/templates/docker
deleted file mode 100644
index 05d6f9535..000000000
--- a/roles/network_plugin/weave/templates/docker
+++ /dev/null
@@ -1,2 +0,0 @@
-# Deployed by Ansible
-DOCKER_OPTS="{% if docker_options is defined %}{{ docker_options }}{% endif %}"