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

[WIP][18.03 backport] dependency changes for newer Go versions #2881

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Aug 19, 2019

# https://github.com/docker/swarmkit/pull/2541 Log GRPC server errors
git cherry-pick -s -S -x cb3b9a72760e1401b88a2a29333abb3524964ddc

# https://github.com/docker/swarmkit/pull/2610 Don't use wrappers for grpc metadata
git cherry-pick -s -S -x 7bbda7ca903f4a31203696e413ec2ac343428c39

# https://github.com/docker/swarmkit/pull/2452 vendor: update gRPC, protobuf
git cherry-pick -s -S -x 59f5b65f0adb67ce417c0424dcefa0c587413201 \
	61c4bd2b513ee0dce41c3dc2977e550523be3915 \
	58cea3d232366b9f4457ff8baa32d002c348463c \
	8a2b6fd64944bcef8154ced28f90aeec6abfeb04


# conflict in first commit because https://github.com/docker/swarmkit/pull/2616 manually updated vendoring;
# Unmerged paths:
#
#	both added:      vendor/golang.org/x/crypto/otr/otr.go
#	both added:      vendor/golang.org/x/sys/windows/svc/go12.go
#	both added:      vendor/golang.org/x/sys/windows/svc/go13.go


# https://github.com/docker/swarmkit/pull/2644 Bump CFSSL
(skipped) git cherry-pick -s -S -x fc8573a2ed730d15a951d5058950f13640df3943


# https://github.com/docker/swarmkit/pull/2649 Bumps GRPC to 1.12.0
git cherry-pick -s -S -x f1afeb17c24376b8aa01d9b2ecb2b59f63578882 \
	4343384f11737119c3fa1524da2cb2707c70e04a \
	6a6993f97a87d17a0f938ef59fcab72ba126a7b8 \
	90418ad1445919c54305b5356146fa15574f8642 \
	e54ad63ab828a231d697b4106d345440b9b53150 \
	8f58755b7a0bb01018694320451cb470e558c9a9

Changes the grpc unary and stream interceptors to implement logging of
errors returned by RPCs (while also letting the prometheus interceptors
do their thing).

The biggest motivation for this change is that if a client times out
while a grpc method is happening, the client may not ever get an error
from the server, but the server should hopefully log an error explaining
why it couldn't handle the request (like what part it may have been
blocked on).

Signed-off-by: Drew Erny <drew.erny@docker.com>
(cherry picked from commit cb3b9a7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

More changes needed;

# github.com/docker/swarmkit/api
api/ca.pb.go:974:13: undefined: metadata.FromContext
api/ca.pb.go:982:10: undefined: metadata.NewContext
api/ca.pb.go:1116:13: undefined: metadata.FromContext
api/ca.pb.go:1124:10: undefined: metadata.NewContext
api/control.pb.go:5847:13: undefined: metadata.FromContext
api/control.pb.go:5855:10: undefined: metadata.NewContext
api/dispatcher.pb.go:1590:13: undefined: metadata.FromContext
api/dispatcher.pb.go:1598:10: undefined: metadata.NewContext
api/health.pb.go:274:13: undefined: metadata.FromContext
api/health.pb.go:282:10: undefined: metadata.NewContext
api/health.pb.go:282:10: too many errors
make: *** [bin/swarmd] Error 2
Exited with code 2

@thaJeztah
Copy link
Member Author

Looks like we need #2649 after all 😓

crosbymichael and others added 11 commits August 20, 2019 01:05
This switches the current usage of NewContext and FromContext to use the
underlying methods in the metadata package.  This will be removed in
future versions of grpc.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
(cherry picked from commit 7bbda7c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 59f5b65)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 61c4bd2)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 58cea3d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Fixes an issue resulting from string matching the error result of grpc
methods. Adds comments documenting the fix, as well as explaining how to
avoid this issue in the future.

Also updates manager/manager.go to use the function MaxRecvMsgSize, instead
of the deprecated equivalent MaxMsgSize.

Signed-off-by: Drew Erny <drew.erny@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 8a2b6fd)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1. We now call `Clone` on the tls config when cloning the MutableTLSConfig
because you can't reuse a tls.Config for multiple connections, which we
would be doing if something called MutableTLSCreds.Clone, which newer
versions of GRPC do.

2. To prevent data races when grpc attempts to log, only set the grpc logger
to discard in test init functions, rather than in TestMain functions as is
documented by the `SetLoggerV2` function.  Also, use `SetLoggerV2` since
SetLogger is deprecated.

3. Wrap the default logger in a private struct that converts a logrus
entry into a grpc LoggerV2, since the grpc Logger is deprecated.

