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

x/build/cmd/coordinator: include -longtest and -race builders for trybots on release branches #37827

Open
dmitshur opened this issue Mar 12, 2020 · 6 comments
Assignees
Milestone

Comments

@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Mar 12, 2020

After a major version of a Go release is out, we send relatively few CLs to the release branch in order to backport fixes for minor releases, per https://golang.org/wiki/MinorReleases policy.

We should consider including -longtest and -race builders in the default tryset (set of builders included in a normal trybot run) for those branches::

  • linux-386-longtest
  • linux-amd64-longtest
  • windows-amd64-longtest
  • darwin-amd64-race
  • freebsd-amd64-race
  • linux-amd64-race
  • windows-amd64-race

Those builders already run as post-submit builders, but also running them during trybots will give us more information to work with before a CL is submitted to a release branch.

We may want to do this only for minor releases, i.e., after a major Go release is out. Or we can do this always, which would include CLs sent to a release branch after the first release candidate is made.

An existing alternative is to manually request those builders via SlowBots.

/cc @cagedmantis @toothrot @golang/osp-team

@gopherbot
Copy link

@gopherbot gopherbot commented May 19, 2020

Change https://golang.org/cl/234531 mentions this issue: cmd/releasebot, cmd/release: include long tests in release process

@gopherbot
Copy link

@gopherbot gopherbot commented May 26, 2020

Change https://golang.org/cl/235337 mentions this issue: dashboard: update test data to Go 1.14

@gopherbot
Copy link

@gopherbot gopherbot commented May 26, 2020

Change https://golang.org/cl/235338 mentions this issue: dashboard: include longtest builders for trybots on release branches

@dmitshur dmitshur self-assigned this May 26, 2020
@dmitshur
Copy link
Contributor Author

@dmitshur dmitshur commented May 26, 2020

There's agreement from release team, and this is needed for #29252.

I'll start with adding just the longtest builders first. Sent CL 235338 for that.

gopherbot pushed a commit to golang/build that referenced this issue May 27, 2020
This test data will be modified to match a change in behavior
in the next commit. Update it to a supported Go version first.

For golang/go#37827.

Change-Id: I83ad967a11f0fdde454edc541e9a8e7cf061f0e0
Reviewed-on: https://go-review.googlesource.com/c/build/+/235337
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
gopherbot pushed a commit to golang/build that referenced this issue Jun 2, 2020
The goal of this test policy change is to decrease the chance of a
Go repository release branch from getting into a state where some
longtest post-submit builders are failing, as this will interfere
with the ability to issue releases after golang/go#29252 is fixed.

There are not that many CLs sent to release branches, so the extra
latency caused by slower longtest builders being a part of trybots
should not be very disruptive.

For golang/go#37827.
For golang/go#29252.

Change-Id: I819b120bb7b763df4acf4583375cdb840793d323
Reviewed-on: https://go-review.googlesource.com/c/build/+/235338
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
@dmitshur
Copy link
Contributor Author

@dmitshur dmitshur commented Jun 2, 2020

CL 235338 is deployed now.

I tried it in an empty CL on release-branch.go1.13 (see here) and the amd64 longtest builders are working as expected, but linux-386-longtest is failing in a strange way:

linux-386-longtest at de020178a36d5ba89b490c998f7f41a9682f31b4

