From 078a49693753eb054b1ebd3c4f0578b7bf4af7f0 Mon Sep 17 00:00:00 2001 From: Dustin Specker Date: Wed, 22 Jun 2022 11:28:18 -0500 Subject: [PATCH] upgrading umbrella w/o changes has no app changes If a Helm upgrade is performed on the OpenStack Umbrella chart using the exact same configuration as the first release, then it's expected for no DaemonSets, Deployments, or StatefulSets to be updated. This did not work as expected. A few changes were required to support this desired behavior: 1. Update glance's configmap-etc.yaml to trim whitespace and convert YAML comment to Helm template comment. Before this change, Helm rendered the template with the YAML comment and a newline for the install phase. On upgrades, Helm rendered the template without the YAML comment and newline causing the hash of configmap-etc to change, thus causing the glance-api Deployment to update. 2. Update openstack.sh script to create a randomly generated memcache secret for glance. Without this change, the glance-api deployment changes each time since Helm randomly generates a new memcache secret if not provided. This behavior is enforced via a new test script, validate-umbrella-upgrade-no-side-effects.sh. The following jobs are always recreated due to hooks: - keystone-bootstrap - keystone-credential-setup - keystone-db-init - keystone-db-sync - keystone-domain-manage - keystone-fernet-setup - keystone-rabbit-init - rabbitmq-cluster-wait Some Jobs are created via CronJobs and could be created during validation. So far, heat-engine-cleaner has been seen, but others could be caught too. So the validation script ignores these pod changes by ignoring if Jobs were recreated. Plus Jobs being recreated should not impact the OpenStack deployment. Change-Id: Iffaa346d814b8d0a3e2292849943219f70d50a23 --- glance/Chart.yaml | 2 +- glance/templates/configmap-etc.yaml | 10 +++-- releasenotes/notes/glance.yaml | 1 + .../deployment/component/common/openstack.sh | 1 + ...lidate-umbrella-upgrade-no-side-effects.sh | 44 +++++++++++++++++++ zuul.d/jobs-openstack-helm.yaml | 1 + 6 files changed, 54 insertions(+), 5 deletions(-) create mode 100755 tools/gate/tests/validate-umbrella-upgrade-no-side-effects.sh diff --git a/glance/Chart.yaml b/glance/Chart.yaml index abc6623747..35efb0c61e 100644 --- a/glance/Chart.yaml +++ b/glance/Chart.yaml @@ -14,7 +14,7 @@ apiVersion: v1 appVersion: v1.0.0 description: OpenStack-Helm Glance name: glance -version: 0.3.6 +version: 0.3.7 home: https://docs.openstack.org/glance/latest/ icon: https://www.openstack.org/themes/openstack/images/project-mascots/Glance/OpenStack_Project_Glance_vertical.png sources: diff --git a/glance/templates/configmap-etc.yaml b/glance/templates/configmap-etc.yaml index 24aa24b062..0a779f0723 100644 --- a/glance/templates/configmap-etc.yaml +++ b/glance/templates/configmap-etc.yaml @@ -117,10 +117,12 @@ limitations under the License. {{- $endpointScheme := tuple "dashboard" "public" "web" . | include "helm-toolkit.endpoints.keystone_endpoint_scheme_lookup" }} {{- $endpointHost := tuple "dashboard" "public" . | include "helm-toolkit.endpoints.endpoint_host_lookup" }} {{- $endpointPort := tuple "dashboard" "public" "web" . | include "helm-toolkit.endpoints.endpoint_port_lookup" }} - # Common browsers don't add default ports like 80 and 443 to the headers - # and URLs therefore CORS should allow to use URLs both with 80,443 and - # without it in the URL. - {{- if eq $endpointPort "80" "443" }} + {{- if eq $endpointPort "80" "443" -}} + {{/* + Common browsers don't add default ports like 80 and 443 to the headers + and URLs therefore CORS should allow to use URLs both with 80,443 and + without it in the URL. + */}} {{- $_ := set $envAll.Values.conf.glance.cors "allowed_origin" ( list ) }} {{- $__allowed_origin := append $envAll.Values.conf.glance.cors.allowed_origin (printf "%s://%s" $endpointScheme $endpointHost) }} {{- $_ := set $envAll.Values.conf.glance.cors "allowed_origin" $__allowed_origin }} diff --git a/releasenotes/notes/glance.yaml b/releasenotes/notes/glance.yaml index 34a1a7e6e0..0785c0c8e6 100644 --- a/releasenotes/notes/glance.yaml +++ b/releasenotes/notes/glance.yaml @@ -27,4 +27,5 @@ glance: - 0.3.4 Change image default version to wallaby - 0.3.5 Migrated PodDisruptionBudget resource to policy/v1 API version - 0.3.6 Add Xena and Yoga values overrides + - 0.3.7 Fix glance-etc template changing due to comment and whitespace between install and first upgrade ... diff --git a/tools/deployment/component/common/openstack.sh b/tools/deployment/component/common/openstack.sh index 3bf31f5355..b6fa86daa1 100755 --- a/tools/deployment/component/common/openstack.sh +++ b/tools/deployment/component/common/openstack.sh @@ -102,6 +102,7 @@ helm upgrade --install $release openstack/ \ ${OSH_EXTRA_HELM_ARGS_LIBVIRT_CGROUP} \ ${OSH_EXTRA_HELM_VIRT_ARGS} \ ${OSH_EXTRA_HELM_ARGS} \ + --set glance.conf.glance.keystone_authtoken.memcache_secret_key="$(openssl rand -hex 64)" \ --set nova.bootstrap.wait_for_computes.enabled=true \ --set libvirt.conf.ceph.enabled=${CEPH_ENABLED} \ --set nova.conf.ceph.enabled=${CEPH_ENABLED} \ diff --git a/tools/gate/tests/validate-umbrella-upgrade-no-side-effects.sh b/tools/gate/tests/validate-umbrella-upgrade-no-side-effects.sh new file mode 100755 index 0000000000..6e34e6b2b8 --- /dev/null +++ b/tools/gate/tests/validate-umbrella-upgrade-no-side-effects.sh @@ -0,0 +1,44 @@ +#!/bin/bash +set -ex + +# This test confirms that upgrading a OpenStack Umbrella Helm release using +# --reuse-values does not result in any unexpected pods from being recreated. +# Ideally, no pods would be created if the upgrade has no configuration change. +# Unfortunately, some jobs have hooks defined such that each Helm release deletes +# and recreates jobs. These jobs are ignored in this test. +# This test aims to validate no Deployment, DaemonSet, or StatefulSet pods are +# changed by verifying the Observed Generation remains the same. + +# This test case is proven by: +# 1. getting the list of DaemonSets, Deployment, StatefulSets after an installation +# 2. performing a helm upgrade with --reuse-values +# 3. getting the list of DaemonSets, Deployment, StatefulSets after the upgrade +# 4. Verifying the list is empty since no applications should have changed + +before_apps_list="$(mktemp)" +after_apps_list="$(mktemp)" + +kubectl get daemonsets,deployments,statefulsets \ + --namespace openstack \ + --no-headers \ + --output custom-columns=Kind:.kind,Name:.metadata.name,Generation:.status.observedGeneration \ + > "$before_apps_list" + +helm upgrade openstack ./openstack \ + --namespace openstack \ + --reuse-values \ + --wait + +kubectl get daemonsets,deployments,statefulsets \ + --namespace openstack \ + --no-headers \ + --output custom-columns=Kind:.kind,Name:.metadata.name,Generation:.status.observedGeneration \ + > "$after_apps_list" + +# get list of apps that exist in after list, but not in before list +changed_apps="$(comm -13 "$before_apps_list" "$after_apps_list")" + +if [ "x$changed_apps" != "x" ]; then + echo "Applications changed unexpectedly: $changed_apps" + exit 1 +fi diff --git a/zuul.d/jobs-openstack-helm.yaml b/zuul.d/jobs-openstack-helm.yaml index 9c7f12f9e3..085471c4a3 100644 --- a/zuul.d/jobs-openstack-helm.yaml +++ b/zuul.d/jobs-openstack-helm.yaml @@ -198,6 +198,7 @@ - ./tools/deployment/developer/common/170-setup-gateway.sh - - ./tools/deployment/developer/common/900-use-it.sh - ./tools/deployment/common/force-cronjob-run.sh + - ./tools/gate/tests/validate-umbrella-upgrade-no-side-effects.sh - job: name: openstack-helm-compute-kit-ussuri-ubuntu_bionic