-
Notifications
You must be signed in to change notification settings - Fork 20
ci: switch to AWS container registry #70
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
Conversation
Test jobs for commit c1ea40b |
| build-debos: | ||
| name: Build and upload debos recipes | ||
| outputs: | ||
| url: ${{ steps.upload_artifacts.outputs.url }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Github doesn't allow commenting on the commit message but that's the problem with this PR. You need a couple things:
- subject line
- body
- signed-off-by
So something more like:
ci: Switch to AWS container registry
The new CI workers are located in AWS and will benefit a little by using ECR.
Signed-off-by: .....
Test jobs for commit c1ea40b |
The new CI workers are located in AWS and will benefit a little by using ECR. Signed-off-by: Satish Mhaske <[email protected]>
c1ea40b to
893ded3
Compare
Test jobs for commit 893ded3 |
|
Hi, sorry just spotting this PR now; thanks Andy for your help in reviewing it! I have no objection on the technical implementation, but I'd like to capture the specific reasons we're doing this change in this conversation or in the commit message. You note in the PR message "As per AWS environment its recommended to use AWS Hosted docker hub instead of Public docker hub", and in the commit "The new CI workers are located in AWS and will benefit a little by using ECR". Both sound like soft requirements and perhaps the latter hints at performance/costs benefits? Could you clarigy if we are doing this for security / best practices reasons from Qualcomm IT, for AWS cost/performance efficiency reasons, or for some other reasons? I trust the registry will always mirror the latest debian:trixie images. Are all images available there and getting cached/audited somehow, or do I need to request new images I want to use? Is that the same process as for the Qualcomm internal container registry? (BTW, I was wondering if you've considered implementing a github action that would check whether Qualcomm repositories have workflows that use the new runners and are not using the Elastic Container Registry; we do this to scan commits to make sure they have a signed-off-by and that it's not using a wrong email address for instance.) |
|
@sampra2025 Not sure you saw my comment above? |
|
@sampra2025 and I connected out of band; he noted: "Yes I think we can close this PR. I think Andy has already implement such changes in some other PR, so its no more required. " (Cc: @doanac) |
|
I haven't implemented anything that would negate this PR. This PR is just a small performance optimization - The workflow is running in AWS, so pulling from an AWS container registry will be a little faster (and probably cheaper). |
|
Hi @sampra2025 and @doanac, thanks for your input, I've sent a new pull request #121 implementing this idea, using the rationale provided in your conversation, and updated to cover the other workflows that use docker images. Thanks for proposing this! |
As per AWS environment its recommended to use AWS Hosted docker hub instead of Public docker hub such as "docker.io", so updating the container registry.