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

[3.4] Fix all the pipeline failues for release 3.4 #14136

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

ahrtr
Copy link
Member

@ahrtr ahrtr commented Jun 20, 2022

Fix issues/14135

Items resolved:

  1. fix the vet error: possible misuse of reflect.SliceHeader;
  2. fix the vet error: call to (*T).Fatal from a non-test goroutine;
  3. bump package golang.org/x/crypto, net and sys;
  4. bump boltdb from 1.3.3 to 1.3.6;
  5. remove the vendor directory;
  6. remove go 1.12.17 and 1.15.15, add go 1.16.15 into pipeline;
  7. bump go version to 1.16 in go.mod;
  8. fix the issue: compile: version go1.16.15 does not match go tool version go1.17.11;
  9. fix data race on compactMainRev and watcherGauge;
  10. fix test failure for TestLeasingTxnOwnerGet in cluster_proxy mode.

Signed-off-by: Benjamin Wang wachao@vmware.com

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

@ahrtr
Copy link
Member Author

ahrtr commented Jun 20, 2022

Copy link
Contributor

@ptabor ptabor left a comment

Choose a reason for hiding this comment

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

Thank you. LGTM.

.github/workflows/tests.yaml Show resolved Hide resolved
echo "${TARGET}"
case "${TARGET}" in
linux-amd64-fmt)
GOARCH=amd64 PASSES='fmt bom dep' ./test
;;
linux-amd64-integration-1-cpu)
GOARCH=amd64 CPU=1 PASSES='integration' RACE="${RACE}" ./test
GOARCH=amd64 CPU=1 PASSES='integration' RACE='false' ./test
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intended to turn RACE of permanently ?
I see you fix some of the races in the other places in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just keep it as it's. Previously when go version is 1.15.15, then RACE is set to false, see tests.yaml#L47

;;
linux-amd64-unit)
GOARCH=amd64 PASSES='unit' RACE="${RACE}" ./test
GOARCH=amd64 PASSES='unit' RACE='false' ./test
Copy link
Member

Choose a reason for hiding this comment

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

I think we should have at least one test running with RACE='true'

Copy link
Member Author

@ahrtr ahrtr Jun 21, 2022

Choose a reason for hiding this comment

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

Yes, I agree. But let's add any new pipeline/test in a separate PR. I just bumped the go version to 1.16.15 in this PR, and keep all others unchanged.

Previously when go version is 1.15.15, then RACE is set to false, see tests.yaml#L47. So I just keep it as it's.

@ahrtr ahrtr force-pushed the 3.4_pipeline_issues branch 2 times, most recently from 94d19a2 to 93bb977 Compare June 21, 2022 20:50
@@ -1,6 +1,6 @@
module go.etcd.io/etcd

go 1.12
go 1.16
Copy link

Choose a reason for hiding this comment

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

I think the idea was to keep this at 1.12 #12840 (comment) CC @ptabor

Copy link
Member Author

Choose a reason for hiding this comment

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

No need to support go 1.12. It's out of support, and is subject to vulnerability. IIRC It is also not compatible with some system packages in newer version.

Note that it's more than 1 year before when ptabor added that comment, and go 1.12 wasn't that old although it's also out of support at that time.

Copy link

Choose a reason for hiding this comment

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

I'm not 100% sure but I think go 1.16 keeps go compatibility promise, so its not subject to vulnerabilities.
I also run into package issues but go mod tidy fixed it ff89836

Either way is fine with me, but both will work IMO.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that the promise of compatibility is for golang language of version < 2.0. There is no guarantee on the core library and tool-chain.

etcdserver/server_test.go Outdated Show resolved Hide resolved
@lavacat
Copy link

lavacat commented Jun 21, 2022

I've made some of the similar fixes here as a stand-alone commits with references to corresponding 3.5 fixes. I think it's easier to track fixes across branches. You can cherry-pick my commits.

You also want this flaky test fix 4aeadec

Items resolved:
1. fix the vet error: possible misuse of reflect.SliceHeader;
2. fix the vet error: call to (*T).Fatal from a non-test goroutine;
3. bump package golang.org/x/crypto, net and sys;
4. bump boltdb from 1.3.3 to 1.3.6;
5. remove the vendor directory;
6. remove go 1.12.17 and 1.15.15, add go 1.16.15 into pipeline;
7. bump go version to 1.16 in go.mod;
8. fix the issue: compile: version go1.16.15 does not match go tool version go1.17.11,
   refer to actions/setup-go#107;
9. fix data race on compactMainRev and watcherGauge;
10. fix test failure for TestLeasingTxnOwnerGet in cluster_proxy mode.

Signed-off-by: Benjamin Wang <wachao@vmware.com>
@ahrtr
Copy link
Member Author

ahrtr commented Jun 21, 2022

I've made some of the similar fixes here as a stand-alone commits with references to corresponding 3.5 fixes. I think it's easier to track fixes across branches. You can cherry-pick my commits.

You also want this flaky test fix 4aeadec

Let's keep the PR as it's for now. It's just a starting point for the 3.4 pipeline and all test. Just as I mentioned in issues/14135, 3.4 pipeline has never green since the first day (on Jun 24, 2021) created.

We can continue to work on other issues & enhancement in separate PRs.

But the top priority is still to work on the pipeline & test failures/flaky. Let's try to add more pipelines (such as RACE=true) in a followup PR, and make sure everything is green.

@ahrtr ahrtr merged commit 953376e into etcd-io:release-3.4 Jun 22, 2022
This was referenced Jun 23, 2022
@ahrtr
Copy link
Member Author

ahrtr commented Jun 23, 2022

Plan for 3.4.19: 14105

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

Successfully merging this pull request may close these issues.

4 participants