Skip to content

Docker multi-arch images push for linux/amd64, linux/arm64 - #32686

Merged
salman2013 merged 16 commits into
masterfrom
salman/devstack-1069/multi-arch-docker-images
Jul 27, 2023
Merged

Docker multi-arch images push for linux/amd64, linux/arm64#32686
salman2013 merged 16 commits into
masterfrom
salman/devstack-1069/multi-arch-docker-images

Conversation

@salman2013

Copy link
Copy Markdown
Contributor

Description:

In this PR i updated github action for docker multi-arch (linux/amd64, linux/arm64) push

Ticket:
Arbi-BOM (view)

@salman2013
salman2013 requested review from UsamaSadiq and awais786 July 7, 2023 19:17
Comment thread Makefile
Comment thread Makefile
@salman2013
salman2013 requested a review from jmbowman July 11, 2023 09:24
@salman2013
salman2013 requested a review from UsamaSadiq July 13, 2023 11:49
Comment thread Makefile
check-types: ## run static type-checking tests
mypy

docker_build:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it possible to keep the separation between build and push steps? I can envision the need to iterate on the build process locally for development/debugging, without wanting to push the results up to Docker Hub.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

working on it.

Comment thread Makefile Outdated
Comment thread Makefile Outdated
@salman2013
salman2013 requested a review from jmbowman July 19, 2023 06:40

@jmbowman jmbowman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The last remaining concern is "will this slow down production deployments?" It doesn't look like GoCD is checking the status of the "Push Docker Images" workflow after merges to master, but we're not sure why and hence not super confident in that. Maybe you and the team can help figure out if this job is on the critical path for stage and production deployments or not? (It will be eventually when we switch to running edx-platform in Kubernetes, but we have some time to optimize before then.)

DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_PASSWORD }}
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
run : make docker_push
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Accidental trailing whitespace?

@UsamaSadiq UsamaSadiq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some minor suggestions otherwise we can go ahead and merge the PR. One hour Docker image building time won't affect our normal deployments pipeline for now so we can go ahead with the merge now and create a follow up issue for the optimisation.

Comment thread .github/workflows/docker-publish.yml
Comment thread .github/workflows/docker-publish.yml Outdated
@salman2013
salman2013 force-pushed the salman/devstack-1069/multi-arch-docker-images branch from d5cdbc7 to f1eb67c Compare July 27, 2023 09:08
@salman2013
salman2013 merged commit e4a1039 into master Jul 27, 2023
@salman2013
salman2013 deleted the salman/devstack-1069/multi-arch-docker-images branch July 27, 2023 09:45
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants