Azure / apiops

APIOps applies the concepts of GitOps and DevOps to API deployment. By using practices from these two methodologies, APIOps can enable everyone involved in the lifecycle of API design, development, and deployment with self-service and automated tools to ensure the quality of the specifications and APIs that they’re building.
https://azure.github.io/apiops
MIT License
309 stars 181 forks source link

[BUG] ADO extractor pipeline fails if there are no Git changes #196

Closed randallc79 closed 1 year ago

randallc79 commented 1 year ago

Release version

APIOps 4.0.1

Describe the bug

On branch artifacts-from-portal-build-9123
nothing to commit, working tree clean
Committing Git changes failed.
At D:\a\_temp\305a15a9-5f02-4d71-ae03-3cb2014854e3.ps1:65 char:28
+ if ($LASTEXITCODE -ne 0) { throw "Committing Git changes failed." }
+                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : OperationStopped: (Committing Git changes failed.:String) [], RuntimeException
+ FullyQualifiedErrorId : Committing Git changes failed.
##[debug]Exit code: 1
##[debug]Leaving Invoke-VstsTool.
##[error]PowerShell exited with code '1'.
##[debug]Processed: ##vso[task.logissue type=error]PowerShell exited with code '1'.
##[debug]Processed: ##vso[task.complete result=Failed]Error detected

Expected behavior

if 'nothing to commit, working tree clean' happen should clean exit, not fail exit.

Actual behavior

fails job, remove artifacts folder and ran again, to show clean run.

Reproduction Steps

Error when executing 'run-extractor' twice in a row. first time ran good, made a small change and re-ran to test change and job failed. Not sure if this has been reported before, I did a quick search and didn't see any bug report off hand.

guythetechie commented 1 year ago

@randallc79 - We have a check here to exit if there's nothing to publish. Looks like that check failed. Would you mind posting the full task output?

waelkdouh commented 1 year ago

@randallc79 please make sure you scrub your logs before posting to ensure no sensitive information gets leaked.

randallc79 commented 1 year ago

attaching a scrubbed log.

APIops_log_failure.txt

vishalJiaa commented 1 year ago

I am also facing the same issue but for me there is a change in files for pull request still facing the issue.

praveenvinay commented 1 year ago

+1

guythetechie commented 1 year ago

Can you please replace your run-publisher-with-env.yaml with this one and rerun? I'm interested in the output of the last task. Added extra logging to see what Git reports.

nathan-j-nd commented 1 year ago

I've also noticed this behavior in v4.0.4 in some of my projects (more detail in a bit)

@guythetechie This issue is with the extract pipeline, not the publish pipeline. I was going to try what you suggested, but realized it wouldn't make a difference if I'm understanding things correctly.

In the project that has this this behavior in I'm using a .gitignore file to ignore all directories except the apis folder. I'm also using an extractor config to get a specific api. Could the existing check for changes not be accounting for .gitignore?

Side note: It would be great if this tool could be configured to not pull down shared artifacts. Using .gitignore to do this for you is pretty hacky.

nathan-j-nd commented 1 year ago

Okay I tried some things and found this is most likely due to inconsistent line endings.

I found the following in @randallc79 's logs: warning: in the working copy of {filename}, LF will be replaced by CRLF the next time Git touches it I found the following in my logs: warning: in the working copy of '{filename}', CRLF will be replaced by LF the next time Git touches it

I added the following to the extract script, just after it sets the $gitStatus variable: Write-Information "Git Status Output... $gitStatus" Sure enough git status showed modified files that match the files in the warnings.

nathan-j-nd commented 1 year ago

I was able to fix this issue by moving the git status check below the add. see https://github.com/yaterchatter/apiops/commit/a6604e2b4b4ecdc1927de63fd9edc9d3e4204469

guythetechie commented 1 year ago

@yaterchatter - thanks for looking into this. And it makes sense that line endings would cause that issue.

My concern with your update is that it will now create unnecessary PRs if the only changes are line endings. I prefer the current code's behavior, which doesn't commit anything. The reason it fails is that git returns an exit code of 1 if there's nothing to commit.

PS C:\Users\source\repos\github\Azure\apiops> git commit
On branch 196-bug-ado-extractor-pipeline-fails-if-there-are-no-git-changes
Your branch is up to date with 'origin/196-bug-ado-extractor-pipeline-fails-if-there-are-no-git-changes'.

nothing to commit, working tree clean
PS C:\Users\source\repos\github\Azure\apiops> $LASTEXITCODE
1

This throw then gets called: https://github.com/Azure/apiops/blob/93755d61dddf27d6d8de470c7858f96f22a42d8b/tools/azdo_pipelines/run-extractor.yaml#L209

I would fix this by updating the line above to exit successfully if there's nothing to commit. Something like:

if ($LASTEXITCODE -ne 0) && ($commitOutput does not say "Nothing to commit") {throw}

Would you be comfortable updating your PR to account for this? If not, happy to do it myself.

Thanks again.

randallc79 commented 1 year ago

Just updated my code to be ---

            Write-Information "Committing changes"
            git -C "$temporaryFolderPath" commit --message "Initial commit"
            #if ($LASTEXITCODE -ne 0) { throw "Committing Git changes failed." }
            if ($LASTEXITCODE -ne 0) && ($commitOutput does not say "Nothing to commit") { throw "Committing Git changes failed." }

About to do some testing with it, if it works (which it looks like it should), I'll updated here.

[EDIT] -> Got a failure right off the back with that line, saying "&&" not a valid statement separator in this version.

after some editing... found this worked better, but still fails...

            Write-Information "Committing changes"
            $commitOutput = git -C "$temporaryFolderPath" commit --message "Initial commit"
            if (($LASTEXITCODE -ne 0) -and ($commitOutput -notlike "*nothing to commit*")) { throw }

---- OUTPUT ----

Validating that changes exist to be published...
Setting git user information...
Adding changes...
warning: in the working copy of 'apim_artifacts_Dev/apis/xxxxx/apiInformation.json', LF will be replaced by CRLF the next time Git touches it
...
warning: in the working copy of 'apim_artifacts_Dev/version sets/xxxxx/apiVersionSetInformation.json', LF will be replaced by CRLF the next time Git touches it
Committing changes
ScriptHalted
At D:\a\_temp\5c812132-2223-471c-9cb3-57562da12100.ps1:65 char:82
+ … -ne 0) -and ($commitOutput -notlike "*nothing to commit*")) { throw }
+                                                                 ~~~~~
+ CategoryInfo          : OperationStopped: (:) [], RuntimeException
+ FullyQualifiedErrorId : ScriptHalted
##[debug]Exit code: 1
##[debug]Leaving Invoke-VstsTool.
##[error]PowerShell exited with code '1'.
##[debug]Processed: ##vso[task.logissue type=error]PowerShell exited with code '1'.
##[debug]Processed: ##vso[task.complete result=Failed]Error detected
##[debug]Leaving D:\a\_tasks\PowerShell_e213ff0f-5d5c-4791-802d-52ea3e7be1f1\2.212.0\powershell.ps1.
waelkdouh commented 1 year ago

@randallc79 can we close this issue?

guythetechie commented 1 year ago

@randallc79 - @yaterchatter created a PR to address this issue. Should be part of our next release, but the code for it is currently in our main branch.