Skip to content

Conversation

@tamird
Copy link
Contributor

@tamird tamird commented Jan 7, 2026

  • packetdrill: use docker version
    This is the standard connectivity check used elsewhere.
  • tools/build_cover.sh: rearrange
    This should make it easier to delete this code when the time comes.
  • Reduce Go version duplication
    Remove images/gpu/cuda-tests-12-8/install_go.sh; golang is already
    installed via apt and the version is immaterial.
  • cuda-tests: make Dockerfiles more similar
    These files are almost identical. Go the rest of the way. Now the only
    difference is the invocation of cmake.
  • Dockerfile: use golang builder images
    This avoids the need for apt-get install golang and other shenanigans.
    It also makes it clear that the version of Go used need not track the
    version specified in go.mod.

@tamird tamird mentioned this pull request Jan 7, 2026
@tamird tamird force-pushed the gpu-cleanup branch 2 times, most recently from bd64cfb to 3d21ff3 Compare January 7, 2026 20:41
This is the standard connectivity check used elsewhere.
tamird added 2 commits January 8, 2026 06:08
This should make it easier to delete this code when the time comes.
Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
installed via apt and the version is immaterial.
@avagin avagin requested a review from zkoopmans January 8, 2026 17:27
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- Use `go_sdk.from_file` instead of repeating the version encoded in
  `go.mod` in `MODULE.bazel`.
- Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup 248b336
PiperOrigin-RevId: 853776390
@zkoopmans
Copy link
Contributor

zkoopmans commented Jan 8, 2026

Yeah, the intent was just to install go to build the third party tool / the runner script (run_sample.go in this directory). This is just a test / benchmark workload...it has nothing to do with overall gVisor build / installs. I'll review this.

copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup 248b336
PiperOrigin-RevId: 853776390
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup 248b336
PiperOrigin-RevId: 853776390
@tamird
Copy link
Contributor Author

tamird commented Jan 8, 2026

@zkoopmans I pushed two more commits, please have a look. We now use multi-stage builds for these Go programs, which gives us version stability without requiring either apt-installed or script-installed Go binaries.

@ayushr2
Copy link
Collaborator

ayushr2 commented Jan 8, 2026

We now use multi-stage builds for these Go programs, which gives us version stability without requiring either apt-installed or script-installed Go binaries.

Thanks for your perseverance @tamird, I think this is quite neat.

Copy link
Contributor

@zkoopmans zkoopmans left a comment

Choose a reason for hiding this comment

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

LGTM

copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.
- **cuda-tests: make Dockerfiles more similar**
  These files are *almost* identical. Go the rest of the way. Now the only
  difference is the invocation of `cmake`.
- **Dockerfile: use golang builder images**
  This avoids the need for `apt-get install golang` and other shenanigans.
  It also makes it clear that the version of Go used need not track the
  version specified in `go.mod`.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup 6abf014
PiperOrigin-RevId: 853776390
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.
- **cuda-tests: make Dockerfiles more similar**
  These files are *almost* identical. Go the rest of the way. Now the only
  difference is the invocation of `cmake`.
- **Dockerfile: use golang builder images**
  This avoids the need for `apt-get install golang` and other shenanigans.
  It also makes it clear that the version of Go used need not track the
  version specified in `go.mod`.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup 6abf014
PiperOrigin-RevId: 853776390
These files are *almost* identical. Go the rest of the way. Now the only
difference is the invocation of `cmake`.
@tamird
Copy link
Contributor Author

tamird commented Jan 8, 2026

Oops, go build -o <path> <URL> is not a thing, updated to use GOBIN=<path> go install <URL> instead.

@tamird tamird force-pushed the gpu-cleanup branch 2 times, most recently from c4222a8 to 6da3c38 Compare January 8, 2026 22:38
@tamird
Copy link
Contributor Author

tamird commented Jan 8, 2026

Oops almost had it right, and now it is actually right.

copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.
- **cuda-tests: make Dockerfiles more similar**
  These files are *almost* identical. Go the rest of the way. Now the only
  difference is the invocation of `cmake`.
