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 using tag prefixes in calculate-docker-image #6048

Open
atalman opened this issue Dec 12, 2024 · 1 comment
Open

Allow using tag prefixes in calculate-docker-image #6048

atalman opened this issue Dec 12, 2024 · 1 comment
Assignees

Comments

@atalman
Copy link
Contributor

atalman commented Dec 12, 2024

Issue with discrepancy between dockerhub image names for manylinux 2_28 images and ecr images:

Current naming:
308535385114.dkr.ecr.us-east-1.amazonaws.com/pytorch/manylinux2_28-builder-rocm6.2.4:804413e4d6f4e378061d47bd4ed99c1cbb462011

Proposed naming:
308535385114.dkr.ecr.us-east-1.amazonaws.com/pytorch/manylinux2_28-builder:rocm6.2.4-804413e4d6f4e378061d47bd4ed99c1cbb462011
similar to
pytorch/manylinux2_28-builder:rocm6.2.4-eed9bb3a0e85082d4d95ad4fa33673a158b8c488
This will rid us of the need to request new ECR entries to be created in the internal docker registry every time there's a ROCm (or even CUDA) version update, and afaict, there'll be no loss of information.
Also, it'll sync up the two repos' naming conventions perfectly.

@jithunnair-amd
Copy link
Collaborator

Thanks for considering my suggestion, @atalman.

Also related: it'd be really helpful to be able to test building binaries against the internal docker registry docker images created by a PR's changes, without needing to merge the PR. Today, this is not straightforward because the binary build jobs use dockerhub images e.g: https://github.com/pytorch/pytorch/blob/fda43c98d1bc82eeb493f6e79a1fd5f1636474a5/.github/workflows/generated-linux-binary-manywheel-nightly.yml#L4
But esp. if we do the above suggested naming alignment, it could be as simple as switching between internal registry and dockerhub based on some notion of is_pr_build?

In fact, the PR binary workflow runs should always use the internal ECR docker images instead of the dockerhub ones. That way, if a PR modifies the logic for building the manylinux docker images, the binaries will test those changes, otherwise it'll just pick up the already-existing docker image which should be practically identical to the one in dockerhub anyway, due to matching sha tag.

@ZainRizvi ZainRizvi moved this to Cold Storage in PyTorch OSS Dev Infra Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Cold Storage
Development

No branches or pull requests

2 participants