From e589ceb6610b139c6cd85ca4d0f569e4a7487b07 Mon Sep 17 00:00:00 2001 From: Stanislav Zaprudskiy Date: Thu, 26 Jan 2023 14:53:32 +0100 Subject: [PATCH 1/7] When applying Deployment wait up to (timeout * replicas) There are cases when having a new Deployment may be taking above the default timeout of 120s. For instance, when a Deployment has multiple replicas, and each replica starts on a separate node, and the Deployment specifies new images, then just pulling these new images for each replica may be taking above the default timeout of 120s. Having the default time multiplied by the number of replicas should provide generally enough time for all replicas to start --- roles/installer/tasks/resources_configuration.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/roles/installer/tasks/resources_configuration.yml b/roles/installer/tasks/resources_configuration.yml index ab202266..64c73cf0 100644 --- a/roles/installer/tasks/resources_configuration.yml +++ b/roles/installer/tasks/resources_configuration.yml @@ -210,6 +210,7 @@ apply: yes definition: "{{ lookup('template', 'deployments/deployment.yaml.j2') }}" wait: yes + wait_timeout: "{{ 120 * replicas or 120 }}" register: this_deployment_result - block: From ad531c8dce97279914eac2b501fad84a5c98bdb4 Mon Sep 17 00:00:00 2001 From: Stanislav Zaprudskiy Date: Thu, 26 Jan 2023 17:09:42 +0100 Subject: [PATCH 2/7] Do not wait for a new Pod name after Deployment change Proper waiting is already performed earlier during Deplyment{apply: yes, wait: yes} - https://github.com/ansible-collections/kubernetes.core/blob/e6ac87409830b6c698b05dba875cca87d45ea761/plugins/module_utils/k8s/waiter.py#L27. And also not every Deployment change produces new RS/Pods. For example, changing Deployment labels won't cause new rollout, but will cause `until` loop to be invoked unnecessarily (when replicas=1). --- roles/installer/tasks/resources_configuration.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/roles/installer/tasks/resources_configuration.yml b/roles/installer/tasks/resources_configuration.yml index 64c73cf0..fd2db1cf 100644 --- a/roles/installer/tasks/resources_configuration.yml +++ b/roles/installer/tasks/resources_configuration.yml @@ -237,11 +237,6 @@ field_selectors: - status.phase=Running register: _new_pod - until: - - _new_pod['resources'] | length - - _new_pod['resources'][0]['metadata']['name'] != tower_pod_name - delay: 5 - retries: 60 - name: Update new resource pod name as a variable. set_fact: From b3a74362aff871f4eb9b9ba73ea2b4ddf1ba46ff Mon Sep 17 00:00:00 2001 From: Stanislav Zaprudskiy Date: Tue, 24 Jan 2023 18:45:24 +0100 Subject: [PATCH 3/7] Make AWX Pod variable to be calculated respecting `creationTimestamp` and `deletionTimestamp` Do not consider Pods marked for deletion when calculating tower_pod to address replicas scale down case - where normally Pods spawned recently are being taken for removal. As well as the case when operator kicked off but some old replicas are still terminating. Respect `creationTimestamp` so to make sure that the newest Pod is taken after Deployment application, in which case multiple RS Pods (from old RS and new RS) could be running simultaneously while the rollout is happening. --- .../tasks/resources_configuration.yml | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/roles/installer/tasks/resources_configuration.yml b/roles/installer/tasks/resources_configuration.yml index fd2db1cf..438af396 100644 --- a/roles/installer/tasks/resources_configuration.yml +++ b/roles/installer/tasks/resources_configuration.yml @@ -13,9 +13,17 @@ - status.phase=Running register: tower_pod +- name: Set the resource pod as a variable. + set_fact: + tower_pod: >- + {{ tower_pod['resources'] + | rejectattr('metadata.deletionTimestamp', 'defined') + | sort(attribute='metadata.creationTimestamp') + | first | default({}) }} + - name: Set the resource pod name as a variable. set_fact: - tower_pod_name: "{{ tower_pod['resources'][0]['metadata']['name'] | default('') }}" + tower_pod_name: "{{ tower_pod['metadata']['name'] | default('') }}" - name: Set user provided control plane ee image set_fact: @@ -238,9 +246,17 @@ - status.phase=Running register: _new_pod + - name: Update new resource pod as a variable. + set_fact: + tower_pod: >- + {{ _new_pod['resources'] + | rejectattr('metadata.deletionTimestamp', 'defined') + | sort(attribute='metadata.creationTimestamp') + | last | default({}) }} + - name: Update new resource pod name as a variable. set_fact: - tower_pod_name: '{{ _new_pod["resources"][0]["metadata"]["name"] }}' + tower_pod_name: '{{ tower_pod["metadata"]["name"] | default("")}}' when: - tower_resources_result.changed or this_deployment_result.changed From 94d68bf382ec8dc4ce28d7d8d154663afd00b7fe Mon Sep 17 00:00:00 2001 From: Stanislav Zaprudskiy Date: Thu, 26 Jan 2023 16:22:07 +0100 Subject: [PATCH 4/7] Make Deployment to be rolled out on CM and Secrets changes With the previous approach, not all associated (mounted) CM/Secrets changes caused the Deployment to be rolled out, but also the Deployment could have been rolled out unnecessary during e.g. Ingress or Service changes (which do not require Pod restarts). Previously existing Pod removal (state: absent) was not complete as other pods continued to exist, but also is not needed with this commit change due to added Pods annotations. The added Deployment Pod annotations now cause the new ReplicaSet version to be rolled out, effectively causing replacement of the previously existing Pods in accordance with the deployment `strategy` (https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#deploymentstrategy-v1-apps, `RollingUpdate`) whenever there is a change in the associated CMs or Secrets referenced in annotations. This implementation is quite standard and widely used for Helm workflows - https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments --- .../tasks/resources_configuration.yml | 64 ++++++++++++------- .../tasks/secret_key_configuration.yml | 4 +- .../templates/deployments/deployment.yaml.j2 | 19 +++++- 3 files changed, 62 insertions(+), 25 deletions(-) diff --git a/roles/installer/tasks/resources_configuration.yml b/roles/installer/tasks/resources_configuration.yml index 438af396..0d56eb1c 100644 --- a/roles/installer/tasks/resources_configuration.yml +++ b/roles/installer/tasks/resources_configuration.yml @@ -40,13 +40,13 @@ kind: Secret namespace: '{{ ansible_operator_meta.namespace }}' name: '{{ ansible_operator_meta.name }}-receptor-ca' - register: _receptor_ca + register: receptor_ca no_log: "{{ no_log }}" - name: Migrate Receptor CA Secret when: - - _receptor_ca['resources'] | default([]) | length - - _receptor_ca['resources'][0]['type'] != "kubernetes.io/tls" + - receptor_ca['resources'] | default([]) | length + - receptor_ca['resources'][0]['type'] != "kubernetes.io/tls" block: - name: Delete old Receptor CA Secret k8s: @@ -61,7 +61,7 @@ register: _receptor_ca_key_file - name: Copy Receptor CA key from old secret to tempfile copy: - content: "{{ _receptor_ca['resources'][0]['data']['receptor-ca.key'] | b64decode }}" + content: "{{ receptor_ca['resources'][0]['data']['receptor-ca.key'] | b64decode }}" dest: "{{ _receptor_ca_key_file.path }}" no_log: "{{ no_log }}" - name: Create tempfile for receptor-ca.crt @@ -71,7 +71,7 @@ register: _receptor_ca_crt_file - name: Copy Receptor CA cert from old secret to tempfile copy: - content: "{{ _receptor_ca['resources'][0]['data']['receptor-ca.crt'] | b64decode }}" + content: "{{ receptor_ca['resources'][0]['data']['receptor-ca.crt'] | b64decode }}" dest: "{{ _receptor_ca_crt_file.path }}" no_log: "{{ no_log }}" - name: Create New Receptor CA secret @@ -79,6 +79,17 @@ apply: true definition: "{{ lookup('template', 'secrets/receptor_ca_secret.yaml.j2') }}" no_log: "{{ no_log }}" + - name: Read New Receptor CA Secret + k8s_info: + kind: Secret + namespace: '{{ ansible_operator_meta.namespace }}' + name: '{{ ansible_operator_meta.name }}-receptor-ca' + register: _receptor_ca + no_log: "{{ no_log }}" + - name: Set receptor_ca variable + set_fact: + receptor_ca: '{{ _receptor_ca }}' + no_log: "{{ no_log }}" - name: Remove tempfiles file: path: "{{ item }}" @@ -114,6 +125,17 @@ apply: true definition: "{{ lookup('template', 'secrets/receptor_ca_secret.yaml.j2') }}" no_log: "{{ no_log }}" + - name: Read Receptor CA secret + k8s_info: + kind: Secret + namespace: '{{ ansible_operator_meta.namespace }}' + name: '{{ ansible_operator_meta.name }}-receptor-ca' + register: _receptor_ca + no_log: "{{ no_log }}" + - name: Set receptor_ca variable + set_fact: + receptor_ca: '{{ _receptor_ca }}' + no_log: "{{ no_log }}" - name: Remove tempfiles file: path: "{{ item }}" @@ -121,14 +143,14 @@ loop: - "{{ _receptor_ca_key_file.path }}" - "{{ _receptor_ca_crt_file.path }}" - when: not _receptor_ca['resources'] | default([]) | length + when: not receptor_ca['resources'] | default([]) | length - name: Check for Receptor work signing Secret k8s_info: kind: Secret namespace: '{{ ansible_operator_meta.namespace }}' name: '{{ ansible_operator_meta.name }}-receptor-work-signing' - register: _receptor_work_signing + register: receptor_work_signing no_log: "{{ no_log }}" - name: Generate Receptor work signing RSA key pair @@ -159,6 +181,17 @@ apply: true definition: "{{ lookup('template', 'secrets/receptor_work_signing_secret.yaml.j2') }}" no_log: "{{ no_log }}" + - name: Read Receptor work signing Secret + k8s_info: + kind: Secret + namespace: '{{ ansible_operator_meta.namespace }}' + name: '{{ ansible_operator_meta.name }}-receptor-work-signing' + register: _receptor_work_signing + no_log: "{{ no_log }}" + - name: Set receptor_work_signing variable + set_fact: + receptor_work_signing: '{{ _receptor_work_signing }}' + no_log: "{{ no_log }}" - name: Remove tempfiles file: path: "{{ item }}" @@ -166,14 +199,13 @@ loop: - "{{ _receptor_work_signing_private_key_file.path }}" - "{{ _receptor_work_signing_public_key_file.path }}" - when: not _receptor_work_signing['resources'] | default([]) | length + when: not receptor_work_signing['resources'] | default([]) | length - name: Apply Resources k8s: apply: yes definition: "{{ lookup('template', item + '.yaml.j2') }}" wait: yes - register: tower_resources_result loop: - 'configmaps/config' - 'secrets/app_credentials' @@ -222,18 +254,6 @@ register: this_deployment_result - block: - - name: Delete pod to reload a resource configuration - k8s: - api_version: v1 - state: absent - kind: Pod - namespace: '{{ ansible_operator_meta.namespace }}' - name: '{{ tower_pod_name }}' - wait: yes - when: - - tower_resources_result.changed - - tower_pod_name | length - - name: Get the new resource pod information after updating resource. k8s_info: kind: Pod @@ -258,7 +278,7 @@ set_fact: tower_pod_name: '{{ tower_pod["metadata"]["name"] | default("")}}' when: - - tower_resources_result.changed or this_deployment_result.changed + - this_deployment_result.changed - name: Verify the resource pod name is populated. assert: diff --git a/roles/installer/tasks/secret_key_configuration.yml b/roles/installer/tasks/secret_key_configuration.yml index 00b64255..e7d022f5 100644 --- a/roles/installer/tasks/secret_key_configuration.yml +++ b/roles/installer/tasks/secret_key_configuration.yml @@ -40,10 +40,10 @@ - name: Set secret key secret set_fact: - __secret_key_secret: '{{ _generated_secret_key["resources"] | default([]) | length | ternary(_generated_secret_key, _secret_key_secret) }}' + secret_key: '{{ _generated_secret_key["resources"] | default([]) | length | ternary(_generated_secret_key, _secret_key_secret) }}' no_log: "{{ no_log }}" - name: Store secret key secret name set_fact: - secret_key_secret_name: "{{ __secret_key_secret['resources'][0]['metadata']['name'] }}" + secret_key_secret_name: "{{ secret_key['resources'][0]['metadata']['name'] }}" no_log: "{{ no_log }}" diff --git a/roles/installer/templates/deployments/deployment.yaml.j2 b/roles/installer/templates/deployments/deployment.yaml.j2 index 5135b713..81e0a519 100644 --- a/roles/installer/templates/deployments/deployment.yaml.j2 +++ b/roles/installer/templates/deployments/deployment.yaml.j2 @@ -20,8 +20,25 @@ spec: labels: {{ lookup("template", "../common/templates/labels/common.yaml.j2") | indent(width=8) | trim }} {{ lookup("template", "../common/templates/labels/version.yaml.j2") | indent(width=8) | trim }} -{% if annotations %} annotations: +{% for template in [ + "configmaps/config", + "secrets/app_credentials", + "storage/persistent", + ] %} + checksum-{{ template | replace('/', '-') }}: "{{ lookup('template', template + '.yaml.j2') | md5 }}" +{% endfor %} +{% for secret in [ + "bundle_cacert", + "route_tls", + "ldap_cacert", + "secret_key", + "receptor_ca", + "receptor_work_signing", + ] %} + checksum-secret-{{ secret }}: "{{ lookup('ansible.builtin.vars', secret, default='')["resources"][0]["data"] | default('') | md5 }}" +{% endfor %} +{% if annotations %} {{ annotations | indent(width=8) }} {% endif %} spec: From f042cb3d0064b7611b3b7896de55800b228a9fa1 Mon Sep 17 00:00:00 2001 From: Stanislav Zaprudskiy Date: Tue, 7 Feb 2023 16:31:26 +0100 Subject: [PATCH 5/7] Fix lint warnings --- roles/backup/tasks/creation.yml | 18 +++++++++--------- roles/installer/tasks/install.yml | 14 +++++++------- roles/restore/tasks/main.yml | 20 ++++++++++---------- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/roles/backup/tasks/creation.yml b/roles/backup/tasks/creation.yml index 06122e86..d4cdffc3 100644 --- a/roles/backup/tasks/creation.yml +++ b/roles/backup/tasks/creation.yml @@ -32,22 +32,22 @@ - this_backup['resources'][0]['metadata']['labels'] - block: - - include_tasks: init.yml + - include_tasks: init.yml - - include_tasks: postgres.yml + - include_tasks: postgres.yml - - include_tasks: awx-cro.yml + - include_tasks: awx-cro.yml - - include_tasks: secrets.yml + - include_tasks: secrets.yml - - name: Set flag signifying this backup was successful - set_fact: - backup_complete: true + - name: Set flag signifying this backup was successful + set_fact: + backup_complete: true - - include_tasks: cleanup.yml + - include_tasks: cleanup.yml when: - - this_backup['resources'][0]['status']['backupDirectory'] is not defined + - this_backup['resources'][0]['status']['backupDirectory'] is not defined - name: Update status variables include_tasks: update_status.yml diff --git a/roles/installer/tasks/install.yml b/roles/installer/tasks/install.yml index 4a37ed86..faca317e 100644 --- a/roles/installer/tasks/install.yml +++ b/roles/installer/tasks/install.yml @@ -39,17 +39,17 @@ - name: Load LDAP CAcert certificate include_tasks: load_ldap_cacert_secret.yml when: - - ldap_cacert_secret != '' + - ldap_cacert_secret != '' - name: Load ldap bind password include_tasks: load_ldap_password_secret.yml when: - - ldap_password_secret != '' + - ldap_password_secret != '' - name: Load bundle certificate authority certificate include_tasks: load_bundle_cacert_secret.yml when: - - bundle_cacert_secret != '' + - bundle_cacert_secret != '' - name: Include admin password configuration tasks include_tasks: admin_password_configuration.yml @@ -66,8 +66,8 @@ - name: Load Route TLS certificate include_tasks: load_route_tls_secret.yml when: - - ingress_type | lower == 'route' - - route_tls_secret != '' + - ingress_type | lower == 'route' + - route_tls_secret != '' - name: Include resources configuration tasks include_tasks: resources_configuration.yml @@ -91,8 +91,8 @@ bash -c "awx-manage migrate --noinput" register: migrate_result when: - - database_check is defined - - (database_check.stdout|trim) != '0' + - database_check is defined + - (database_check.stdout|trim) != '0' - name: Initialize Django include_tasks: initialize_django.yml diff --git a/roles/restore/tasks/main.yml b/roles/restore/tasks/main.yml index 42c36997..704f5da0 100644 --- a/roles/restore/tasks/main.yml +++ b/roles/restore/tasks/main.yml @@ -32,24 +32,24 @@ - this_restore['resources'][0]['metadata']['labels'] - block: - - include_tasks: init.yml + - include_tasks: init.yml - - include_tasks: import_vars.yml + - include_tasks: import_vars.yml - - include_tasks: secrets.yml + - include_tasks: secrets.yml - - include_tasks: deploy_awx.yml + - include_tasks: deploy_awx.yml - - include_tasks: postgres.yml + - include_tasks: postgres.yml - - name: Set flag signifying this restore was successful - set_fact: - tower_restore_complete: True + - name: Set flag signifying this restore was successful + set_fact: + tower_restore_complete: True - - include_tasks: cleanup.yml + - include_tasks: cleanup.yml when: - - this_restore['resources'][0]['status']['restoreComplete'] is not defined + - this_restore['resources'][0]['status']['restoreComplete'] is not defined - name: Update status variables include_tasks: update_status.yml From 336ea58a0af4e6ddb2d1ba6c0cb70eea74a05509 Mon Sep 17 00:00:00 2001 From: Stanislav Zaprudskiy Date: Fri, 23 Dec 2022 13:57:13 +0100 Subject: [PATCH 6/7] AWX: Add `termination_grace_period_seconds` --- README.md | 41 ++++++++++++ config/crd/bases/awx.ansible.com_awxs.yaml | 4 ++ .../awx-operator.clusterserviceversion.yaml | 5 ++ .../installer/files/pre-stop/termination-env | 66 +++++++++++++++++++ .../files/pre-stop/termination-master | 50 ++++++++++++++ .../files/pre-stop/termination-waiter | 33 ++++++++++ .../tasks/resources_configuration.yml | 1 + .../configmaps/pre_stop_scripts.yaml.j2 | 16 +++++ .../templates/deployments/deployment.yaml.j2 | 65 ++++++++++++++++++ 9 files changed, 281 insertions(+) create mode 100644 roles/installer/files/pre-stop/termination-env create mode 100755 roles/installer/files/pre-stop/termination-master create mode 100755 roles/installer/files/pre-stop/termination-waiter create mode 100644 roles/installer/templates/configmaps/pre_stop_scripts.yaml.j2 diff --git a/README.md b/README.md index 7631924f..494683bf 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,7 @@ An [Ansible AWX](https://github.com/ansible/awx) operator for Kubernetes built w * [Upgrade of instances without auto upgrade](#upgrade-of-instances-without-auto-upgrade) * [Service Account](#service-account) * [Labeling operator managed objects](#labeling-operator-managed-objects) + * [Pods termination grace period](#pods-termination-grace-period) * [Uninstall](#uninstall) * [Upgrading](#upgrading) * [Backup](#backup) @@ -1248,6 +1249,46 @@ spec: ... ``` +#### Pods termination grace period + +During deployment restarts or new rollouts, when old ReplicaSet Pods are being +terminated, the corresponding jobs which are managed (executed or controlled) +by old AWX Pods may end up in `Error` state as there is no mechanism to +transfer them to the newly spawned AWX Pods. To work around the problem one +could set `termination_grace_period_seconds` in AWX spec, which does the +following: + +* It sets the corresponding + [`terminationGracePeriodSeconds`](https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-termination) + Pod spec of the AWX Deployment to the value provided + + > The grace period is the duration in seconds after the processes running in + > the pod are sent a termination signal and the time when the processes are + > forcibly halted with a kill signal + +* It adds a + [`PreStop`](https://kubernetes.io/docs/concepts/containers/container-lifecycle-hooks/#hook-handler-execution) + hook script, which will keep AWX Pods in terminating state until it finished, + up to `terminationGracePeriodSeconds`. + + > This grace period applies to the total time it takes for both the PreStop + > hook to execute and for the Container to stop normally + + While the hook script just waits until the corresponding AWX Pod (instance) + no longer has any managed jobs, in which case it finishes with success and + hands over the overall Pod termination process to normal AWX processes. + +One may want to set this value to the maximum duration they accept to wait for +the affected Jobs to finish. Keeping in mind that such finishing jobs may +increase Pods termination time in such situations as `kubectl rollout restart`, +AWX upgrade by the operator, or Kubernetes [API-initiated +evictions](https://kubernetes.io/docs/concepts/scheduling-eviction/api-eviction/). + + +| Name | Description | Default | +| -------------------------------- | --------------------------------------------------------------- | ------- | +| termination_grace_period_seconds | Optional duration in seconds pods needs to terminate gracefully | not set | + ### Uninstall ### To uninstall an AWX deployment instance, you basically need to remove the AWX kind related to that instance. For example, to delete an AWX instance named awx-demo, you would do: diff --git a/config/crd/bases/awx.ansible.com_awxs.yaml b/config/crd/bases/awx.ansible.com_awxs.yaml index 0d645191..dbca15f5 100644 --- a/config/crd/bases/awx.ansible.com_awxs.yaml +++ b/config/crd/bases/awx.ansible.com_awxs.yaml @@ -526,6 +526,10 @@ spec: type: array type: object type: object + termination_grace_period_seconds: + description: Optional duration in seconds pods needs to terminate gracefully + type: integer + format: int32 service_labels: description: Additional labels to apply to the service type: string diff --git a/config/manifests/bases/awx-operator.clusterserviceversion.yaml b/config/manifests/bases/awx-operator.clusterserviceversion.yaml index 46e0fd94..43018338 100644 --- a/config/manifests/bases/awx-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/awx-operator.clusterserviceversion.yaml @@ -622,6 +622,11 @@ spec: x-descriptors: - urn:alm:descriptor:com.tectonic.ui:advanced - urn:alm:descriptor:com.tectonic.ui:hidden + - displayName: Termination Grace Period Seconds + path: termination_grace_period_seconds + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:advanced + - urn:alm:descriptor:com.tectonic.ui:hidden - displayName: Service Labels path: service_labels x-descriptors: diff --git a/roles/installer/files/pre-stop/termination-env b/roles/installer/files/pre-stop/termination-env new file mode 100644 index 00000000..f9d58fa8 --- /dev/null +++ b/roles/installer/files/pre-stop/termination-env @@ -0,0 +1,66 @@ +# file, which when exists, indicates that `master` script has successfully +# completed pre-stop script execution +marker_file="${PRE_STOP_MARKER_FILE:-/var/lib/pre-stop/.termination_marker}" + +# file which the running `master` script continuously updates (mtime) to +# indicate it's still running. this file is then read by `watcher`s to +# understand if they still have to wait for `termination_marker` +heartbeat_file="${PRE_STOP_HEARTBEAT_FILE:-/var/lib/pre-stop/.heartbeat}" + +# file which: +# * `watcher`s create when they bail out because they didn't see the +# `heartbeat_file` to be updated within `$heartbeat_failed_threshold`; +# * `master` creates when its handler command fails; +# when scripts see such file, they also give up +bailout_file="${PRE_STOP_BAILOUT_FILE:-/var/lib/pre-stop/.bailout}" +heartbeat_threshold="${PRE_STOP_HEARTBEAT_THRESHOLD:-60}" + +# where the scripts' stdout/stderr are streamed +stdout="${PRE_STOP_STDOUT:-/proc/1/fd/1}" +stderr="${PRE_STOP_STDERR:-/proc/1/fd/2}" + +# command the `master` script executes, which when successfully finishes, +# causes the script to create the `marker_file` +handler="${PRE_STOP_HANDLER:-bash -c \"PYTHONUNBUFFERED=x awx-manage disable_instance --wait --retry=inf\"}" + +log_prefix="${PRE_STOP_LOG_PREFIX:-preStop.exec}" +[[ -n ${PRE_STOP_LOG_ROLE} ]] && log_prefix="${log_prefix}] [$PRE_STOP_LOG_ROLE" + +# interval at which `watcher`s check for `marker_file` presence +recheck_sleep="${PRE_STOP_RECHECK_SLEEP:-1}" +# interval at which `watcher`s report into $stdout that they are still watching +report_every="${PRE_STOP_REPORT_EVERY:-30}" + +function log { + printf "[%s] $1\n" "$log_prefix" "${@:2}" +} + +function parameters_string { + for param in "$@"; do + printf "%s=\"%s\"\n" "$param" "${!param}" + done | paste -s -d ' ' +} + +function check_bailout { + if [[ -f $bailout_file ]]; then + log "\"%s\" file has been detected, accepting bail out signal and failing the hook script" \ + "$bailout_file" + exit 1 + fi +} + +function check_heartbeat { + if [[ -f $heartbeat_file ]]; then + delta=$(( $(date +%s) - $(stat -c %Y "$heartbeat_file") )) + else + delta=$(( $(date +%s) - $1 )) + fi + + if [[ $delta -gt $heartbeat_threshold ]]; then + log "The heartbeat file hasn't been updated since %ss, which is above the threshold of %ds, assuming the master is not operating and failing the hook script" \ + $delta + $heartbeat_threshold + touch "$bailout_file" + exit 1 + fi +} diff --git a/roles/installer/files/pre-stop/termination-master b/roles/installer/files/pre-stop/termination-master new file mode 100755 index 00000000..a34df232 --- /dev/null +++ b/roles/installer/files/pre-stop/termination-master @@ -0,0 +1,50 @@ +#/usr/bin/env bash + +PRE_STOP_LOG_ROLE="${PRE_STOP_LOG_ROLE:-master}" +source $(dirname "$0")/termination-env + +{ + +log "The hook has started: %s" \ + "$(parameters_string \ + "marker_file" \ + "heartbeat_file" \ + "bailout_file" \ + "handler" \ + )" + +touch "$heartbeat_file" + +set -o pipefail +eval "$handler" 2>&1 | while IFS= read -r line; do + # we check the files here and break early, but overall script termination + # happens later - as we need to distinguish between files detection and + # command failure, while bash doesn't offer a simple way to do this here + # inside the loop (`exit` does not terminate the script) + [[ -f $bailout_file ]] && break + [[ -f $marker_file ]] && break + + log "[handler] %s" "$line" + touch "$heartbeat_file" +done +ec=$? +set +o pipefail + +# process various cases in specific order +check_bailout + +if [[ -f $marker_file ]]; then + log "Done! The marker file has been detected, assuming some other instance of the script has run to completion" + exit 0 +elif [[ $ec -ne 0 ]]; then + log "The handler has failed with \"%d\" exit code, failing the hook script too" \ + $ec + # signal others to bail out + touch "$bailout_file" + exit $ec +else + log "Done! Generating the marker file allowing to proceed to termination" + touch "$marker_file" +fi + +} > "$stdout" 2> "$stderr" diff --git a/roles/installer/files/pre-stop/termination-waiter b/roles/installer/files/pre-stop/termination-waiter new file mode 100755 index 00000000..e7762c59 --- /dev/null +++ b/roles/installer/files/pre-stop/termination-waiter @@ -0,0 +1,33 @@ +#/usr/bin/env bash + +PRE_STOP_LOG_ROLE="${PRE_STOP_LOG_ROLE:-waiter}" +source $(dirname "$0")/termination-env + +{ + +log "The hook has started: %s" \ + "$(parameters_string \ + "marker_file" \ + "heartbeat_file" \ + "bailout_file" \ + "recheck_sleep" \ + "report_every" \ + )" + +n=0 +checks_started=$(date +%s) + +while ! [[ -f $marker_file ]]; do + check_bailout + check_heartbeat $checks_started + + if [[ $(($n % $report_every)) -eq 0 ]]; then + log "Waiting for the marker file to be accessible..." + fi + n=$(($n + 1)) + sleep $recheck_sleep +done + +log "The marker file found, exiting to proceed to termination" + +} > "$stdout" 2> "$stderr" diff --git a/roles/installer/tasks/resources_configuration.yml b/roles/installer/tasks/resources_configuration.yml index 0d56eb1c..f4286443 100644 --- a/roles/installer/tasks/resources_configuration.yml +++ b/roles/installer/tasks/resources_configuration.yml @@ -208,6 +208,7 @@ wait: yes loop: - 'configmaps/config' + - 'configmaps/pre_stop_scripts' - 'secrets/app_credentials' - 'rbac/service_account' - 'storage/persistent' diff --git a/roles/installer/templates/configmaps/pre_stop_scripts.yaml.j2 b/roles/installer/templates/configmaps/pre_stop_scripts.yaml.j2 new file mode 100644 index 00000000..4aeff8c4 --- /dev/null +++ b/roles/installer/templates/configmaps/pre_stop_scripts.yaml.j2 @@ -0,0 +1,16 @@ +{% if termination_grace_period_seconds is defined %} +apiVersion: v1 +kind: ConfigMap +metadata: + name: '{{ ansible_operator_meta.name }}-{{ deployment_type }}-pre-stop-scripts' + namespace: '{{ ansible_operator_meta.namespace }}' + labels: + {{ lookup("template", "../common/templates/labels/common.yaml.j2") | indent(width=4) | trim }} +data: + termination-master: | + {{ lookup("file", "files/pre-stop/termination-master") | indent(width=4) | trim }} + termination-waiter: | + {{ lookup("file", "files/pre-stop/termination-waiter") | indent(width=4) | trim }} + termination-env: | + {{ lookup("file", "files/pre-stop/termination-env") | indent(width=4) | trim }} +{% endif %} diff --git a/roles/installer/templates/deployments/deployment.yaml.j2 b/roles/installer/templates/deployments/deployment.yaml.j2 index 81e0a519..3c5de60f 100644 --- a/roles/installer/templates/deployments/deployment.yaml.j2 +++ b/roles/installer/templates/deployments/deployment.yaml.j2 @@ -23,6 +23,7 @@ spec: annotations: {% for template in [ "configmaps/config", + "configmaps/pre_stop_scripts", "secrets/app_credentials", "storage/persistent", ] %} @@ -139,6 +140,23 @@ spec: mountPath: "/var/run/redis" - name: "{{ ansible_operator_meta.name }}-redis-data" mountPath: "/data" +{% if termination_grace_period_seconds is defined %} + - name: pre-stop-data + mountPath: /var/lib/pre-stop + - name: pre-stop-scripts + mountPath: /var/lib/pre-stop/scripts + lifecycle: + preStop: + exec: + command: + - bash + - -c + # redis image doesn't support writing to `/proc/1/fd/*` + - > + PRE_STOP_STDOUT=/dev/stdout + PRE_STOP_STDERR=/dev/stderr + /var/lib/pre-stop/scripts/termination-waiter +{% endif %} resources: {{ redis_resource_requirements }} - image: '{{ _image }}' name: '{{ ansible_operator_meta.name }}-web' @@ -226,6 +244,18 @@ spec: {% endif %} {% if web_extra_volume_mounts -%} {{ web_extra_volume_mounts | indent(width=12, first=True) }} +{% endif %} +{% if termination_grace_period_seconds is defined %} + - name: pre-stop-data + mountPath: /var/lib/pre-stop + - name: pre-stop-scripts + mountPath: /var/lib/pre-stop/scripts + lifecycle: + preStop: + exec: + command: + - bash + - /var/lib/pre-stop/scripts/termination-waiter {% endif %} env: - name: MY_POD_NAMESPACE @@ -308,6 +338,18 @@ spec: {% endif %} {% if task_extra_volume_mounts -%} {{ task_extra_volume_mounts | indent(width=12, first=True) }} +{% endif %} +{% if termination_grace_period_seconds is defined %} + - name: pre-stop-data + mountPath: /var/lib/pre-stop + - name: pre-stop-scripts + mountPath: /var/lib/pre-stop/scripts + lifecycle: + preStop: + exec: + command: + - bash + - /var/lib/pre-stop/scripts/termination-master {% endif %} env: - name: SUPERVISOR_WEB_CONFIG_PATH @@ -377,6 +419,18 @@ spec: mountPath: "/var/lib/awx/projects" {% if ee_extra_volume_mounts -%} {{ ee_extra_volume_mounts | indent(width=12, first=True) }} +{% endif %} +{% if termination_grace_period_seconds is defined %} + - name: pre-stop-data + mountPath: /var/lib/pre-stop + - name: pre-stop-scripts + mountPath: /var/lib/pre-stop/scripts + lifecycle: + preStop: + exec: + command: + - bash + - /var/lib/pre-stop/scripts/termination-waiter {% endif %} env: {% if development_mode | bool %} @@ -412,6 +466,9 @@ spec: {% if security_context_settings|length %} {{ security_context_settings | to_nice_yaml | indent(8) }} {% endif %} +{% endif %} +{% if termination_grace_period_seconds is defined %} + terminationGracePeriodSeconds: {{ termination_grace_period_seconds }} {% endif %} volumes: {% if bundle_ca_crt %} @@ -441,6 +498,14 @@ spec: items: - key: ldap-ca.crt path: 'ldap-ca.crt' +{% endif %} +{% if termination_grace_period_seconds is defined %} + - name: pre-stop-data + emptyDir: {} + - name: pre-stop-scripts + configMap: + name: '{{ ansible_operator_meta.name }}-{{ deployment_type }}-pre-stop-scripts' + defaultMode: 0775 {% endif %} - name: "{{ ansible_operator_meta.name }}-application-credentials" secret: From 49d1f00dbd5c2cdd840333e9de03b3789f45de0e Mon Sep 17 00:00:00 2001 From: Stanislav Zaprudskiy Date: Wed, 22 Feb 2023 10:40:34 +0100 Subject: [PATCH 7/7] Do not keep web container running during termination We could let the web container terminate as usual, as there are no reasons to keep it running as it doesn't participate in job control. Additionally, it stops receiving traffic with the beginning of termination > At the same time as the kubelet is starting graceful shutdown, the > control plane removes that shutting-down Pod from EndpointSlice (and > Endpoints) objects where these represent a Service with a configured > selector @ https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-termination --- .../templates/deployments/deployment.yaml.j2 | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/roles/installer/templates/deployments/deployment.yaml.j2 b/roles/installer/templates/deployments/deployment.yaml.j2 index 3c5de60f..32670640 100644 --- a/roles/installer/templates/deployments/deployment.yaml.j2 +++ b/roles/installer/templates/deployments/deployment.yaml.j2 @@ -244,18 +244,6 @@ spec: {% endif %} {% if web_extra_volume_mounts -%} {{ web_extra_volume_mounts | indent(width=12, first=True) }} -{% endif %} -{% if termination_grace_period_seconds is defined %} - - name: pre-stop-data - mountPath: /var/lib/pre-stop - - name: pre-stop-scripts - mountPath: /var/lib/pre-stop/scripts - lifecycle: - preStop: - exec: - command: - - bash - - /var/lib/pre-stop/scripts/termination-waiter {% endif %} env: - name: MY_POD_NAMESPACE