GitHub Action Deploy TRE fails on first run at ACR login step

[jonnyry]

Describe the bug

GitHub Action Deploy Azure TRE fails on first run at the 2nd ACR Login step with error $ec numeric argument required.

On subsequent runs it passes, as the 2nd ACR Login step is not executed, due to the 1st ACR Login step passing:

Steps to reproduce

1. Deploy the TRE using the "Deploy Azure TRE" GitHub Action for the first time (/to an empty Azure environment).
2. Observe the error above on first run.
3. Run a second time and observe the error does not occur.

Azure TRE release version (e.g. v0.14.0 or main)

main (v0.15.2)

[marrobi]

I believe this is this fix in the deployment repo - https://github.com/microsoft/AzureTRE-Deployment/pull/74/files#diff-5a2b3ccde8a4255fb43032fe938e96b572e2bee154824cab96b2027cb6a405c9R222 - need to make sure its applied everywhere needed.

[jonnyry]

Great, I'll rerun once merged.

[jonnyry]

@marrobi is there an upcoming release due that will include the fix in the PR above?

[marrobi]

Is it blocking you? Does a second run work? It's not something I've hit and deployed multiple PRs this week that cover this piece of code.

Are you using the deployment repo - if so it's a template - so you can just make the pipeline fix yourself until we do get this resolved.

[jonnyry]

No, it's not blocking - the second run is fine :-)

I was partly asking the question to understand from a more general perspective how external PRs are handled by the core team and if there is release schedule / roadmap you're working to.

[marrobi]

So security and blocking bug fixes are prioritised. Then this board should give a general idea as to what is being worked on - https://github.com/orgs/microsoft/projects/559 - albeit priorities may change. We aim to do a release month, or more frequently if required.

There is no core team working on a roadmap. Features/enhancements are only done when working directly with customers or based on PRs from external parties. This may change in the future but is the current situation.

Hope that helps.

[marrobi]

@nasaldrops @jonnyry I'm trying to work out the difference between the two approaches. I ask GitHub Copilot(!)...

---

Compare the following bash:

          for i in {1..3}; do
            az acr login --name "${{ secrets.CI_CACHE_ACR_NAME }}" && ec=0 && break || ec=\$? && sleep 10
          done
          # shellcheck disable=SC2242
          (exit ${ec})

with

          for i in {1..3}; do
            az acr login --name "${{ secrets.CI_CACHE_ACR_NAME }}" && ec=0 && break || ec="$?" && sleep 10
          done
          # shellcheck disable=SC2242
          (exit "$ec")

---

After some explanation is says:

In this specific context, both scripts will behave the same way as exit codes are integers and do not contain spaces or special characters. However, the second script follows better bash scripting practices.

So #3861 wins ;-) (and doesn't have linting issues). I'll get it merged unless anybody raises an issue. @nasaldrops are we good to close #3853? Thanks.

[jonnyry]

Thanks for merging @marrobi