-
Notifications
You must be signed in to change notification settings - Fork 69
feat(ws): Upgrade Go to 1.24 - tensorboard-controller component #755
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
base: notebooks-v1
Are you sure you want to change the base?
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/assign @mkoushni |
Signed-off-by: Marina Koushnir <[email protected]>
f510725 to
ab3a5d0
Compare
liavweiss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, Marina. In addition to the review comments, please include the go.sum file in the PR as well.
This file is important for validating module dependencies and ensuring consistent, reproducible builds across different environments.
|
|
||
| WORKDIR /workspace/tensorboard-controller | ||
| COPY tensorboard-controller/go.mod tensorboard-controller/go.sum ./ | ||
| RUN go mod download |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can copy go.mod and go.sum first, then run go mod download, and only after that copy the rest of the source
This preserves Docker layer caching: dependencies are downloaded only when the module files change, not every time source code changes. It significantly speeds up builds.
# Copy only Go module files first for better layer caching
COPY tensorboard-controller/go.mod tensorboard-controller/go.sum ./
# Download dependencies based on mod files
RUN go mod download
# Now copy the full source
COPY tensorboard-controller/ ./
| WORKDIR / | ||
| COPY --from=builder /workspace/tensorboard-controller/manager . | ||
| USER 65532:65532 | ||
| COPY --from=builder /manager /manager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the copy of the compiled binary should be before switching to the non-root user
This aligns with standard Docker conventions and avoids permission or ownership ambiguity in the final image.
| COPY --from=builder /manager /manager | |
| COPY --from=builder /manager /manager | |
| USER 65532:65532 |
Goal
This PR updates the required Go version for the tensorboard-controller component to Go 1.24.x. This change is necessary to maintain compatibility with updated Kubernetes client libraries and ensure the component is built with current, supported security patches.
Changes Made
Updated the go.mod file to specify go 1.24.
Synchronized dependencies via go mod tidy and go mod vendor to lock in compatible library versions (including necessary updates to transitive dependencies like golang.org/x/text, etc.).