[apiserver][test] Trust E2E_API_SERVER_RAY_IMAGE verbatim (closes #4346)#4770
Open
1fanwang wants to merge 2 commits intoray-project:masterfrom
Open
[apiserver][test] Trust E2E_API_SERVER_RAY_IMAGE verbatim (closes #4346)#47701fanwang wants to merge 2 commits intoray-project:masterfrom
1fanwang wants to merge 2 commits intoray-project:masterfrom
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 918b54a. Configure here.
The apiserver e2e helpers `withRayImage` in `apiserver/test/e2e/types.go` and `apiserver/test/e2e/apiserversdk/types.go` second-guess the configured image: when `runtime.GOARCH == "arm64"` they auto-append `-aarch64`. This prevents Apple-silicon developers from running the suite against an x86_64-only remote cluster, even when they explicitly set `E2E_API_SERVER_RAY_IMAGE` to an x86_64 image. PR ray-project#4348 already removed the same pattern from `ray-operator/test/support/environment.go`. This change applies the same cleanup to the two remaining occurrences, completing ray-project#4346. `apiserver/Makefile:14-16` already provides the correct arm64-suffixed default for arm64 hosts via `ifeq (arm64, $(shell go env GOARCH))`, so `make test` behavior on arm64 is preserved. Signed-off-by: 1fanwang <1fannnw@gmail.com>
Without `export`, Make-defined defaults don't propagate to the `go test` subprocess in `e2e-test`, so the e2e helpers fall back to their hard-coded defaults. On arm64 hosts running `make local-e2e-test`, this caused `load-ray-test-image` to preload `rayproject/ray:2.46.0-py310-aarch64` into kind while the Go test would (post the previous commit) ask for `rayproject/ray:2.46.0-py310` — a mismatch caught by Cursor Bugbot in the PR review. Exporting both vars makes the Makefile the single source of truth for the target image and base URL, restoring the on-arm64 behavior that the in-Go GOARCH detection used to mask. Signed-off-by: 1fanwang <1fannnw@gmail.com>
1fanwang
force-pushed
the
apiserver-test-trust-image-env-4346
branch
2 times, most recently
from
54e6d55 to
b0af3b1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
withRayImageinapiserver/test/e2e/types.goandapiserver/test/e2e/apiserversdk/types.gosecond-guesses the configured Ray image: whenruntime.GOARCH == "arm64", it auto-appends-aarch64to whateverE2E_API_SERVER_RAY_IMAGEis set to. This prevents Apple-silicon developers from running the apiserver e2e suite against an x86_64-only remote cluster — the use case reported in #4346.#4348 already removed the same pattern from
ray-operator/test/support/environment.go. This PR applies the same cleanup to the two remaining occurrences in the apiserver tests, completing #4346.apiserver/Makefile:14-16already provides the correct arm64-suffixed default for arm64 hosts viaifeq (arm64, $(shell go env GOARCH)), somake testbehavior on arm64 is preserved without any in-Go-code mangling.Related issue number
Closes #4346.
Follow-up to #4348.
Checks
go build ./...andgo vet ./test/...pass on darwin/arm64. The change is a pure deletion mirroring the merged Support Multi-Arch Image in CI #4348; CI will exercise the Makefile path on both arm64 and amd64 runners.