Skip to content
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

fix: go in docker failing #1989

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

nrwiersma
Copy link
Contributor

@nrwiersma nrwiersma commented Oct 1, 2024

What is the problem I am trying to address?

In v0.15.2, an error occurs when trying to use the proxy:

INFO[5:50PM]: exit status 2: go: cannot find GOROOT directory: 'go' binary is trimmed and GOROOT is not set
	http-method=GET http-path=/github.com/hamba/avro/@v/list kind=Not Found module= operation=download.ListHandler ops=[download.ListHandler pool.List protocol.List vcsLister.List] request-id=ba64734c-a3b6-4cf2-a0ac-54199c55153a version=

It seems in Go 1.21, so defaults are no longer baked into the binary, causing issues when running Go.

My first attempt at a fix was to set GOROOT in the Dockerfile, but Athens does not pass all env vars to the Go binary when executing it. I did consider passing GOROOT by default to the Go binary, but this seems wrong for people not running the docker image. There are also multiple go call sites, each using their own env calling conventions.

The downside to this approach is the image grows from 170MB to 379MB.

How is the fix applied?

To get around the complexity, the root of the Dockerfile is set to golang:<ver>-alpine. This added the least complexity.

@nrwiersma nrwiersma self-assigned this Oct 1, 2024
@nrwiersma nrwiersma requested a review from a team as a code owner October 1, 2024 18:23
@@ -27,19 +27,17 @@ RUN DATE="$(date -u +%Y-%m-%d-%H:%M:%S-%Z)" && \
-ldflags "-X github.com/gomods/athens/pkg/build.version=$VERSION -X github.com/gomods/athens/pkg/build.buildDate=$DATE -s -w" \
-o /bin/athens-proxy ./cmd/proxy

FROM alpine:${ALPINE_VERSION}
FROM golang:${GOLANG_VERSION}-alpine
Copy link
Contributor

Choose a reason for hiding this comment

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

So, I think if we just set the values for GOROOT=/go, GOSUMDB=off, and GOPROXY=direct then it'll fix this issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

The only thing I'm not super sure of is if our users would find turning the sumdb off and proxy to direct a sane default. Maybe we can figure out what it used to default to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does not work, you must move the GOROOT through the code, otherwise you get the same error.

I will make the needed changes, so we have a full comparison, but my concerns are as follows:

  1. It is not obvious that GOROOT needs to move through the code, and will likely be missed down the line
  2. I doubt this is the last rodio we will have with moving the Go bin around like this, and breaks are a little untestable.

In my testing earlier today I found that setting GOPROXY, GOSUMDB, and GOROOT alleviated errors on a base alpine image. The base alpine image is much smaller than the Go one.
@matt0x6F
Copy link
Contributor

matt0x6F commented Oct 2, 2024

For thoroughness, I pulled your branch, built the image locally, and ran it:

❯ docker run -it athens-test /bin/sh                        
/ # go mod download -json github.com/golang/go@latest
{
        "Path": "github.com/golang/go",
        "Version": "v0.0.0-20241002005801-630d4fb600ae",
        "Query": "latest",
        "Info": "/root/go/pkg/mod/cache/download/github.com/golang/go/@v/v0.0.0-20241002005801-630d4fb600ae.info",
        "GoMod": "/root/go/pkg/mod/cache/download/github.com/golang/go/@v/v0.0.0-20241002005801-630d4fb600ae.mod",
        "Zip": "/root/go/pkg/mod/cache/download/github.com/golang/go/@v/v0.0.0-20241002005801-630d4fb600ae.zip",
        "Dir": "/root/go/pkg/mod/github.com/golang/go@v0.0.0-20241002005801-630d4fb600ae",
        "Sum": "h1:j5XR87AKVbqM/5KRzpMFSxhMzMfXeLGM4nvLQZCcp9Y=",
        "GoModSum": "h1:VnTjtYw+XLkxokOYpCb9NBW3cOTFO8+uqxF7o10XJQk="
}
/ # 

@matt0x6F
Copy link
Contributor

matt0x6F commented Oct 2, 2024

@nrwiersma
Copy link
Contributor Author

nrwiersma commented Oct 2, 2024

This version uses the original Alpine base. The env control was more centralised than I had originally thought, so it is less ugly than I had imagined.

For reference, /usr/local/go/go.env contains the defaults like GOPROXY and GOSUMDB. I think this is a good base for the install, and they can be overridden by Athens when needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants