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

[release-3.4]: .travis.yml: Test with go v1.15.11 #12849

Merged
merged 12 commits into from
Apr 19, 2021

Conversation

lilic
Copy link
Contributor

@lilic lilic commented Apr 9, 2021

Currently, in CI the tests are only run with go v1.12, this adds also go v1.15.7.

@lilic
Copy link
Contributor Author

lilic commented Apr 9, 2021

I was not sure if there are any other places that would need updates, from what I can tell this is the only thing for CI testing as the Makefile is not used otherwise, please correct me if I am wrong, still new to the codebase. 😄🤞

.travis.yml Outdated
@@ -7,6 +7,7 @@ services: docker

go:
- 1.12.12
- 1.15.7
Copy link
Contributor

Choose a reason for hiding this comment

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

I would consider excluding a lot of variants
to save limited CI resources: for analogy please see:

exclude:

I think it's sufficient to run:

  • linux-amd64-fmt
  • linux-amd64-unit
  • linux-amd64-integration-4-cpu
  • linux-amd64-functional
  • linux-amd64-grpcproxy
  • all-build

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, makes sense will do!

This reminds me actually, about the actual version, I wanted to use the latest go 1.15.x which is v1.15.11 but it seems there is no tagged image version for this go version, how does that work?

@codecov-io
Copy link

codecov-io commented Apr 10, 2021

Codecov Report

❗ No coverage uploaded for pull request base (release-3.4@b7e5f5b). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 7f97dfd differs from pull request most recent head 2926a8d. Consider uploading reports for the commit 2926a8d to get more accurate results
Impacted file tree graph

@@              Coverage Diff               @@
##             release-3.4   #12849   +/-   ##
==============================================
  Coverage               ?   73.05%           
==============================================
  Files                  ?      427           
  Lines                  ?    33812           
  Branches               ?        0           
==============================================
  Hits                   ?    24703           
  Misses                 ?     7208           
  Partials               ?     1901           
Flag Coverage Δ
all 73.05% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7e5f5b...2926a8d. Read the comment docs.

@lilic lilic changed the title [release-3.4]: .travis.yml: Test with go v1.15.7 WIP: [release-3.4]: .travis.yml: Test with go v1.15.7 Apr 12, 2021
@lilic lilic force-pushed the test-go-1-15 branch 4 times, most recently from 4cdce57 to f9459a1 Compare April 14, 2021 11:35
@lilic lilic force-pushed the test-go-1-15 branch 3 times, most recently from 71d6a4e to 5d4badc Compare April 15, 2021 14:57
@lilic
Copy link
Contributor Author

lilic commented Apr 16, 2021

Thank you @ptabor for your review and approval! So tests were green for go 1.15 and go 1.12 in the:

Sadly now they failed twice in a row, I did notice a few flakes, around TestBalancerUnderNetworkPartitionWatchFollower , which I cannot reproduce locally ever. I don't have the ability to just retest the individual failures so I had to force push. 🤞 🟢

For the TLS cypher suites tests sadly the direct cherry-pick of 60e44286fa3c0c0 did not work due to go 1.12 not having the tls.CipherSuites() function. I solved this by requiring two files, which one is used depends on the build tags go versions, I am open to suggestions on a different approach but I rather played it safe here. The go 1.15 files are aligned to what we have in master and others were kept the same way.

I still have the open question around the latest go version:
I wanted to use the latest go 1.15.x which is v1.15.11 but it seems there is no tagged image version for this go version, how does that work? I can also bump in a follow up PR and we leave it as is for now.

@lilic lilic changed the title WIP: [release-3.4]: .travis.yml: Test with go v1.15.7 [release-3.4]: .travis.yml: Test with go v1.15.7 Apr 16, 2021
lilic and others added 7 commits April 19, 2021 11:18
Currently in CI the tests are only run with go v1.12, this adds also go
v1.15.11.

Excludes certain variants for v1.15.
This patch is needed due to go 1.15 erroring on:

"Setctty set but Ctty not valid in child".
In go1.13, the TLS13 is enablled by default, and as per go1.13 release notes :
TLS 1.3 cipher suites are not configurable. All supported cipher suites are safe,
and if PreferServerCipherSuites is set in Config the preference order is based
on the available hardware.

Fixing the test case for go1.13 by limiting the TLS version to TLS12
lilic and others added 3 commits April 19, 2021 11:49
Currently with race it fails, we can enable this at a later point.
In fact this commit rewrites the functionality to use upstream list of
ciphers instead of checking whether the lists are in sync using ast
analysis.
Cherry-pick of 60e4428 from master branch does not work due to
missing `tls.CipherSuites()` function. We work around by using go build
tags for both the building and tests.
…egration/...'

grpc proxy opens additional 2 watching channels. The metric is shared
between etcd-server & grpc_proxy, so all assertions on number of open
watch channels need to take in consideration the additional "2"
channels.
@ptabor ptabor changed the title [release-3.4]: .travis.yml: Test with go v1.15.7 [release-3.4]: .travis.yml: Test with go v1.15.11 Apr 19, 2021
@ptabor
Copy link
Contributor

ptabor commented Apr 19, 2021

TestBalancerUnderNetworkPartitionWatchFollower

FYI: I hope I fixed this flake in master in: a57e967

@lilic
Copy link
Contributor Author

lilic commented Apr 19, 2021

Thanks for the heads up @ptabor! Have no hit that one anymore it was mainly TestV3WatchCancellation for the past few runs so I cherry-picked one of your commits from master. But can cherry-pick the commit as well next time I hit TestBalancerUnderNetworkPartitionWatchFollower !

Also, it's green, please have another look, thank you! 🎉

@ptabor ptabor merged commit c274aa5 into etcd-io:release-3.4 Apr 19, 2021
@lilic lilic deleted the test-go-1-15 branch April 19, 2021 16:20
@hexfusion hexfusion linked an issue Apr 23, 2021 that may be closed by this pull request
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.

etcd Go maintenance plan
4 participants