From beecf009010a2ffa45598c6a04933f5644f3f629 Mon Sep 17 00:00:00 2001 From: Tim Bielawa Date: Fri, 10 Feb 2017 08:00:16 -0800 Subject: Trying to fix up/audit note some changes --- .../common/openshift-cluster/upgrades/etcd/backup.yml | 6 ++++++ .../common/openshift-cluster/upgrades/etcd/upgrade.yml | 15 +++++++++++++++ .../openshift-cluster/upgrades/post_control_plane.yml | 6 ++++++ roles/openshift_hosted_templates/tasks/main.yml | 2 ++ roles/openshift_manageiq/tasks/main.yaml | 7 +++++++ roles/openshift_node_upgrade/tasks/main.yml | 6 ++++++ 6 files changed, 42 insertions(+) diff --git a/playbooks/common/openshift-cluster/upgrades/etcd/backup.yml b/playbooks/common/openshift-cluster/upgrades/etcd/backup.yml index 45aabf3e4..7ef79afa9 100644 --- a/playbooks/common/openshift-cluster/upgrades/etcd/backup.yml +++ b/playbooks/common/openshift-cluster/upgrades/etcd/backup.yml @@ -29,12 +29,18 @@ - name: Check available disk space for etcd backup shell: df --output=avail -k {{ openshift.common.data_dir }} | tail -n 1 register: avail_disk + # AUDIT:changed_when: `false` because we are only inspecting + # state, not manipulating anything + changed_when: false # TODO: replace shell module with command and update later checks - name: Check current embedded etcd disk usage shell: du -k {{ openshift.etcd.etcd_data_dir }} | tail -n 1 | cut -f1 register: etcd_disk_usage when: embedded_etcd | bool + # AUDIT:changed_when: `false` because we are only inspecting + # state, not manipulating anything + changed_when: false - name: Abort if insufficient disk space for etcd backup fail: diff --git a/playbooks/common/openshift-cluster/upgrades/etcd/upgrade.yml b/playbooks/common/openshift-cluster/upgrades/etcd/upgrade.yml index 690858c53..a9b5b94e6 100644 --- a/playbooks/common/openshift-cluster/upgrades/etcd/upgrade.yml +++ b/playbooks/common/openshift-cluster/upgrades/etcd/upgrade.yml @@ -9,21 +9,36 @@ register: etcd_rpm_version failed_when: false when: not openshift.common.is_containerized | bool + # AUDIT:changed_when: `false` because we are only inspecting + # state, not manipulating anything + changed_when: false + - name: Record containerized etcd version command: docker exec etcd_container rpm -qa --qf '%{version}' etcd\* register: etcd_container_version failed_when: false when: openshift.common.is_containerized | bool + # AUDIT:changed_when: `false` because we are only inspecting + # state, not manipulating anything + changed_when: false + - name: Record containerized etcd version command: docker exec etcd_container rpm -qa --qf '%{version}' etcd\* register: etcd_container_version failed_when: false when: openshift.common.is_containerized | bool and not openshift.common.is_etcd_system_container | bool + # AUDIT:changed_when: `false` because we are only inspecting + # state, not manipulating anything + changed_when: false + - name: Record containerized etcd version command: runc exec etcd_container rpm -qa --qf '%{version}' etcd\* register: etcd_container_version failed_when: false when: openshift.common.is_containerized | bool and openshift.common.is_etcd_system_container | bool + # AUDIT:changed_when: `false` because we are only inspecting + # state, not manipulating anything + changed_when: false # I really dislike this copy/pasta but I wasn't able to find a way to get it to loop # through hosts, then loop through tasks only when appropriate diff --git a/playbooks/common/openshift-cluster/upgrades/post_control_plane.yml b/playbooks/common/openshift-cluster/upgrades/post_control_plane.yml index 4135f7e94..d2ed64300 100644 --- a/playbooks/common/openshift-cluster/upgrades/post_control_plane.yml +++ b/playbooks/common/openshift-cluster/upgrades/post_control_plane.yml @@ -51,6 +51,9 @@ '{"spec":{"template":{"spec":{"containers":[{"name":"router","image":"{{ router_image }}","livenessProbe":{"tcpSocket":null,"httpGet":{"path": "/healthz", "port": 1936, "host": "localhost", "scheme": "HTTP"},"initialDelaySeconds":10,"timeoutSeconds":1}}]}}}}' --api-version=v1 with_items: "{{ haproxy_routers }}" + # AUDIT:changed_when_note: `false` not being set here. What we + # need to do is check the current router image version and see if + # this task needs to be ran. - name: Check for default registry command: > @@ -66,6 +69,9 @@ {{ oc_cmd }} patch dc/docker-registry -n default -p '{"spec":{"template":{"spec":{"containers":[{"name":"registry","image":"{{ registry_image }}"}]}}}}' --api-version=v1 + # AUDIT:changed_when_note: `false` not being set here. What we + # need to do is check the current registry image version and see + # if this task needs to be ran. # Check for warnings to be printed at the end of the upgrade: - name: Check for warnings diff --git a/roles/openshift_hosted_templates/tasks/main.yml b/roles/openshift_hosted_templates/tasks/main.yml index 7d176bce3..89b92dfcc 100644 --- a/roles/openshift_hosted_templates/tasks/main.yml +++ b/roles/openshift_hosted_templates/tasks/main.yml @@ -4,6 +4,8 @@ become: False register: copy_hosted_templates_mktemp run_once: True + # AUDIT:changed_when: not set here because this task actually + # creates something - name: Create tar of OpenShift examples local_action: command tar -C "{{ role_path }}/files/{{ content_version }}/{{ hosted_deployment_type }}" -cvf "{{ copy_hosted_templates_mktemp.stdout }}/openshift-hosted-templates.tar" . diff --git a/roles/openshift_manageiq/tasks/main.yaml b/roles/openshift_manageiq/tasks/main.yaml index e58947fd2..f202486a5 100644 --- a/roles/openshift_manageiq/tasks/main.yaml +++ b/roles/openshift_manageiq/tasks/main.yaml @@ -47,6 +47,9 @@ register: oshawkular_create_cluster_role failed_when: "'already exists' not in oshawkular_create_cluster_role.stderr and oshawkular_create_cluster_role.rc != 0" changed_when: oshawkular_create_cluster_role.rc == 0 + # AUDIT:changed_when_note: Checking the return code is insufficient + # here. We really need to verify the if the role even exists before + # we run this task. - name: Configure role/user permissions command: > @@ -56,6 +59,10 @@ register: osmiq_perm_task failed_when: "'already exists' not in osmiq_perm_task.stderr and osmiq_perm_task.rc != 0" changed_when: osmiq_perm_task.rc == 0 + # AUDIT:changed_when_note: Checking the return code is insufficient + # here. We really need to compare the current role/user permissions + # with their expected state. I think we may have a module for this? + - name: Configure 3_2 role/user permissions command: > diff --git a/roles/openshift_node_upgrade/tasks/main.yml b/roles/openshift_node_upgrade/tasks/main.yml index b1d5f0e0f..609ca2a6e 100644 --- a/roles/openshift_node_upgrade/tasks/main.yml +++ b/roles/openshift_node_upgrade/tasks/main.yml @@ -75,3 +75,9 @@ # so containerized services should restart quickly as well. retries: 24 delay: 5 + # AUDIT:changed_when: `false` because we are only inspecting the + # state of the node, we aren't changing anything (we changed node + # service state in the previous task). You could say we shouldn't + # override this because something will be changing (the state of a + # service), but that should be part of the last task. + changed_when: false -- cgit v1.2.3