Skip to content

x/build/cmd/gomote: warn when gomote binary is too old #30929

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.

Sign up for GitHub

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

Jump to bottom
Closed
bradfitz opened this issue Mar 19, 2019 · 7 comments
Closed

x/build/cmd/gomote: warn when gomote binary is too old #30929

bradfitz opened this issue Mar 19, 2019 · 7 comments
Labels
Builders x/build issues (builders, bots, dashboards) FeatureRequest Issues asking for a new feature that does not need a proposal. FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@bradfitz
Copy link
Contributor

A common gomote complaint is that the set of builders has changed since the last time the user built the gomote binary.

We could pretty easily get it from the server instead.

/cc @ianlancetaylor
@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. FeatureRequest Issues asking for a new feature that does not need a proposal. labels Mar 19, 2019
@bradfitz bradfitz self-assigned this Mar 19, 2019
@gopherbot gopherbot added this to the Unreleased milestone Mar 19, 2019
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Mar 19, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/168341 mentions this issue: cmd/coordinator: add a JSON mode to the /builders handler

gopherbot pushed a commit to golang/build that referenced this issue Mar 21, 2019
@bradfitz
cmd/coordinator: add a JSON mode to the /builders handler
32d155d 
And add a test that indirectly verifies that the BuildConfig and
HostConfig are JSON serializable. They weren't due to an exported func.

But that exported func shouldn't be exported, so unexport it and move
more policy into dashboard/builders.go. (There's been a number of
recent cleanup CLs to move all policy into dashboard/builders.go
instead of sprinkled all over the coordinator)

A future CL will use this JSON in gomote create.

Updates golang/go#30929

Change-Id: I726eaf6a4f3eeaab27d31e2642cb7642111ccd67
Reviewed-on: https://go-review.googlesource.com/c/build/+/168341
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/169678 mentions this issue: cmd/gomote: get list of buildlet types from server

@bradfitz
Copy link
Contributor Author

I did create and thought that was enough, but other parts of gomote still use the hard-coded info.

Reopening.

@bradfitz bradfitz reopened this Mar 27, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/170397 mentions this issue: cmd/gomote: add -firewall flag to run, defaulting to off

gopherbot pushed a commit to golang/build that referenced this issue Apr 2, 2019
@bradfitz
cmd/gomote: add -firewall flag to run, defaulting to off
49a6a61 
While debugging golang/go#25386 I found that the firewall defaulting
to on made debugging modules hard. So make gomote default to
no outbound firewall and make it opt-in for people who
want to reproduce the builder more.

Also in this CL: start to use server's builder config, not local state
(follow up to CL 169678 which was incomplete). It's still incomplete
in this CL, but there's now a panic with more useful information to
users telling them to update their binary.

Updates golang/go#30929

Change-Id: I17bded71919af1e7a9181866a1349eb72da40051
Reviewed-on: https://go-review.googlesource.com/c/build/+/170397
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@bradfitz bradfitz changed the title x/build/cmd/gomote: get list of buildlets from the server x/build/cmd/gomote: get buildlet configs from the server Apr 12, 2019
@bradfitz
Copy link
Contributor Author

This stalled a bit because I realized https://farmer.golang.org/builders?mode=json wasn't sufficient, given how many unexported fields are in dashboard.BuildConfig and HostConfig.

Maybe instead we should just hash it all server-side and have the client check that its hash matches, otherwise warm loudly to the user that their gomote binary is old. We could even pull both sides' dates from their module info and tell them how old it is.

@bradfitz bradfitz changed the title x/build/cmd/gomote: get buildlet configs from the server x/build/cmd/gomote: warn when gomote binary is too old May 29, 2019
@bradfitz bradfitz removed their assignment May 29, 2019
@dmitshur
Copy link
Member

dmitshur commented Sep 16, 2019 •
edited
Loading

Another possible instance of this problem is in #33309 (comment) (see last paragraph of the comment).

However, just fetching https://farmer.golang.org/builders?mode=json wouldn't have been sufficient, because the problem was that gomote was old enough to be missing a flag and not setting an env var. We probably need something to tell if the binary itself is too old. Maybe module versions and the module mirror can come in handy.

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsFix The path to resolution is known, but the work has not been done. labels Sep 17, 2019
@cagedmantis
Copy link
Contributor

This was fixed as part of #47521.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 8, 2023
@golang golang locked and limited conversation to collaborators Aug 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) FeatureRequest Issues asking for a new feature that does not need a proposal. FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Archived in project
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@bradfitz @cagedmantis @dmitshur @gopherbot