Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/block96 Undo the changes made in case of pipeline generation failure #108

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
100 changes: 97 additions & 3 deletions scripts/pipelines/azure-devops/pipeline_generator.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ red='\e[0;31m'
# Common var
commonTemplatesPath="scripts/pipelines/azure-devops/templates/common"

# Undo-Level for the script. Used to clean up the resources in case of a failure
undoStage=0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment will work but I'd put this one either at the start of the "importConfigFile" function body or I'd put all undoStage assignments at the bottom between where the functions are called. Doesn't matter what you choose but better be consistent, I guess.


function help {
echo ""
echo "Generates a pipeline on Azure DevOps based on the given definition."
Expand Down Expand Up @@ -152,11 +155,18 @@ function createNewBranch {
echo -e "${green}Creating the new branch: ${sourceBranch}..."
echo -ne ${white}

# Create the new branch.
cd "${localDirectory}"

# store current branch into a variable (only used for rollback/undo)
originalBranch=$(git branch --show-current)

[ $? != "0" ] && echo -e "${red}The local directory: '${localDirectory}' cannot be found, please check the path." && exit 1

# Create the new branch.
git checkout -b ${sourceBranch}

# set undo-stage (clear local branches)
undoStage=1
}

function copyYAMLFile {
Expand Down Expand Up @@ -201,14 +211,30 @@ function commitCommonFiles {

git commit -m "Adding the source YAML"
git push -u origin ${sourceBranch}

# clean up remote-branches
undoStage=2
}

function createPipeline {
echo -e "${green}Generating the pipeline from the YAML template..."
echo -ne ${white}

# Create Azure Pipeline
az pipelines create --name $pipelineName --yml-path "${pipelinePath}/${yamlFile}" --skip-first-run true

pipelineResult=$(az pipelines create --name $pipelineName --yml-path "${pipelinePath}/${yamlFile}" --skip-first-run true) || {
# if the script fails, clean pollution
echo "There was an error creating the pipeline! Please check if you specified the name and got all settings right!"

undoPreviousSteps

exit 127
}

pipelineId=$(echo "$pipelineResult" | python -c "import sys, json; print(json.load(sys.stdin)['id'])")

# pipeline got created successfully
undoStage=3
}

# Function that adds the variables to be used in the pipeline.
Expand All @@ -218,7 +244,15 @@ function addCommonPipelineVariables {
echo "Skipping creation of the variable artifactPath as the flag has not been used."
else
# Add the extra artifact to store variable.
az pipelines variable create --name "artifactPath" --pipeline-name "$pipelineName" --value "${artifactPath}"
variableResult=$(az pipelines variable create --pipeline-name "$pipelineName" --value "${artifactPath}") || {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SC2034: variableResult appears unused. Verify use (or export if used externally).

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
variableResult=$(az pipelines variable create --pipeline-name "$pipelineName" --value "${artifactPath}") || {
az pipelines variable create --pipeline-name "$pipelineName" --value "${artifactPath}" || {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is there a reason for the assignment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the variable result - not at all. Regarding the pipeline though I need it because I need to extract the id :) Azures Rest API uses the id of the pipeline to delete it :)

# if the variable-creation fails, clean up

echo "There was an error creating the pipeline-artifact-path variable!. Exiting"

undoPreviousSteps

exit 127
}
fi
}

Expand Down Expand Up @@ -272,6 +306,66 @@ function createPR {
fi
}

function removeLocalBranches {
# visit the directory and switch to the branch which was present before
cd "${localDirectory}"

git checkout $originalBranch
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SC2154: originalBranch is referenced but not assigned.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]


git branch -D ${sourceBranch}
}

function removeRemoteBranches {
cd "${localDirectory}"

# update list of remotes
git fetch

# delete mapped remote-branch
git push origin --delete ${sourceBranch}
}
Comment on lines +169 to +177
Copy link
Contributor

@patricpoba patricpoba May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have to delete the pull request created by the createPR step before removing the remote branch. that means you need another function to undo the createPR step.


function removePipelineFiles {
# clear the generated pipeline-files
cd "${localDirectory}"

if [ -d "./pipelines" ]; then
rm -rf ./pipelines
fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need more granularity than just removing it, as this would also undo any previous runs of the pipeline generator. What we want is to only remove it when there were no previous runs. If there were previous runs, we need to keep a copy of the previous state of the .pipeline, alternatively we could also check what type of config file is currently used and granularly delete only the files belonging to that config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really good suggestion that I need to apply! Thank you :) I just need to see how

}

function removePipeline {
az pipelines delete --id ${pipelineId}
}

function abandonPullRequest {
az repos pr update --id id --status abandoned
}

function undoPreviousSteps {
# free all resources

if [ ${undoStage} -gt 2 ]; then
echo "Removing pipeline with id: ${pipelineId}"

removePipeline
fi

if [ ${undoStage} -gt 1 ]; then
echo "Removing all pipeline-files and remote branches!"

removePipelineFiles
removePullRequest
removeRemoteBranches
fi

if [ ${undoStage} -gt 0 ]; then
echo "Removing all local branches!"

removeLocalBranches
fi
}

if [[ "$help" == "true" ]]; then help; fi

importConfigFile
Expand Down