- **Dockerfile: use golang builder images**
  This avoids the need for `apt-get install golang` and other shenanigans.
  It also makes it clear that the version of Go used need not track the
  version specified in `go.mod`.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup 6abf014
PiperOrigin-RevId: 853776390
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.
- **cuda-tests: make Dockerfiles more similar**
  These files are *almost* identical. Go the rest of the way. Now the only
  difference is the invocation of `cmake`.
- **Dockerfile: use golang builder images**
  This avoids the need for `apt-get install golang` and other shenanigans.
  It also makes it clear that the version of Go used need not track the
  version specified in `go.mod`.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup 6da3c38
PiperOrigin-RevId: 853776390
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.
- **cuda-tests: make Dockerfiles more similar**
  These files are *almost* identical. Go the rest of the way. Now the only
  difference is the invocation of `cmake`.
- **Dockerfile: use golang builder images**
  This avoids the need for `apt-get install golang` and other shenanigans.
  It also makes it clear that the version of Go used need not track the
  version specified in `go.mod`.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup 6da3c38
PiperOrigin-RevId: 853776390
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.
- **cuda-tests: make Dockerfiles more similar**
  These files are *almost* identical. Go the rest of the way. Now the only
  difference is the invocation of `cmake`.
- **Dockerfile: use golang builder images**
  This avoids the need for `apt-get install golang` and other shenanigans.
  It also makes it clear that the version of Go used need not track the
  version specified in `go.mod`.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup 6da3c38
PiperOrigin-RevId: 853776390
This avoids the need for `apt-get install golang` and other shenanigans.
It also makes it clear that the version of Go used need not track the
version specified in `go.mod`.
@tamird
Copy link
Contributor Author

tamird commented Jan 8, 2026

Another buglet, this time it was run_sample ending up in /usr/bin rather than /. Another sync plz!

copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.
- **cuda-tests: make Dockerfiles more similar**
  These files are *almost* identical. Go the rest of the way. Now the only
  difference is the invocation of `cmake`.
- **Dockerfile: use golang builder images**
  This avoids the need for `apt-get install golang` and other shenanigans.
  It also makes it clear that the version of Go used need not track the
  version specified in `go.mod`.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup 6da3c38
PiperOrigin-RevId: 853776390
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.
- **cuda-tests: make Dockerfiles more similar**
  These files are *almost* identical. Go the rest of the way. Now the only
  difference is the invocation of `cmake`.
- **Dockerfile: use golang builder images**
  This avoids the need for `apt-get install golang` and other shenanigans.
  It also makes it clear that the version of Go used need not track the
  version specified in `go.mod`.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup ba07231
PiperOrigin-RevId: 853776390
copybara-service bot pushed a commit that referenced this pull request Jan 9, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.
- **cuda-tests: make Dockerfiles more similar**
  These files are *almost* identical. Go the rest of the way. Now the only
  difference is the invocation of `cmake`.
- **Dockerfile: use golang builder images**
  This avoids the need for `apt-get install golang` and other shenanigans.
  It also makes it clear that the version of Go used need not track the
  version specified in `go.mod`.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup ba07231
PiperOrigin-RevId: 853776390
copybara-service bot pushed a commit that referenced this pull request Jan 9, 2026
- **packetdrill: use docker version**
  This is the standard connectivity check used elsewhere.
- **tools/build_cover.sh: rearrange**
  This should make it easier to delete this code when the time comes.
- **Reduce Go version duplication**
  Remove `images/gpu/cuda-tests-12-8/install_go.sh`; golang is already
  installed via apt and the version is immaterial.
- **cuda-tests: make Dockerfiles more similar**
  These files are *almost* identical. Go the rest of the way. Now the only
  difference is the invocation of `cmake`.
- **Dockerfile: use golang builder images**
  This avoids the need for `apt-get install golang` and other shenanigans.
  It also makes it clear that the version of Go used need not track the
  version specified in `go.mod`.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12458 from tamird:gpu-cleanup ba07231
PiperOrigin-RevId: 853776390
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants