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

Allow to pass docker login when in CI, but not in GitHub actions #6702

Merged

Conversation

adeepn
Copy link
Member

@adeepn adeepn commented Jun 6, 2024

Description

Allow to use Docker credentials for oras-cli push artifacts when CI=true is set, but not in GitHub action.

TODO: fix GITHUB_OUTPUT use if CI=true is set.

Jira reference number AR-2357

How Has This Been Tested?

  • correct push to external container registry with CI=true

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings

@adeepn adeepn requested a review from a team as a code owner June 6, 2024 13:09
@github-actions github-actions bot added the size/small PR with less then 50 lines label Jun 6, 2024
@ColorfulRhino
Copy link
Collaborator

ColorfulRhino commented Jun 6, 2024

Could you explain the reasoning behind this to make it easier to understand? Thanks!

@rpardini
Copy link
Member

rpardini commented Jun 6, 2024

I'll take a guess: I've conflated CI=true with "is under Github Actions".
CI=true I think was classically introduced by Jenkins if memory doesn't fail me

@adeepn
Copy link
Member Author

adeepn commented Jun 7, 2024

I'll take a guess: I've conflated CI=true with "is under Github Actions". CI=true I think was classically introduced by Jenkins if memory doesn't fail me

Jenkins, GitLab, hand-run if needed etc

@adeepn
Copy link
Member Author

adeepn commented Jun 7, 2024

Could you explain the reasoning behind this to make it easier to understand? Thanks!

When building, we can store artifacts in the registry. However, if the build occurs within a Docker container, we must authenticate before invoking oras-cli. The line at

DOCKER_ARGS+=("--mount" "type=bind,source=${docker_config_file_host},target=${docker_config_file_docker}")
transfers Docker authentication information into the build container.

It might be necessary to modify the condition if CI==true to if UPLOAD_TO_OCI_ONLY==yes to better reflect the intended functionality.

@igorpecovnik igorpecovnik added Work in progress Unfinished / work in progress Discussion Being discussed - Voice your opinions :) 08 Milestone: Third quarter release labels Jun 7, 2024
@adeepn adeepn force-pushed the AR-2357-allow-to-pass-docker-login branch from 839c5cf to 1f97fa9 Compare June 7, 2024 12:55
@ColorfulRhino
Copy link
Collaborator

I see, thanks for your explanation!

@igorpecovnik
Copy link
Member

Even this is more or less internal switch, lets add it to the documentation.

@igorpecovnik igorpecovnik added Needs Documentation New feature needs documentation entry and removed Work in progress Unfinished / work in progress labels Jun 9, 2024
@ColorfulRhino
Copy link
Collaborator

Even this is more or less internal switch, lets add it to the documentation.

Good idea.

I think work in progress still applies? Since:

TODO: fix GITHUB_OUTPUT use if CI=true is set.

@igorpecovnik igorpecovnik added the Work in progress Unfinished / work in progress label Jun 9, 2024
@adeepn adeepn force-pushed the AR-2357-allow-to-pass-docker-login branch from 1f97fa9 to 5e13cb1 Compare June 24, 2024 08:35
@github-actions github-actions bot added the Framework Framework components label Jun 24, 2024
…ment

Signed-off-by: Viacheslav Bocharov <adeep@lexina.in>
@adeepn adeepn force-pushed the AR-2357-allow-to-pass-docker-login branch from 5e13cb1 to f6542cc Compare June 24, 2024 08:39
@adeepn
Copy link
Member Author

adeepn commented Jun 24, 2024

Seems found more lines to fix...

@adeepn adeepn marked this pull request as draft June 24, 2024 08:41
@adeepn adeepn marked this pull request as ready for review June 24, 2024 08:49
Signed-off-by: Viacheslav Bocharov <adeep@lexina.in>
Signed-off-by: Viacheslav Bocharov <adeep@lexina.in>
@adeepn adeepn force-pushed the AR-2357-allow-to-pass-docker-login branch from 7bd5663 to 3f412e5 Compare June 24, 2024 08:49
@adeepn adeepn added Needs review Seeking for review and removed Work in progress Unfinished / work in progress labels Jun 25, 2024
@igorpecovnik
Copy link
Member

Few lines to describe what this switch do https://github.com/armbian/documentation/blob/master/docs/Developer-Guide_Build-Options.md
and I have no more objections. I assume it doesn't break default GH functionality?

@adeepn
Copy link
Member Author

adeepn commented Jul 4, 2024

Few lines to describe what this switch do https://github.com/armbian/documentation/blob/master/docs/Developer-Guide_Build-Options.md and I have no more objections.

armbian/documentation#440

I assume it doesn't break default GH functionality?

No. Should not affect to github actions

@igorpecovnik igorpecovnik added Ready to merge Reviewed, tested and ready for merge and removed Needs review Seeking for review Needs Documentation New feature needs documentation entry labels Jul 4, 2024
@igorpecovnik igorpecovnik merged commit c27c55b into armbian:main Jul 4, 2024
8 checks passed
@adeepn adeepn deleted the AR-2357-allow-to-pass-docker-login branch July 5, 2024 11:28
@ColorfulRhino
Copy link
Collaborator

armbian/documentation#440

Well done 👍

@ColorfulRhino ColorfulRhino added the Documentation finished New feature was properly added to docs label Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
08 Milestone: Third quarter release Discussion Being discussed - Voice your opinions :) Documentation finished New feature was properly added to docs Framework Framework components Ready to merge Reviewed, tested and ready for merge size/small PR with less then 50 lines
Development

Successfully merging this pull request may close these issues.

4 participants