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.

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

x/build/cmd/watchflakes: fails to run after a build output property field gitilesCommit → gitiles_commit rename #69609

Closed
dmitshur opened this issue Sep 24, 2024 · 7 comments
Closed

x/build/cmd/watchflakes: fails to run after a build output property field gitilesCommit → gitiles_commit rename #69609

dmitshur opened this issue Sep 24, 2024 · 7 comments
Assignees
@dr2chase
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@dmitshur
Copy link
Contributor

Watchflakes is currently in a continuous crashloop:

$ kubectl logs -p watchflakes-deployment-7dd687d899-8rf75
watchflakes: 2024/09/24 12:12:43 ListBuilders  
watchflakes: 2024/09/24 12:12:44 ReadBoard go master
watchflakes: 2024/09/24 12:12:44 ListCommits go master
watchflakes: 2024/09/24 12:12:44 ListBuilders go master
watchflakes: 2024/09/24 12:12:45 GetBuilds gotip-linux-amd64-ssacheck
watchflakes: 2024/09/24 12:12:45 GetBuilds gotip-linux-386-softfloat
[...]
watchflakes: 2024/09/24 12:12:45 GetBuilds gotip-linux-386-longtest
watchflakes: 2024/09/24 12:12:45 GetBuilds gotip-js-wasm
watchflakes: 2024/09/24 12:12:45 GetBuilds gotip-linux-amd64-longtest-race
watchflakes: 2024/09/24 12:12:46 repo mismatch:  go https://ci.chromium.org/b/8735981222897055585

It keeps running into the log.Fatalf at luci.go:366.

The problem is likely related to the fact that the output property "sources" now has a field named "gitiles_commit", whereas previously that field was named "gitilesCommit", and watchflakes looks only for the previous name at this time.

For reference:

crrev.com/c/5875121 might be what caused the field to have a new underscore-separated name.

CC @golang/release, @dr2chase, @cherrymui.
@dmitshur dmitshur added Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done. labels Sep 24, 2024
@dmitshur dmitshur added this to the Unreleased milestone Sep 24, 2024
@gabyhelp
Copy link

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/615515 mentions this issue: cmd/watchflakes: look for both gitilesCommit and gitiles_commit

@dr2chase
Copy link
Contributor

Not sure how to test it, but https://go.dev/cl/615515 might work.

@dmitshur dmitshur moved this to In Progress in Go Release Sep 24, 2024
@dmitshur
Copy link
Contributor Author

dmitshur commented Sep 24, 2024 •
edited
Loading

One way to test it would be to run watchflakes locally; it should no longer run into the "repo mismatch" error.
You'll also know it works when you deploy the fix and the production watchflakes instance resumes working.

Thanks for fixing this.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Release Sep 24, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/615976 mentions this issue: cmd/coordinator/internal/lucipoll: update for new JSONPB field naming

@dmitshur
Copy link
Contributor Author

The production instance isn't healthy yet:

$ kubectl get pods | grep watchflakes
watchflakes-deployment-7dd687d899-8rf75    0/1   CrashLoopBackOff   1876 (3m53s ago)   17d

@dr2chase Have you had a chance to deploy watchflakes with this fix included?

@dr2chase
Copy link
Contributor

I had not realized I needed to give it an explicit kick, let me try that.

gopherbot pushed a commit to golang/build that referenced this issue Sep 27, 2024
@dmitshur @gopherbot
cmd/coordinator/internal/lucipoll: update for new JSONPB field naming
a411a18 
As of crrev.com/c/5875121, golangbuild switched to the new default
behavior of using 'proto' field naming (such as "gitiles_commit")
instead of using protobuf default names (such as "gitilesCommit").

The previous behavior can still be achieved by explicitly using the
https://pkg.go.dev/go.chromium.org/luci/luciexe/build/properties#OptProtoUseJSONNames
option, but we choose to accept the new default behavior since it's
fine for our needs.

So, update a few places in lucipoll for the new field naming pattern
as was also recently done for watchflakes in CL 615515. Not handling
the old pattern here since lucipoll only needs to handle new builds.

Also drop the unused "omitempty" option from BuilderConfigProperties
while here - it's never used for marshaling, only unmarshaling.

For golang/go#69609.
For golang/go#65913.

Change-Id: Icf2f942fa9821f8781c9a2ecaddd2a3e10496ba2
Reviewed-on: https://go-review.googlesource.com/c/build/+/615976
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: David Chase <[email protected]>
@gabyhelp gabyhelp mentioned this issue Oct 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dr2chase dr2chase

Labels
Builders x/build issues (builders, bots, dashboards) 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
@dmitshur @dr2chase @gopherbot @gabyhelp