4. Stop using transport.StreamFromContext to get the peer address when checking
redirects in the raft proxy.  grpc/transport is an internal package that
downstream should not depend on, as the API may change (StreamFromContext
for example is removed in >=1.11).  peer.FromContext can get the peer address
just as well.

Signed-off-by: Ying Li <ying.li@docker.com>
(cherry picked from commit f1afeb1)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1. The GRPC update stopped returning errors when the listener underlying
a GRPC server closed, so remove that assertion from the tests.

2. Change the first error tracking in node not to check the GRPC error type for
x509 errors, since it's changed between GRPC <1.10 and >=1.10, and the error
message has changed as well in >=1.11. Also change the error message checking
to something that works in <1.10 as well as >=1.11.

Signed-off-by: Ying Li <ying.li@docker.com>
(cherry picked from commit 4343384)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…r that

can surface TLS handshake errors, which GRPC considers transient.

Signed-off-by: Ying Li <ying.li@docker.com>
(cherry picked from commit 6a6993f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
to reconnect.  Just return an error - if an error is returned, the agent
will reconnect anyway - it seems like the transport closing was a way
to convince GRPC's load balancer to start reconnecting automatically.
See moby#797.

Signed-off-by: Ying Li <ying.li@docker.com>
(cherry picked from commit 90418ad)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
no longer returns an EOF.

Signed-off-by: Ying Li <ying.li@docker.com>
(cherry picked from commit e54ad63)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Ying Li <ying.li@docker.com>
(cherry picked from commit 8f58755)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the 18.03_backport_bump_vendors_for_golang branch from 9d4808c to dbee359 Compare August 19, 2019 23:05
@thaJeztah
Copy link
Member Author

Vendor check failing

diff -r vendor/github.com/golang/protobuf/ptypes/empty/empty.pb.go .vendor.bak/github.com/golang/protobuf/ptypes/empty/empty.pb.go
4c4,13
< package empty // import "github.com/golang/protobuf/ptypes/empty"
---
> /*
> Package empty is a generated protocol buffer package.
> 
> It is generated from these files:
> 	google/protobuf/empty.proto
> 
> It has these top-level messages:
> 	Empty
> */
> package empty
31,56d39
< 	XXX_NoUnkeyedLiteral struct{} `json:"-"`
< 	XXX_unrecognized     []byte   `json:"-"`
< 	XXX_sizecache        int32    `json:"-"`
< }
< 
< func (m *Empty) Reset()         { *m = Empty{} }
< func (m *Empty) String() string { return proto.CompactTextString(m) }
< func (*Empty) ProtoMessage()    {}
< func (*Empty) Descriptor() ([]byte, []int) {
< 	return fileDescriptor_empty_39e6d6db0632e5b2, []int{0}
< }
< func (*Empty) XXX_WellKnownType() string { return "Empty" }
< func (m *Empty) XXX_Unmarshal(b []byte) error {
< 	return xxx_messageInfo_Empty.Unmarshal(m, b)
< }
< func (m *Empty) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) {
< 	return xxx_messageInfo_Empty.Marshal(b, m, deterministic)
< }
< func (dst *Empty) XXX_Merge(src proto.Message) {
< 	xxx_messageInfo_Empty.Merge(dst, src)
< }
< func (m *Empty) XXX_Size() int {
< 	return xxx_messageInfo_Empty.Size(m)
< }
< func (m *Empty) XXX_DiscardUnknown() {
< 	xxx_messageInfo_Empty.DiscardUnknown(m)
59c42,46
< var xxx_messageInfo_Empty proto.InternalMessageInfo
---
> func (m *Empty) Reset()                    { *m = Empty{} }
> func (m *Empty) String() string            { return proto.CompactTextString(m) }
> func (*Empty) ProtoMessage()               {}
> func (*Empty) Descriptor() ([]byte, []int) { return fileDescriptor0, []int{0} }
> func (*Empty) XXX_WellKnownType() string   { return "Empty" }
65c52
< func init() { proto.RegisterFile("google/protobuf/empty.proto", fileDescriptor_empty_39e6d6db0632e5b2) }
---
> func init() { proto.RegisterFile("google/protobuf/empty.proto", fileDescriptor0) }
67c54
< var fileDescriptor_empty_39e6d6db0632e5b2 = []byte{
---
> var fileDescriptor0 = []byte{
diff -r vendor/google.golang.org/grpc/health/grpc_health_v1/health.pb.go .vendor.bak/google.golang.org/grpc/health/grpc_health_v1/health.pb.go
4c4,14
< package grpc_health_v1 // import "google.golang.org/grpc/health/grpc_health_v1"
---
> /*
> Package grpc_health_v1 is a generated protocol buffer package.
> 
> It is generated from these files:
> 	grpc_health_v1/health.proto
> 
> It has these top-level messages:
> 	HealthCheckRequest
> 	HealthCheckResponse
> */
> package grpc_health_v1
49c59
< 	return fileDescriptor_health_8e5b8a3074428511, []int{1, 0}
---
> 	return fileDescriptor0, []int{1, 0}
53,78c63
< 	Service              string   `protobuf:"bytes,1,opt,name=service" json:"service,omitempty"`
< 	XXX_NoUnkeyedLiteral struct{} `json:"-"`
< 	XXX_unrecognized     []byte   `json:"-"`
< 	XXX_sizecache        int32    `json:"-"`
< }
< 
< func (m *HealthCheckRequest) Reset()         { *m = HealthCheckRequest{} }
< func (m *HealthCheckRequest) String() string { return proto.CompactTextString(m) }
< func (*HealthCheckRequest) ProtoMessage()    {}
< func (*HealthCheckRequest) Descriptor() ([]byte, []int) {
< 	return fileDescriptor_health_8e5b8a3074428511, []int{0}
< }
< func (m *HealthCheckRequest) XXX_Unmarshal(b []byte) error {
< 	return xxx_messageInfo_HealthCheckRequest.Unmarshal(m, b)
< }
< func (m *HealthCheckRequest) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) {
< 	return xxx_messageInfo_HealthCheckRequest.Marshal(b, m, deterministic)
< }
< func (dst *HealthCheckRequest) XXX_Merge(src proto.Message) {
< 	xxx_messageInfo_HealthCheckRequest.Merge(dst, src)
< }
< func (m *HealthCheckRequest) XXX_Size() int {
< 	return xxx_messageInfo_HealthCheckRequest.Size(m)
< }
< func (m *HealthCheckRequest) XXX_DiscardUnknown() {
< 	xxx_messageInfo_HealthCheckRequest.DiscardUnknown(m)
---
> 	Service string `protobuf:"bytes,1,opt,name=service" json:"service,omitempty"`
81c66,69
< var xxx_messageInfo_HealthCheckRequest proto.InternalMessageInfo
---
> func (m *HealthCheckRequest) Reset()                    { *m = HealthCheckRequest{} }
> func (m *HealthCheckRequest) String() string            { return proto.CompactTextString(m) }
> func (*HealthCheckRequest) ProtoMessage()               {}
> func (*HealthCheckRequest) Descriptor() ([]byte, []int) { return fileDescriptor0, []int{0} }
91,116c79
< 	Status               HealthCheckResponse_ServingStatus `protobuf:"varint,1,opt,name=status,enum=grpc.health.v1.HealthCheckResponse_ServingStatus" json:"status,omitempty"`
< 	XXX_NoUnkeyedLiteral struct{}                          `json:"-"`
< 	XXX_unrecognized     []byte                            `json:"-"`
< 	XXX_sizecache        int32                             `json:"-"`
< }
< 
< func (m *HealthCheckResponse) Reset()         { *m = HealthCheckResponse{} }
< func (m *HealthCheckResponse) String() string { return proto.CompactTextString(m) }
< func (*HealthCheckResponse) ProtoMessage()    {}
< func (*HealthCheckResponse) Descriptor() ([]byte, []int) {
< 	return fileDescriptor_health_8e5b8a3074428511, []int{1}
< }
< func (m *HealthCheckResponse) XXX_Unmarshal(b []byte) error {
< 	return xxx_messageInfo_HealthCheckResponse.Unmarshal(m, b)
< }
< func (m *HealthCheckResponse) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) {
< 	return xxx_messageInfo_HealthCheckResponse.Marshal(b, m, deterministic)
< }
< func (dst *HealthCheckResponse) XXX_Merge(src proto.Message) {
< 	xxx_messageInfo_HealthCheckResponse.Merge(dst, src)
< }
< func (m *HealthCheckResponse) XXX_Size() int {
< 	return xxx_messageInfo_HealthCheckResponse.Size(m)
< }
< func (m *HealthCheckResponse) XXX_DiscardUnknown() {
< 	xxx_messageInfo_HealthCheckResponse.DiscardUnknown(m)
---
> 	Status HealthCheckResponse_ServingStatus `protobuf:"varint,1,opt,name=status,enum=grpc.health.v1.HealthCheckResponse_ServingStatus" json:"status,omitempty"`
119c82,85
< var xxx_messageInfo_HealthCheckResponse proto.InternalMessageInfo
---
> func (m *HealthCheckResponse) Reset()                    { *m = HealthCheckResponse{} }
> func (m *HealthCheckResponse) String() string            { return proto.CompactTextString(m) }
> func (*HealthCheckResponse) ProtoMessage()               {}
> func (*HealthCheckResponse) Descriptor() ([]byte, []int) { return fileDescriptor0, []int{1} }
206c172
< func init() { proto.RegisterFile("grpc_health_v1/health.proto", fileDescriptor_health_8e5b8a3074428511) }
---
> func init() { proto.RegisterFile("grpc_health_v1/health.proto", fileDescriptor0) }
208,209c174,175
< var fileDescriptor_health_8e5b8a3074428511 = []byte{
< 	// 269 bytes of a gzipped FileDescriptorProto
---
> var fileDescriptor0 = []byte{
> 	// 213 bytes of a gzipped FileDescriptorProto
222,226c188,189
< 	0xb8, 0xd6, 0x29, 0x91, 0x4b, 0x30, 0x33, 0x1f, 0x4d, 0xa1, 0x13, 0x37, 0x44, 0x65, 0x00, 0x28,
< 	0x70, 0x03, 0x18, 0xa3, 0x74, 0xd2, 0xf3, 0xf3, 0xd3, 0x73, 0x52, 0xf5, 0xd2, 0xf3, 0x73, 0x12,
< 	0xf3, 0xd2, 0xf5, 0xf2, 0x8b, 0xd2, 0xf5, 0x41, 0x1a, 0xa0, 0x71, 0xa0, 0x8f, 0x1a, 0x33, 0xab,
< 	0x98, 0xf8, 0xdc, 0x41, 0xa6, 0x41, 0x8c, 0xd0, 0x0b, 0x33, 0x4c, 0x62, 0x03, 0x47, 0x92, 0x31,
< 	0x20, 0x00, 0x00, 0xff, 0xff, 0xb7, 0x70, 0xc4, 0xa7, 0xc3, 0x01, 0x00, 0x00,
---
> 	0xb8, 0x36, 0x89, 0x0d, 0x1c, 0x82, 0xc6, 0x80, 0x00, 0x00, 0x00, 0xff, 0xff, 0x53, 0x2b, 0x65,
> 	0x20, 0x60, 0x01, 0x00, 0x00,
diff -r vendor/google.golang.org/grpc/health/grpc_health_v1/health.proto .vendor.bak/google.golang.org/grpc/health/grpc_health_v1/health.proto
1,2c1
< // Copyright 2015, gRPC Authors
< // All rights reserved.
---
> // Copyright 2017 gRPC authors.
16,18d14
< // The canonical version of this proto can be found at
< // https://github.com/grpc/grpc-proto/blob/master/grpc/health/v1/health.proto
< 
23,28d18
< option csharp_namespace = "Grpc.Health.V1";
< option go_package = "google.golang.org/grpc/health/grpc_health_v1";
< option java_multiple_files = true;
< option java_outer_classname = "HealthProto";
< option java_package = "io.grpc.health.v1";
< 
35,37c25,27
<     UNKNOWN = 0;
<     SERVING = 1;
<     NOT_SERVING = 2;
---
>  	UNKNOWN = 0;
> 	SERVING = 1;
> 	NOT_SERVING = 2;
42c32
< service Health {
---
> service Health{
44c34
< }
---
> } 
+ inconsistent dependencies! what you have in vendor.conf does not match with what you have in vendor
make: *** [dep-validate] Error 1
Exited with code 2

@thaJeztah
Copy link
Member Author

thaJeztah commented Aug 19, 2019

Perhaps needs #2283 added (reverted)

@thaJeztah thaJeztah changed the title [18.03 backport] backport changes for newer Go versions [18.03 backport] dependency changes for newer Go versions Aug 20, 2019
@thaJeztah thaJeztah changed the title [18.03 backport] dependency changes for newer Go versions [WIP][18.03 backport] dependency changes for newer Go versions Aug 20, 2019
Signed-off-by: Drew Erny <drew.erny@docker.com>
@dperny
Copy link
Collaborator

dperny commented Sep 20, 2019

@thaJeztah I reran vndr and make generate and pushed up the results. Hopefully this will fix the broken test and we can merge.

@thaJeztah
Copy link
Member Author

Ah, whoops; I don't think we need this; after some discussion, updating all of these dependencies was only needed for a small change in one dependency; changing that dependency caused a huge ripple effect to other dependencies. For now, we resolved that by forking cfssl and backporting google/certificate-transparency-go#286

I'll close this, but keep the branch for now, in case we need to revisit that decision 🤗

@thaJeztah thaJeztah closed this Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants