feat: Upgrade Go to 1.24 - notebook-controller component. #721

[abdallahsamabd]

No description provided.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kimwnasptd for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[abdallahsamabd]

links
https://github.com/abdallahsamabd/notebooks/actions/runs/19413082564/job/55537283384
https://github.com/abdallahsamabd/notebooks/actions/runs/19413082562/job/55537283348
https://github.com/abdallahsamabd/notebooks/actions/runs/19413082561/job/55537283395
https://github.com/abdallahsamabd/notebooks/actions/runs/19413082555/job/55537283349

[abdallahsamabd]

[liavweiss]

Thank you, Abdalla
Validations and code are looking good
Please also change the version (Go) lines in the README.md file:

• https://github.com/abdallahsamabd/notebooks/blob/feat/721/components/notebook-controller/README.md#:~:text=need%20to%20use-,Go%201.17,-and%20up%20in
• https://github.com/abdallahsamabd/notebooks/blob/feat/721/components/notebook-controller/README.md#:~:text=go-,version%20v1.17%2B,-.

[abdallahsamabd]

/assign

[liavweiss]

close: #721
Thank you, Abdalla
/lgtm

[liavweiss]

Hey @abdallahsamabd , following my earlier review, I noticed one more thing that could be improved. It’s not mandatory, but it would make the code cleaner.

notebooks/components/notebook-controller/Dockerfile

Line 30 in 8c139d6

RUN CGO_ENABLED=0 GOOS=linux GO111MODULE=on go build -a -mod=mod -o manager main.go

I checked our environment and the module cache is writable, so -mod=mod doesn’t add anything in Go 1.20+.
GO111MODULE=on is also deprecated and has no effect since modules are always enabled now.

We can remove these flags, though it’s not required for the PR approval.

[google-oss-prow]

New changes are detected. LGTM label has been removed.

[abdallahsamabd]

done

[andyatmiami]

Hey @abdallahsamabd - thanks for the these changes! I think we are close - but need some help from you articulating why a few decisions were made in this PR.

In general - consider using the PR description to provide valuable context like this at the time you open the PR so I don't have to reach back out to ask 😎

i..e with a PR titled Upgrade to Go 1.24 seeing things like:

• controller-gen upgrade
• something? to do with hashicorp modules

should definitely perk your attention and realize you need to comment on why these changes are necessarily/included.

THANKS!

[andyatmiami]

sorry - i don't see any discussion highlighting why exactly we are doing this (?)

I'm not (yet) commenting on if this is a good or bad change - only that we absolutely should be calling it out in the commit message to awareness.

Please provide justification/motivation behind adding this code block to help me review better 🙇

[abdallahsamabd]

before adding this block, when running the code locally always failed on this line
COPY --from=builder /go/pkg/mod/github.com/hashicorp third_party/hashicorp

Go 1.24 behavior change: With Go 1.24, go mod download may not fully extract modules to /go/pkg/mod in a way that allows copying them. The explicit loop:
Finds all HashiCorp modules via go list -m all
Downloads each module explicitly with go mod download
Extracts them by running go list "$pkg/..." to ensure they're available in /go/pkg/mod
Without this step: The COPY command (COPY --from=builder /go/pkg/mod/github.com/hashicorp third_party/hashicorp) would fail because the modules might not be fully extracted or available at that path.
This ensures the HashiCorp source code is included in the final Docker image for license compliance, which is a common requirement for containerized applications using third-party dependencies.

[andyatmiami]

Did we need to increase controller-gen to version 0.14.0 as a result of the Go upgrade to 1.24?

If its required - it at minimum should be called out in the commit message. If its not required - it feels like scope creep (imho) - unless all other controllers in notebooks-v1 are also using this 0.14.0 version (at which point aligning for consistency I could appreciate would be nice to have!)

[abdallahsamabd]

without this changes, when I ran unit tests, it failed
https://github.com/abdallahsamabd/notebooks/actions/runs/19412389148/job/55535609268
as I understand, controller-gen v0.8.0 is not compatible with Go 1.24. The newer version (v0.14.0) supports Go 1.24

[andyatmiami]

/ok-to-test