HASH[build runtime/internal/sys]
HASH[build runtime/internal/sys]: "go1.13.10"
HASH[build runtime/internal/sys]: "compile\n"
HASH[build runtime/internal/sys]: "goos linux goarch 386\n"
HASH[build runtime/internal/sys]: "import \"runtime/internal/sys\"\n"
HASH[build runtime/internal/sys]: "omitdebug false standard true local false prefix \"\"\n"
HASH[build runtime/internal/sys]: "modinfo \"\"\n"
HASH[build runtime/internal/sys]: "compile compile version go1.13.10 [] []\n"
HASH[build runtime/internal/sys]: "asm \"asm version go1.13.10\" [] []\n"
HASH[build runtime/internal/sys]: "GO386=sse2\n"
HASH /workdir/go/src/runtime/internal/sys/arch.go: d9b0b7e72538d421b2607acaba60ca49f20ef584b3d1d191c6729e35fbb8101d
HASH[build runtime/internal/sys]: "file arch.go 2bC35yU41CGyYHrKumDK\n"
HASH /workdir/go/src/runtime/internal/sys/arch_386.go: bf78e84e2c3328ece06b3f3d589a945bef4f8eaaa49f53fc3597c1cfb44624e8
HASH[build runtime/internal/sys]: "file arch_386.go v3joTiwzKOzgaz89WJqU\n"
HASH /workdir/go/src/runtime/internal/sys/intrinsics_stubs.go: 071919e7a015c55e28d889b9cb5c84d42a084a7919d5c717914d25c0782dbf75
HASH[build runtime/internal/sys]: "file intrinsics_stubs.go BxkZ56AVxV4o2Im5y1yE\n"
HASH /workdir/go/src/runtime/internal/sys/stubs.go: 23b3e5c631b086fe7a2dec4bf044600e034bf6a8eeb25e0a19efc4ce6311423d
HASH[build runtime/internal/sys]: "file stubs.go I7PlxjGwhv56LexL8ERg\n"
HASH /workdir/go/src/runtime/internal/sys/sys.go: 55e021891200a7e6a5c371c8a1ab71b6c15aeb16ea6c1b192185d17df8c8b18f
HASH[build runtime/internal/sys]: "file sys.go VeAhiRIAp-alw3HIoatx\n"
HASH /workdir/go/src/runtime/internal/sys/zgoarch_386.go: 1b220caae2cffb996d161554ada914bccd4545502e267625962a6294d5203568
HASH[build runtime/internal/sys]: "file zgoarch_386.go GyIMquLP-5ltFhVUrakU\n"
HASH /workdir/go/src/runtime/internal/sys/zgoos_linux.go: 806c088d7491b4560a28a5af86a52b459ebbf155ea455af873baa0bf697355e4
HASH[build runtime/internal/sys]: "file zgoos_linux.go gGwIjXSRtFYKKKWvhqUr\n"
HASH /workdir/go/src/runtime/internal/sys/zversion.go: 135087d0f7a37b934f92dcd1d33cadac6a4560b6de44b2050eb26ed3f676d1f7
HASH[build runtime/internal/sys]: "file zversion.go E1CH0Peje5NPktzR0zyt\n"
HASH /workdir/go/src/runtime/internal/sys/intrinsics_386.s: fd84d19707a42418ed46281b43a12f1669e6d8bdb6272f91efc76698bee8f582
HASH[build runtime/internal/sys]: "file intrinsics_386.s _YTRlwekJBjtRigbQ6Ev\n"
HASH[build runtime/internal/sys]: d61728ef2b8a15eb0b8ba83904c2b2c7bca5fc101bfc2dd64158894353b02864
runtime/internal/sys true
go tool dist: unexpected stale targets reported by go list -gcflags="" -ldflags="" for [std]:
	STALE archive/tar: stale dependency: internal/cpu
	STALE archive/zip: stale dependency: internal/cpu
	STALE bufio: stale dependency: internal/cpu
	STALE bytes: stale dependency: internal/cpu
	STALE compress/bzip2: stale dependency: internal/cpu
...

This is strange and needs investigation. We've seen this problem happen in other builders, e.g., #33598. /cc @bcmills

@dmitshur
Copy link
Contributor Author

@dmitshur dmitshur commented Jun 2, 2020

Making the tree slightly different has allowed the linux-386-longtest builder to start properly.

gopherbot pushed a commit to golang/build that referenced this issue Jun 2, 2020
The goal of this change is to reduce the chance of issuing a release
with an unintended regression that would be caught by running long
tests. This change adds long tests that are run during the -prepare
step, in addition to all the existing short tests that are run.

Executing the long tests is implemented by adding two new test-only
release targets. For a release to be considered complete, all release
targets must complete.

These test-only targets are built only for the purpose of verifying
their tests succeed, or to block the release otherwise.
They do not produce release artifacts.

The new test-only targets are named after the builder which is used
to perform their tests, and they are:

• linux-amd64-longtest
• windows-amd64-longtest

More builders may be added in the future, but care must be taken
to ensure the test execution environment is as close as possible
to that of build.golang.org post-submit builders, in order to
avoid false positives and false negatives.

As part of a gradual rollout, this change also adds a flag to skip
longtest builders. It's meant to be used in case a long test proves
to be flaky, and enough confidence can be gained through testing
elsewhere that the failure is not a regression caused by a change
merged to the release branch.

For now, its default value includes both longtest builders, so they
are currently opt-in and this CL is a no-op. After testing proves
that it is viable to rely on this (and any issues preventing that
from being possible are resolved), the default value of the flag
will be changed to the empty string.

For golang/go#29252.
For golang/go#39054.
For golang/go#37827.
Fixes golang/go#24614.

Change-Id: I33ea6601aade2873f857c63f00a0c11821f35a95
Reviewed-on: https://go-review.googlesource.com/c/build/+/234531
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants