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

go.mod: name packages with go.etcd.io/etcd/v3 #11823

Merged
merged 8 commits into from
Apr 28, 2020

Conversation

philips
Copy link
Contributor

@philips philips commented Apr 26, 2020

Attempt to fix #11820

  • make works
  • ./scripts/genproto.sh makes no changes after running
  • make test passes
  • ensure bill-of-materials.json is OK
  • TEST_OPTS="PASSES='functional'" make test

@codecov-io
Copy link

codecov-io commented Apr 26, 2020

Codecov Report

Merging #11823 into master will decrease coverage by 0.39%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11823      +/-   ##
==========================================
- Coverage   66.29%   65.89%   -0.40%     
==========================================
  Files         403      403              
  Lines       36984    36984              
==========================================
- Hits        24517    24370     -147     
- Misses      10980    11099     +119     
- Partials     1487     1515      +28     
Impacted Files Coverage Δ
auth/range_perm_cache.go 68.57% <ø> (ø)
auth/store.go 52.38% <ø> (-3.66%) ⬇️
client/client.go 83.98% <ø> (ø)
client/discover.go 0.00% <ø> (ø)
client/keys.go 55.77% <ø> (-35.68%) ⬇️
client/members.go 85.48% <ø> (ø)
clientv3/auth.go 51.25% <ø> (ø)
clientv3/balancer/balancer.go 86.36% <ø> (ø)
clientv3/client.go 74.05% <ø> (-0.59%) ⬇️
clientv3/clientv3util/util.go 0.00% <ø> (ø)
... and 184 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 262c939...c9cdefa. Read the comment docs.

@philips
Copy link
Contributor Author

philips commented Apr 27, 2020

Update: resolved in cf9c568

This is weird, consistently getting this error after all of this:

coverage: 28.1% of statements                                                                                                      
Too many goroutines running after all test(s).                                                                                     
1 instances of:                                                                                                                    
go.etcd.io/etcd/v3/pkg/testutil.interestingGoroutines(...)                                                                         
        /home/philips/etcd/pkg/testutil/leak.go:113 +0x8f                                                                          
go.etcd.io/etcd/v3/pkg/testutil.CheckLeakedGoroutine(...)                                                                          
        /home/philips/etcd/pkg/testutil/leak.go:45 +0xe1                                                                           
go.etcd.io/etcd/v3/etcdserver/api/v2v3_test.TestMain(...)                                                                          
        /home/philips/etcd/etcdserver/api/v2v3/main_test.go:40 +0x31c                                                              
main.main()                                                                                                                        
        _testmain.go:104 +0x334  

@philips philips marked this pull request as draft April 27, 2020 03:48
philips added 7 commits April 28, 2020 00:57
This change makes the etcd package compatible with the existing Go
ecosystem for module versioning.

Used this tool to update package imports:
  https://github.com/KSubedi/gomove
Found potential locations via

```
git grep 'go.etcd.io/etcd$'
```
add go.etcd.io/etcd/v3 to existing filters
bill-of-materials:
- update to new package names

test/updatebom.sh scripts:
- Update to the right package names
- Don't add bom tool to go.mod
gofmt tests were failing, fix.
@philips philips marked this pull request as ready for review April 28, 2020 01:02
@philips
Copy link
Contributor Author

philips commented Apr 28, 2020

Integration is failing on a test that almost certainly latency flakey:

--- FAIL: TestBalancerUnderNetworkPartitionWatchLeader (3.24s)
2220    network_partition_test.go:266: took too long to detect leader lost

@philips philips changed the title WIP go.etcd.io/etcd/v3 in go.mod attempt2 go.mod: name packages with go.etcd.io/etcd/v3 Apr 28, 2020
@philips philips requested a review from gyuho April 28, 2020 05:40
@xiang90
Copy link
Contributor

xiang90 commented Apr 28, 2020

@philips

Have you tried to write a simple program with etcd/client dependency?

@philips
Copy link
Contributor Author

philips commented Apr 28, 2020 via email

@xiang90
Copy link
Contributor

xiang90 commented Apr 28, 2020

What do you mean? What is it you are trying to test?

Something like

import go.etcd.io/etcd/v3/clientv3

func main() {
    _, _ = clientv3.New(...)
   ...
}

Just to make sure the client dependency can be resolved correctly outside the etcd project.

@xiang90
Copy link
Contributor

xiang90 commented Apr 28, 2020

I gave it a try. It seems working well.

@philips
Copy link
Contributor Author

philips commented Apr 28, 2020 via email

@xiang90
Copy link
Contributor

xiang90 commented Apr 28, 2020

LGTM

@philips
Copy link
Contributor Author

philips commented Apr 28, 2020

cc @etcd-io/maintainers-etcd we should all take a look at this go.mod change

Copy link
Contributor

@gyuho gyuho left a comment

Choose a reason for hiding this comment

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

awesome, thank you so much!

@philips
Copy link
Contributor Author

philips commented Apr 28, 2020 via email

Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

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

Great!! Thanks @philips lgtm

@gyuho
Copy link
Contributor

gyuho commented Apr 28, 2020

@philips Can we update https://github.com/etcd-io/etcd/blob/master/CHANGELOG-3.5.md in a separate PR (or just separate commit is good too)?

@philips
Copy link
Contributor Author

philips commented Apr 28, 2020

i will add the changelog and try to fix markdown docs before merging

Use the new package path in the docs and announce it in the CHANGELOG
@philips
Copy link
Contributor Author

philips commented Apr 28, 2020

@gyuho Take a look at the docs and CHANGELOG entry here: d88d765

@philips philips merged commit b28a627 into etcd-io:master Apr 28, 2020
This was referenced Apr 28, 2020
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Jul 7, 2020
with the merge of etcd-io/etcd#11823
etcd v3.5.0 will now have a properly imported versioned path

this fixes our pending migration to newer repo
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Jul 7, 2020
with the merge of etcd-io/etcd#11823
etcd v3.5.0 will now have a properly imported versioned path

this fixes our pending migration to newer repo
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Jul 7, 2020
with the merge of etcd-io/etcd#11823
etcd v3.5.0 will now have a properly imported versioned path

this fixes our pending migration to newer repo
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Jul 7, 2020
with the merge of etcd-io/etcd#11823
etcd v3.5.0 will now have a properly imported versioned path

this fixes our pending migration to newer repo
harshavardhana added a commit to minio/minio that referenced this pull request Jul 8, 2020
with the merge of etcd-io/etcd#11823
etcd v3.5.0 will now have a properly imported versioned path

this fixes our pending migration to newer repo
@ymmt2005 ymmt2005 mentioned this pull request Aug 16, 2020
5 tasks
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.

fix go.etcd.io/etcd versioned import path
5 participants