[ci] Fix crane auth issue for nightly multi arch tagging#53483
[ci] Fix crane auth issue for nightly multi arch tagging#53483
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR ensures multi-architecture nightly images are tagged and indexed correctly by:
- Prefixing all image tags with the
rayproject/rayrepository. - Adding Docker Hub authorization via AWS SSM for
raytravisbot. - Mocking
authorize_dockerin the unit test to avoid real login.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ci/ray_ci/automation/generate_index.py | Prefix tags with rayproject/ray: before indexing. |
| ci/ray_ci/automation/docker_tags_lib.py | Add authorize_docker (SSM fetch + docker login). |
| ci/ray_ci/automation/test_docker_tags_lib.py | Mock authorize_docker in test_generate_index. |
| .buildkite/build.rayci.yml | Remove skip-on-premerge tag from nightly step. |
Comments suppressed due to low confidence (3)
ci/ray_ci/automation/docker_tags_lib.py:662
- Add a docstring explaining what
_get_ssmdoes, its parameters, and return value for clarity.
def _get_ssm(token_name):
ci/ray_ci/automation/test_docker_tags_lib.py:693
- After mocking
authorize_docker, add an assertion such asmock_authorize_docker.assert_called_once()intest_generate_indexto verify the login step is invoked.
@mock.patch("ci.ray_ci.automation.docker_tags_lib.authorize_docker")
ci/ray_ci/automation/generate_index.py:18
- Prefixing tags before your suffix-based matching logic may break the
-aarch64checks (they currently expect raw tags). Consider splitting on:or applying therayproject/ray:prefix only when pushing, not during index selection.
tags = [f"rayproject/ray:{tag}" for tag in tags]
|
|
||
|
|
||
| def generate_index(index_name: str, tags: List[str]) -> bool: | ||
| print(f"Generating index {index_name} with tags {tags}") |
There was a problem hiding this comment.
[nitpick] Replace print with the shared logger (e.g. logger.info) to keep logging consistent across this module.
| print(f"Generating index {index_name} with tags {tags}") | |
| logger.info(f"Generating index {index_name} with tags {tags}") |
|
|
||
|
|
||
| def _get_ssm(token_name): | ||
| client = boto3.client("ssm", region_name="us-west-2") |
There was a problem hiding this comment.
[nitpick] Avoid hard-coding the AWS region. Consider reading region_name from configuration or an environment variable to improve flexibility.
| client = boto3.client("ssm", region_name="us-west-2") | |
| aws_region = os.environ.get("AWS_REGION", "us-west-2") | |
| client = boto3.client("ssm", region_name=aws_region) |
|
indexes got pushed to dockerhub here: https://hub.docker.com/r/rayproject/ray/tags?name=nightly |
| import requests | ||
| import runfiles | ||
|
|
||
| import boto3 |
There was a problem hiding this comment.
nit: could you add back a blank line? and maybe also sort the 3rd-party imports?
|
|
||
| bazel_workspace_dir = os.environ.get("BUILD_WORKSPACE_DIRECTORY", "") | ||
| SHA_LENGTH = 6 | ||
| DOCKERHUB_SSM_NAME = "docker_hub_password" |
| tags: | ||
| - docker | ||
| - oss | ||
| - skip-on-premerge |
There was a problem hiding this comment.
this needs to be restored.
|
|
||
|
|
||
| def test_generate_index(): | ||
| @mock.patch("ci.ray_ci.automation.docker_tags_lib.authorize_docker") |
| tags = list_image_tags( | ||
| prefix, RayType.RAY, PYTHON_VERSIONS_RAY, PLATFORMS_RAY, ARCHITECTURES_RAY | ||
| ) | ||
| tags = [f"rayproject/ray:{tag}" for tag in tags] |
There was a problem hiding this comment.
move these down, and call them refs or images ?
There was a problem hiding this comment.
wdym by moving these down?
There was a problem hiding this comment.
hmm.. I do not know what I was talking about..
It think I meant that you would want to add it after tags are being paired.
I also mentioned that to stop using not in and in for suffix checking.
There was a problem hiding this comment.
I updated the logic by using endswith instead
There was a problem hiding this comment.
and I don't think adding rayproject/ray before or after tags are paired should change anything?
|
Test pass https://buildkite.com/ray-project/postmerge/builds/10708#0197560c-8819-429f-832a-213028dba23e |
- Add prefix `rayproject/ray` for all tags - Authorize docker with credentials from SSM - Mock authorize docker in unit test since it's not needed --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
- Add prefix `rayproject/ray` for all tags - Authorize docker with credentials from SSM - Mock authorize docker in unit test since it's not needed --------- Signed-off-by: kevin <kevin@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
rayproject/rayfor all tags