Feature/block96 Undo the changes made in case of pipeline generation failure by BenjaminE98 · Pull Request #108 · devonfw/hangar

BenjaminE98 · 2022-05-25T09:24:19Z

This closes #96

If the script fails, we rollback the necessary changes like removing the remote branches. removing the pipeline and local branches.

The idea was to create a variable called undoStage. This sets the stages which need to be undone.

sonatype-lift · 2022-05-25T09:29:49Z

scripts/pipelines/azure-devops/pipeline_generator.sh

    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}") || {


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 ]

patricpoba · 2022-05-26T11:30:51Z

scripts/pipelines/azure-devops/pipeline_generator.sh

+    # Create the new branch.
    git checkout -b ${sourceBranch}
+
+    # clear local-branches


clear local-branches comment may be misleading for another developer.
may be something like set undo stage (clear local-branches)

patricpoba · 2022-05-26T12:00:29Z

scripts/pipelines/azure-devops/pipeline_generator.sh

+function removeRemoteBranches {
+    cd "${localDirectory}"
+
+    # update list of remotes
+    git fetch
+
+    # delete mapped remote-branch
+    git push origin --delete ${sourceBranch}
+}


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.

patricpoba · 2022-05-26T12:09:02Z

scripts/pipelines/azure-devops/pipeline_generator.sh

+    az pipelines delete --id ${pipelineId}
+}
+
+function clearPollution {


@BenjaminE98
I understand what you mean hear by the name clearPollution but I suggest renaming it to something more self explanatory like undoPreviousSteps or rollbackPreviousActions or something else :-)

patricpoba

The work done looks good to me. I left a few comments based on what I have seen so far.

Edit:
Also delete the docker credentials created by the script in AzureDevOps otherwise the subsequent script runs will fails.

shiftwolf · 2022-06-03T06:12:48Z

scripts/pipelines/azure-devops/pipeline_generator.sh

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


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.

shiftwolf · 2022-06-03T06:41:05Z

scripts/pipelines/azure-devops/pipeline_generator.sh

    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}") || {


Suggested change

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

az pipelines variable create --pipeline-name "$pipelineName" --value "${artifactPath}" || {

Or is there a reason for the assignment?

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 :)

shiftwolf · 2022-06-03T06:56:24Z

scripts/pipelines/azure-devops/pipeline_generator.sh

+    if [ -d "./pipelines" ]; then
+        rm -rf ./pipelines
+    fi


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.

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

sonatype-lift · 2022-06-03T14:41:03Z

scripts/pipelines/azure-devops/pipeline_generator.sh

    else
        # Add the extra artifact to store variable.
-        az pipelines variable create --name "artifactPath" --pipeline-name "$pipelineName" --value "${artifactPath}"
+        $(az pipelines variable create --pipeline-name "$pipelineName" --value "${artifactPath}") || {


SC2091: Remove surrounding $() to avoid executing output (or use eval if intentional).

(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 ]

sonatype-lift · 2022-06-08T06:45:53Z

scripts/pipelines/azure-devops/pipeline_generator.sh

+pipelinesDirectoryAlreadyExists=false
+
+if [ -d "./.pipelines" ]; then
+    pipelinesDirectoryAlreadyExists=true


SC2034: pipelinesDirectoryAlreadyExists 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 ]

scripts/pipelines/azure-devops/pipeline_generator.sh

sonatype-lift · 2022-07-06T13:34:36Z

scripts/pipelines/azure-devops/pipeline_generator.sh

+    # visit the directory and switch to the branch which was present before
+    cd "${localDirectory}"
+
+    git checkout $originalBranch


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 ]

BenjaminE98 added 6 commits May 18, 2022 17:31

first draft

e2477ef

Update pipeline_generator.sh

a5c193c

checking for errors and syntax cleanup

bea7817

fixed a bug in the undoStage-Comparison

1206935

comment clean up

61df1f4

Update pipeline_generator.sh

85731af

BenjaminE98 changed the title ~~Clear pipeline~~ Feature/block96 Undo the changes made in case of pipeline generation failure May 25, 2022

BenjaminE98 requested a review from patricpoba May 25, 2022 09:26

sonatype-lift bot reviewed May 25, 2022

View reviewed changes

BenjaminE98 requested review from shiftwolf May 26, 2022 08:15

patricpoba reviewed May 26, 2022

View reviewed changes

rename pipeline and abandon pull request

1562b7a

shiftwolf reviewed Jun 3, 2022

View reviewed changes

case sensitiv file removal

7182244

sonatype-lift bot reviewed Jun 3, 2022

View reviewed changes

BenjaminE98 added 3 commits June 3, 2022 17:18

added Timos suggestions

c94f718

file removal

4a5107f

no need to clean files manually since git will do this

e817cc9

BenjaminE98 marked this pull request as ready for review June 8, 2022 06:45

sonatype-lift bot reviewed Jun 8, 2022

View reviewed changes

scripts/pipelines/azure-devops/pipeline_generator.sh Show resolved Hide resolved

BenjaminE98 and others added 3 commits June 24, 2022 13:51

unused variable

fa45009

Merge branch 'master' into clear_pipeline

339682c

Update pipeline_generator.sh

e0cdc15

sonatype-lift bot reviewed Jul 6, 2022

View reviewed changes

BenjaminE98 added 2 commits July 6, 2022 15:36

further workaround the changes

3ae8b43

Update pipeline_generator.sh

0012ee0

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

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

Conversation

BenjaminE98 commented May 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonatype-lift bot May 25, 2022

Choose a reason for hiding this comment

Uh oh!

patricpoba May 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patricpoba May 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patricpoba May 26, 2022

Choose a reason for hiding this comment

Uh oh!

patricpoba left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shiftwolf Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

shiftwolf Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

shiftwolf Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

BenjaminE98 Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

shiftwolf Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

BenjaminE98 Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

sonatype-lift bot Jun 3, 2022

Choose a reason for hiding this comment

Uh oh!

sonatype-lift bot Jun 8, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonatype-lift bot Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

BenjaminE98 commented May 25, 2022 •

edited

Loading

patricpoba May 26, 2022 •

edited

Loading

patricpoba May 26, 2022 •

edited

Loading

patricpoba left a comment •

edited

Loading