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

Updates to all porter bundles to build docker images for amd64 platform from arm64 machines locally #3827

Merged
merged 11 commits into from
Jan 23, 2024

Conversation

m1p1h
Copy link
Contributor

@m1p1h m1p1h commented Jan 16, 2024

This update fixes #3823

What is being addressed

Porter bundles built on arm64 machines locally currently generate docker images for arm64 platforms (not amd64) which result in bundle deployment errors.

How is this addressed

  • Specifies amd64 platform in porter docker template files
  • Identifies underlying architecture of dev machine running docker and specifies amd64 platform when building runtime images of a bundle

@github-actions github-actions bot added the external PR from an external contributor label Jan 16, 2024
@m1p1h
Copy link
Contributor Author

m1p1h commented Jan 16, 2024

@m1p1h the command you issued was incorrect. Please try again.

Examples are:

@microsoft-github-policy-service agree

and

@microsoft-github-policy-service agree company="your company"

@microsoft-github-policy-service agree company="nhs"

Copy link
Member

@marrobi marrobi left a comment

Choose a reason for hiding this comment

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

One question, couple of find replace errors, thanks for looking into this.

devops/scripts/bundle_runtime_image_build.sh Show resolved Hide resolved
templates/workspaces/airlock-import-review/Dockerfile.tmpl Outdated Show resolved Hide resolved
templates/workspaces/unrestricted/Dockerfile.tmpl Outdated Show resolved Hide resolved
@marrobi
Copy link
Member

marrobi commented Jan 16, 2024

/test-extended

Copy link

github-actions bot commented Jan 16, 2024

Unit Test Results

588 tests   588 ✅  7s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit bbd19e2.

♻️ This comment has been updated with latest results.

Copy link

🤖 pr-bot 🤖

⚠️ When using /test-extended on external PRs, the SHA of the checked commit must be specified

(in response to this comment from @marrobi)

@m1p1h
Copy link
Contributor Author

m1p1h commented Jan 17, 2024

@marrobi - I don't see a way of addressing the lint warning where porter build encapsulates the docker build based on the suggested fix.

Setting the --platform value as variable also doesn't stop it complaining. The only way around it is to include a hadolint.yaml with the following:

ignored:
  - DL3029

@marrobi
Copy link
Member

marrobi commented Jan 17, 2024

@m1p1h happy for you to add the ignore to .github/linters/.hadolint.yaml

@m1p1h
Copy link
Contributor Author

m1p1h commented Jan 19, 2024

@marrobi could we rerun the tests

@m1p1h
Copy link
Contributor Author

m1p1h commented Jan 19, 2024

sorry - was some trailing white space..should be ok now

Copy link
Member

@marrobi marrobi left a comment

Choose a reason for hiding this comment

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

LGTM

@marrobi
Copy link
Member

marrobi commented Jan 19, 2024

/test-extended 7185718

@marrobi marrobi enabled auto-merge (squash) January 19, 2024 14:02
Copy link

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/7584785429 (with refid a27aa4c2)

(in response to this comment from @marrobi)

@marrobi marrobi disabled auto-merge January 19, 2024 14:44
@marrobi
Copy link
Member

marrobi commented Jan 19, 2024

@m1p1h actually, missed, can you add an update to the changelog, just a simple one liner with reference the the PR.

An unrelated dependency issue seems to have raised it's head. I'll try fix that while you update the changelog. Thanks.

@marrobi
Copy link
Member

marrobi commented Jan 19, 2024

/test-extended 14dec54

Copy link

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/7585457878 (with refid a27aa4c2)

(in response to this comment from @marrobi)

@m1p1h
Copy link
Contributor Author

m1p1h commented Jan 19, 2024

@m1p1h actually, missed, can you add an update to the changelog, just a simple one liner with reference the the PR.

An unrelated dependency issue seems to have raised it's head. I'll try fix that while you update the changelog. Thanks.

changelog updated

@marrobi
Copy link
Member

marrobi commented Jan 19, 2024

/test-extended 1f3d4b2

Copy link

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/7586142819 (with refid a27aa4c2)

(in response to this comment from @marrobi)

@marrobi
Copy link
Member

marrobi commented Jan 19, 2024

/test-extended 2d4817d

Copy link

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/7587163633 (with refid a27aa4c2)

(in response to this comment from @marrobi)

1 similar comment
Copy link

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/7587163633 (with refid a27aa4c2)

(in response to this comment from @marrobi)

@marrobi
Copy link
Member

marrobi commented Jan 22, 2024

/test-destroy-env

Copy link

Destroying PR test environment (RG: rg-trea27aa4c2)... (run: https://github.com/microsoft/AzureTRE/actions/runs/7613378501)

Copy link

PR test environment destroy complete (RG: rg-trea27aa4c2)

@marrobi
Copy link
Member

marrobi commented Jan 22, 2024

/test-extended

Copy link

🤖 pr-bot 🤖

⚠️ When using /test-extended on external PRs, the SHA of the checked commit must be specified

(in response to this comment from @marrobi)

@marrobi
Copy link
Member

marrobi commented Jan 22, 2024

/test-extended 3ffb74dee

Copy link

🤖 pr-bot 🤖

⚠️ The specified SHA 3ffb74dee is not the latest commit on the PR. Please validate the latest commit and re-run /test

(in response to this comment from @marrobi)

@marrobi
Copy link
Member

marrobi commented Jan 22, 2024

/test-extended 2d4817d

Copy link

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/7613837987 (with refid a27aa4c2)

(in response to this comment from @marrobi)

@marrobi marrobi enabled auto-merge (squash) January 22, 2024 21:58
@marrobi
Copy link
Member

marrobi commented Jan 23, 2024

/test-extended

Copy link

🤖 pr-bot 🤖

⚠️ When using /test-extended on external PRs, the SHA of the checked commit must be specified

(in response to this comment from @marrobi)

@marrobi
Copy link
Member

marrobi commented Jan 23, 2024

/test-extended bbd19e2

Copy link

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/7624044675 (with refid a27aa4c2)

(in response to this comment from @marrobi)

@marrobi marrobi merged commit 3196089 into microsoft:main Jan 23, 2024
10 checks passed
@marrobi
Copy link
Member

marrobi commented Jan 23, 2024

@m1p1h thanks for the contribution.

Took a bit of getting merged, weird dependency issue appeared, then our scheduled clean up script deleted the environment I was trying to redeploy to (I didn't realise this was the case), then a duplicate storage account ID which is rare but something we know about!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external PR from an external contributor
Projects
None yet
3 participants