From 4df7129a387a2bebb0e0e77d4756f00d42ad0aa0 Mon Sep 17 00:00:00 2001 From: Jing Qi Date: Thu, 5 Dec 2024 11:58:48 +0800 Subject: [PATCH] feat(RELEASE-1252): make run-file-updates task idempotent Signed-off-by: Jing Qi The PR is to make the task idempotent by ensuring that a new MR isn't created unnecessarily when one already exists with the same content in the seed and replacements of paths parameter. And it added some logic to check the keys in the paths parameter. If the keys exist but their content does not require any replacement, return success. --- .../tasks/process-file-updates-task/README.md | 3 + .../process-file-updates-task.yaml | 62 ++++++++++- .../process-file-updates-task/tests/mocks.sh | 22 ++++ ...-file-updates-replacements-idempotent.yaml | 104 ++++++++++++++++++ 4 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 internal/tasks/process-file-updates-task/tests/test-process-file-updates-replacements-idempotent.yaml diff --git a/internal/tasks/process-file-updates-task/README.md b/internal/tasks/process-file-updates-task/README.md index 2c53d516e..99b6f0e31 100644 --- a/internal/tasks/process-file-updates-task/README.md +++ b/internal/tasks/process-file-updates-task/README.md @@ -16,6 +16,9 @@ replacements to a yaml file that already exists. It will attempt to create a Mer | tempDir | temp dir for cloning and updates | Yes | /tmp/$(context.taskRun.uid)/file-updates | | internalRequestPipelineRunName | name of the PipelineRun that called this task | No | - | +## Changes in 0.1.0 +* make run-file-updates task idempotent + ## Changes in 0.0.2 * add new `internalRequestPipelineRunName` parameter and result - Tekton only supports passing task results as pipeline results, diff --git a/internal/tasks/process-file-updates-task/process-file-updates-task.yaml b/internal/tasks/process-file-updates-task/process-file-updates-task.yaml index 739fdc6e3..c84bd4b7f 100644 --- a/internal/tasks/process-file-updates-task/process-file-updates-task.yaml +++ b/internal/tasks/process-file-updates-task/process-file-updates-task.yaml @@ -4,7 +4,7 @@ kind: Task metadata: name: process-file-updates-task labels: - app.kubernetes.io/version: "0.0.2" + app.kubernetes.io/version: "0.1.0" annotations: tekton.dev/pipelines.minVersion: "0.12.1" tekton.dev/tags: release @@ -160,6 +160,7 @@ spec: exit 1 fi + keyNotFound=false for (( REPLACEMENT_INDEX=0; REPLACEMENT_INDEX < REPLACEMENTS_LENGTH; REPLACEMENT_INDEX++ )); do echo "REPLACEMENT: #${REPLACEMENT_INDEX}" key="$(jq -cr ".[${PATH_INDEX}].replacements[${REPLACEMENT_INDEX}].key" "${updatePathsTmpfile}")" @@ -173,6 +174,7 @@ spec: foundAt=$(head -n 1 "${TEMP}/found.txt") if (( foundAt == 0 )); then echo "NOT FOUND" + keyNotFound=true continue fi echo "FOUND" @@ -236,14 +238,66 @@ spec: error="\"no replacements were performed\"" \ yq -o json --null-input '.str = strenv(error), .error = strenv(error)' \ | tee "$(results.fileUpdatesInfo.path)" - echo -n "Failed" |tee "$(results.fileUpdatesState.path)" + if [[ "$keyNotFound" == true ]]; then + echo -n "Failed" |tee "$(results.fileUpdatesState.path)" + else + echo -n "Success" |tee "$(results.fileUpdatesState.path)" + fi # it should exit 0 otherwise the task does not set the results # this way the InternalRequest can see what was wrong exit 0 fi echo -e "\n*** START LOCAL CHANGES ***\n" - git diff + git diff | tee "${TEMP}"/tempMRFile.diff + + if [[ -s "${TEMP}"/tempMRFile.diff ]]; then + # replacements exist + # It's to deal with the lines like "@@ -N1,N2 +N3,N4 @@", + # but it's possible to meet a line like "@@ -4,7 +4,7 @@ $schema: /app/app-settings.yml". + awk '/^@@/ {match($0, /@@ ([^@]+) @@/, arr); print arr[1]}' "${TEMP}"/tempMRFile.diff |\ + tee "${TEMP}"/lineFile + fi + + openMRList=$(glab mr list -R "${UPSTREAM_REPO}" --search "Konflux release" |grep "^!"| cut -f1 | tr -d '!') + for mrNum in ${openMRList} ; do + foundMR=false + glab mr diff "${mrNum}" --repo "${UPSTREAM_REPO}" > "${TEMP}"/oneMR.diff + + if [[ ! -s "${TEMP}"/lineFile ]]; then + # only seed files + grep '^[+][^+]' "${TEMP}"/oneMR.diff | sed -E 's/^[+]//' | tee "${TEMP}/mrFile" + + # compare if the contents and the updated file names are the same + if diff -q "${TEMP}"/mrFile "${targetFile}" > /dev/null; then + grep "^diff --git" "${TEMP}"/oneMR.diff | tee "${TEMP}"/tmpFile + while read -r line ; do + changedFileName=$(echo "$line" | awk '{sub(/^b\//, "", $NF); print $NF}') + if [[ "${changedFileName}" != "${targetFile}" ]]; then + continue + fi + foundMR=true + break + done < "${TEMP}"/tmpFile + fi + else + # replacements exist + grep '^[-+@]' "${TEMP}"/oneMR.diff | sed -E 's/^[@]//; s/@@.*//' | tee "${TEMP}/mrFile" + grep '^[-+@]' "${TEMP}"/tempMRFile.diff | sed -E 's/^[@]//; s/@@.*//' | tee "${TEMP}/tmpFile" + if diff -q "${TEMP}"/mrFile "${TEMP}"/tmpFile > /dev/null; then + foundMR=true + fi + fi + # if all the lines are matched, the MR is the same one + # it should exit 0 if the same MR exists + if [[ "$foundMR" == true ]]; then + echo "there is an existing MR with the same updates in the repo" + echo "{\"merge_request\":\"${UPSTREAM_REPO}/-/merge_requests/${mrNum}\"}" \ + | tee -a "$(results.fileUpdatesInfo.path)" + echo -n "Success" | tee "$(results.fileUpdatesState.path)" + exit 0 + fi + done echo -e "\n*** END LOCAL CHANGES ***\n" WORKING_BRANCH=$(uuidgen |awk '{print substr($1, 1, 8)}') @@ -253,7 +307,7 @@ spec: GITLAB_MR_MSG="[Konflux release] $(params.application): fileUpdates changes ${WORKING_BRANCH}" gitlab_create_mr --head "$WORKING_BRANCH" --target-branch "$REVISION" --title "${GITLAB_MR_MSG}" \ --description "${GITLAB_MR_MSG}" --upstream-repo "${UPSTREAM_REPO}" | jq '. | tostring' \ - |tee -a "$(results.fileUpdatesInfo.path)" + | tee -a "$(results.fileUpdatesInfo.path)" echo -n "Success" |tee "$(results.fileUpdatesState.path)" diff --git a/internal/tasks/process-file-updates-task/tests/mocks.sh b/internal/tasks/process-file-updates-task/tests/mocks.sh index c693b9502..3aabbd637 100644 --- a/internal/tasks/process-file-updates-task/tests/mocks.sh +++ b/internal/tasks/process-file-updates-task/tests/mocks.sh @@ -28,11 +28,33 @@ function git() { if [[ "$*" == "config"* ]]; then /usr/bin/git "$@" fi + if [[ "$*" == "diff"* ]]; then + /usr/bin/git "$@" + fi } function glab() { if [[ "$*" == *"mr create"* ]]; then gitRepo=$(echo "$*" | cut -f5 -d/ | cut -f1 -d.) echo "/merge_request/1" + elif [[ "$*" == *"mr list"* ]]; then + echo '!1' + elif [[ "$*" == *"mr diff"* ]]; then + gitRepo=$(echo "$*" | cut -f5 -d/ | cut -f1 -d.) + if [[ "${gitRepo}" == "replace-idempotent" ]]; then + echo "diff --git a/addons/my-addon2.yaml b/addons/my-addon2.yaml +--- a/addons/my-addon2.yaml ++++ b/addons/my-addon2.yaml +@@ -1,2 +1,2 @@ +-indexImage: ++indexImage: Jack +" + else + echo "diff --git a/test/one-update.yaml b/test/one-update.yaml ++++ b/test/one-update.yaml +@@ -1,2 +1,2 @@ ++indexImage: Jack +" + fi fi } diff --git a/internal/tasks/process-file-updates-task/tests/test-process-file-updates-replacements-idempotent.yaml b/internal/tasks/process-file-updates-task/tests/test-process-file-updates-replacements-idempotent.yaml new file mode 100644 index 000000000..d8ab45942 --- /dev/null +++ b/internal/tasks/process-file-updates-task/tests/test-process-file-updates-replacements-idempotent.yaml @@ -0,0 +1,104 @@ +--- +apiVersion: tekton.dev/v1 +kind: Pipeline +metadata: + name: test-process-file-updates-replacements-idempotent +spec: + description: | + Run the process-file-updates task with replacements. No commit + is created if there is a mocked MR with the same content. + workspaces: + - name: tests-workspace + tasks: + - name: setup + workspaces: + - name: pipeline + workspace: tests-workspace + taskSpec: + workspaces: + - name: pipeline + steps: + - name: setup-values + image: quay.io/konflux-ci/release-service-utils:e633d51cd41d73e4b3310face21bb980af7a662f + script: | + #!/usr/bin/env bash + set -eux + + mkdir -p "$(workspaces.pipeline.path)/$(context.pipelineRun.uid)/file-updates" + cd "$(workspaces.pipeline.path)/$(context.pipelineRun.uid)/file-updates" + mkdir replace-idempotent + cd replace-idempotent + git config --global init.defaultBranch main + git init . + git config --global user.email "test@test.com" + git config --global user.name "tester" + + mkdir addons + cat > "addons/my-addon2.yaml" << EOF + indexImage: + name: test + EOF + git add addons/my-addon2.yaml + git commit -m "prior commit" + - name: run-task + taskRef: + name: process-file-updates-task + params: + - name: upstream_repo + value: "https://some.gitlab/test/replace-idempotent" + - name: repo + value: "https://some.gitlab/test/replace-idempotent" + - name: ref + value: "main" + - name: paths + value: >- + [{"path":"addons/my-addon2.yaml","replacements":[{"key":".indexImage", + "replacement":"|indexImage.*|indexImage: Jack|"}]}] + - name: application + value: "scott" + - name: file_updates_secret + value: "file-updates-secret" + - name: tempDir + value: "$(workspaces.pipeline.path)/$(context.pipelineRun.uid)/file-updates" + - name: internalRequestPipelineRunName + value: $(context.pipelineRun.name) + workspaces: + - name: pipeline + workspace: tests-workspace + runAfter: + - setup + - name: check-result + runAfter: + - run-task + params: + - name: fileUpdatesInfo + value: $(tasks.run-task.results.fileUpdatesInfo) + - name: fileUpdatesState + value: $(tasks.run-task.results.fileUpdatesState) + - name: tempDir + value: "$(workspaces.pipeline.path)/$(context.pipelineRun.uid)/file-updates" + taskSpec: + params: + - name: fileUpdatesInfo + type: string + - name: fileUpdatesState + type: string + - name: tempDir + type: string + steps: + - name: check-result + image: quay.io/konflux-ci/release-service-utils:e633d51cd41d73e4b3310face21bb980af7a662f + script: | + #!/usr/bin/env bash + set -eux + + cd "$(params.tempDir)/replace-idempotent" + commits=$(git log --oneline | wc -l) + + echo "Test no more commit created except the one in setup step" + test "${commits}" == "1" + rm -rf "$(params.tempDir)" + + workspaces: + - name: pipeline + workspace: tests-workspace