From 91ab3161423264d7011efb8b45ca7f73e589a868 Mon Sep 17 00:00:00 2001 From: Clark Boylan Date: Fri, 7 Apr 2023 08:45:53 -0700 Subject: [PATCH] Fix gerrit upgrade config diff checking There were two problems with our gerrit upgrade config diff checking. The first is that we were comparing using command exit codes after pipeing diff to tee without setting pipefail. This meant that even if the diff failed we got an exit code of 0 from tee and everything passed happily. Second we were not checking our pre gerrit state diff state. If the old gerrit version also updated the config on disk we wouldn't get a diff after upgrading to the new version. I think that is ultimately what broke for us here because the 3.6 and 3.7 config diffs are empty, but they differ from what we initially write to disk. As for explaining why this might happen I can only assume some update to 3.6 made the changes we saw after we had deployed 3.6. As a result of checking things more thoroughly we need to update our config to remove any delta. This removes some extraneous quoting around gitweb config entries to do that. Change-Id: I9c7e73d5f64546fb57a21a249b29e2aca9229ac7 --- .../roles/gerrit/templates/gerrit.config.j2 | 16 ++--- playbooks/zuul/upgrade-review.yaml | 66 ++++++++++++++++--- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/playbooks/roles/gerrit/templates/gerrit.config.j2 b/playbooks/roles/gerrit/templates/gerrit.config.j2 index cecf699485..cbebbaf71c 100644 --- a/playbooks/roles/gerrit/templates/gerrit.config.j2 +++ b/playbooks/roles/gerrit/templates/gerrit.config.j2 @@ -144,17 +144,17 @@ allowTopMenu = false # Custom code browser linking for Gitea. [gitweb] - branch = "${project}/src/branch/${branch}" - file = "${project}/src/commit/${hash}/${file}" - filehistory = "${project}/commits/branch/${branch}/${file}" + branch = ${project}/src/branch/${branch} + file = ${project}/src/commit/${hash}/${file} + filehistory = ${project}/commits/branch/${branch}/${file} linkname = gitea - project = "${project}" - revision = "${project}/commit/${commit}" - roottree = "${project}/src/commit/${commit}" + project = ${project} + revision = ${project}/commit/${commit} + roottree = ${project}/src/commit/${commit} type = custom urlEncode = false - tag = "${project}/src/tag/${tag}" - url = "https://opendev.org/" + tag = ${project}/src/tag/${tag} + url = https://opendev.org/ [index] type = LUCENE threads = 8 diff --git a/playbooks/zuul/upgrade-review.yaml b/playbooks/zuul/upgrade-review.yaml index 8db3a2f375..08f111807e 100644 --- a/playbooks/zuul/upgrade-review.yaml +++ b/playbooks/zuul/upgrade-review.yaml @@ -4,7 +4,35 @@ # # In particular it bootstraps users and test changes assuming a test env. -- name: Ensure initial gerrit state +- name: Ensure initial gerrit state without starting Gerrit + import_playbook: ../service-review.yaml + vars: + gerrit_container_image: docker.io/opendevorg/gerrit:3.6 + gerrit_run_init: false + gerrit_run_reindex: false + gerrit_run_compose_up: false + +- hosts: "review:!disabled" + name: "Record pre Gerrit deployment configuration state" + tasks: + # This allows us to check that our config file isn't modified by newer + # Gerrit versions. + - name: Backup config files pre deploy + block: + - name: Find .config files + find: + paths: /home/gerrit2/review_site/etc + patterns: '*.config' + register: _config_files_pre_deploy + + - name: 'Backup config file' + copy: + src: '{{ item }}' + dest: '{{ item }}.pre-deploy' + remote_src: true + loop: "{{ _config_files_pre_deploy.files | map(attribute='path') | list }}" + +- name: Start Gerrit on the old version of Gerrit import_playbook: ../service-review.yaml vars: gerrit_container_image: docker.io/opendevorg/gerrit:3.6 @@ -30,20 +58,20 @@ # This allows us to check that our config file isn't modified by newer # Gerrit versions. - - name: Backup config files + - name: Backup config files pre upgrade block: - name: Find .config files find: paths: /home/gerrit2/review_site/etc patterns: '*.config' - register: _config_files + register: _config_files_pre_upgrade - name: 'Backup config file' copy: src: '{{ item }}' dest: '{{ item }}.pre-upgrade' remote_src: true - loop: "{{ _config_files.files | map(attribute='path') | list }}" + loop: "{{ _config_files_pre_upgrade.files | map(attribute='path') | list }}" # Record h2 cache files. We will use this to highlight any new caches # under the new Gerrit version. @@ -64,17 +92,35 @@ - hosts: "review:!disabled" name: "Post upgrade config check" tasks: - - name: Diff config files + - name: Diff config files pre deploy to pre upgrade shell: | - diff -u {{ item }}.pre-upgrade {{ item }} | tee {{ item }}.diff - loop: "{{ _config_files.files | map(attribute='path') | list }}" - register: _diff_output + set -o pipefail + diff -u {{ item }}.pre-deploy {{ item }}.pre-upgrade | tee {{ item }}.deploy.diff + args: + executable: /bin/bash + loop: "{{ _config_files_pre_upgrade.files | map(attribute='path') | list }}" + register: _diff_output_deploy - - name: Check config diffs + - name: Diff config files pre upgrade to post upgrade + shell: | + set -o pipefail + diff -u {{ item }}.pre-upgrade {{ item }} | tee {{ item }}.upgrade.diff + args: + executable: /bin/bash + loop: "{{ _config_files_pre_upgrade.files | map(attribute='path') | list }}" + register: _diff_output_upgrade + + - name: Check the config diffs after deployment fail: msg: 'Difference detected in file {{ item.item }} ' when: item.rc != 0 - loop: '{{ _diff_output.results }}' + loop: '{{ _diff_output_deploy.results }}' + + - name: Check the config diffs after upgrade + fail: + msg: 'Difference detected in file {{ item.item }} ' + when: item.rc != 0 + loop: '{{ _diff_output_upgrade.results }}' - name: Record Gerrit new cache files find: