Bump protobuf 1.2.0, and re-generate #2837

thaJeztah · 2019-03-20T11:41:18Z

thaJeztah · 2019-03-20T12:31:02Z

Hmf.. looks like protobuf 1.2.x is not backward compatible, and no longer allows using things as map keys;

remotes/remotes.go:73:17:warning: invalid map key type github.com/docker/swarmkit/api.Peer (unconvert)
remotes/remotes.go:69:49:warning: invalid map key type github.com/docker/swarmkit/api.Peer (unconvert)
remotes/remotes.go:60:14:warning: invalid map key type github.com/docker/swarmkit/api.Peer (unconvert)
remotes/remotes.go:49:21:warning: invalid map key type github.com/docker/swarmkit/api.Peer (unconvert)
remotes/remotes.go:23:16:warning: invalid map key type github.com/docker/swarmkit/api.Peer (unconvert)
agent/exec/dockerapi/executor.go:42:17:warning: invalid map key type github.com/docker/swarmkit/api.PluginDescription (unconvert)
agent/agent.go:394:14:warning: invalid map key type github.com/docker/swarmkit/api.Peer (unconvert)
manager/state/store/memory.go:583:6:warning: cannot compare oldN.GetMeta().Version != meta.Version (operator != not defined for github.com/docker/swarmkit/api.Version) (unconvert)
manager/allocator/cnmallocator/portallocator.go:41:25:warning: invalid map key type github.com/docker/swarmkit/api.PortConfig (unconvert)
manager/scheduler/scheduler.go:469:25:warning: cannot compare node.Meta.Version != nodeInfo.Meta.Version (operator != not defined for github.com/docker/swarmkit/api.Version) (unconvert)
manager/scheduler/scheduler.go:381:32:warning: invalid map key type commonSpecKey (unconvert)
manager/scheduler/nodeinfo.go:53:39:warning: invalid map key type versionedService (unconvert)
manager/scheduler/nodeinfo.go:39:21:warning: invalid map key type versionedService (unconvert)
manager/orchestrator/task.go:77:53:warning: cannot compare *s.SpecVersion == *t.SpecVersion (operator == not defined for github.com/docker/swarmkit/api.Version) (unconvert)
manager/orchestrator/restart/restart.go:366:43:warning: cannot compare *replacementTask.SpecVersion != restartInfo.specVersion (operator != not defined for github.com/docker/swarmkit/api.Version) (unconvert)
manager/orchestrator/restart/restart.go:231:52:warning: cannot compare *t.SpecVersion != restartInfo.specVersion (operator != not defined for github.com/docker/swarmkit/api.Version) (unconvert)
manager/dispatcher/dispatcher.go:710:8:warning: cannot compare task.Status == *status (operator == not defined for github.com/docker/swarmkit/api.TaskStatus) (unconvert)
remotes/remotes.go:23:12:error: invalid map key type api.Peer (vet)
remotes/remotes.go:49:17:error: invalid map key type api.Peer (vet)
remotes/remotes.go:60:10:error: invalid map key type api.Peer (vet)
remotes/remotes.go:69:45:error: invalid map key type api.Peer (vet)
remotes/remotes.go:73:13:error: invalid map key type api.Peer (vet)
manager/state/store/memory.go:583:29:error: invalid operation: oldN.GetMeta().Version != meta.Version (struct containing []byte cannot be compared) (vet)
agent/exec/dockerapi/executor.go:42:13:error: invalid map key type api.PluginDescription (vet)
manager/allocator/cnmallocator/portallocator.go:41:21:error: invalid map key type api.PortConfig (vet)
agent/exec/dockerapi/executor.go:42:13:error: invalid map key type api.PluginDescription (vet)
manager/allocator/cnmallocator/portallocator.go:41:21:error: invalid map key type api.PortConfig (vet)
manager/state/store/memory.go:583:29:error: invalid operation: oldN.GetMeta().Version != meta.Version (struct containing []byte cannot be compared) (vet)
remotes/remotes.go:23:12:error: invalid map key type api.Peer (vet)
remotes/remotes.go:49:17:error: invalid map key type api.Peer (vet)
remotes/remotes.go:60:10:error: invalid map key type api.Peer (vet)
remotes/remotes.go:69:45:error: invalid map key type api.Peer (vet)
remotes/remotes.go:73:13:error: invalid map key type api.Peer (vet)
remotes/remotes_test.go:15:15:error: invalid map key type api.Peer (vet)
remotes/remotes_test.go:86:11:error: invalid operation: next == peers[0] (struct containing []byte cannot be compared) (vet)
remotes/remotes_test.go:98:11:error: invalid operation: next != peers[0] (struct containing []byte cannot be compared) (vet)
remotes/remotes_test.go:254:8:error: invalid operation: p == peers[0] (struct containing []byte cannot be compared) (vet)
remotes/remotes_test.go:311:11:error: invalid operation: peer == peers[0] (struct containing []byte cannot be compared) (vet)
remotes/remotes_test.go:15:15:error: too many errors (vet)

See the discussion on golang/protobuf#594

In general, the proto specification does not formally define what equality means for protos. Since comparability is undefined, and using protos as map keys requires comparability, then that means using protos as map keys is undefined behavior. The fact that proto3 structs were comparable before was unfortunate because it gave the illusion that this was the case when it wasn't.

And related discussion on protocolbuffers/protobuf#272

thaJeztah · 2019-03-20T12:53:30Z

I see some mention here gogo/protobuf#275 of adding option (gogoproto.goproto_unrecognized) = false;

thaJeztah · 2019-03-20T13:32:21Z

Ok, looking much better with that option set; only see this failure;

manager/allocator/network.go:186::error: Entry.Errorf format %s has arg nc.ingressNetwork.Spec.Annotations of wrong type github.com/docker/swarmkit/api.Annotations (vet)
make: *** [check] Error 1
Exited with code 2

thaJeztah · 2019-03-20T15:09:36Z

Hm.. not out of the woods yet

--- FAIL: TestIsStateDirty (0.07s)
panic: unexpected field type in StoreSnapshot [recovered]
	panic: unexpected field type in StoreSnapshot

goroutine 42 [running]:
testing.tRunner.func1(0xc0005f1600)
	/usr/local/go/src/testing/testing.go:792 +0x6a7
panic(0x1344980, 0x1641540)
	/usr/local/go/src/runtime/panic.go:513 +0x1b9
github.com/docker/swarmkit/manager.(*Manager).IsStateDirty(0xc0000ba4e0, 0x15214e0, 0xc0000ba4e0, 0x1659ec0)
	/home/circleci/.go_workspace/src/github.com/docker/swarmkit/manager/dirty.go:48 +0xa53
github.com/docker/swarmkit/manager.TestIsStateDirty(0xc0005f1600)
	/home/circleci/.go_workspace/src/github.com/docker/swarmkit/manager/dirty_test.go:56 +0x715
testing.tRunner(0xc0005f1600, 0x1521598)
	/usr/local/go/src/testing/testing.go:827 +0x163
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:878 +0x651
FAIL	github.com/docker/swarmkit/manager	0.376s

Panic comes from here; https://github.com/docker/swarmkit/blob/cb78da37418bbee263ecb6e8ab4c83b51a93b8dc/manager/dirty.go#L13-L57

Before: panic: unexpected field type in StoreSnapshot After: panic: unexpected field type in StoreSnapshot: XXX_NoUnkeyedLiteral struct Signed-off-by: Sebastiaan van Stijn <[email protected]>

Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah · 2019-10-20T12:47:40Z

Looks like the bump was taken care of in #2886

I'll open a PR for the remaining changes (could be useful)

codecov · 2019-10-20T13:02:34Z

Codecov Report

Merging #2837 into master will decrease coverage by 0.19%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master    #2837     +/-   ##
=========================================
- Coverage   61.62%   61.42%   -0.2%     
=========================================
  Files         139      139             
  Lines       22615    22615             
=========================================
- Hits        13936    13892     -44     
- Misses       7194     7248     +54     
+ Partials     1485     1475     -10

thaJeztah force-pushed the bump_protobuf branch from 5965cf1 to 92148fa Compare March 20, 2019 13:35

thaJeztah mentioned this pull request Mar 20, 2019

Bump protobuf v1.2.0 containerd/containerd#3116

Merged

thaJeztah force-pushed the bump_protobuf branch from 9029a54 to 44cfaf9 Compare April 3, 2019 22:57

thaJeztah mentioned this pull request Apr 13, 2019

Update gogo/protobuf to v1.1.1 #2713

Closed

thaJeztah added 2 commits October 20, 2019 14:45

Improve information if IsStateDirty() fails

3547e72

Before: panic: unexpected field type in StoreSnapshot After: panic: unexpected field type in StoreSnapshot: XXX_NoUnkeyedLiteral struct Signed-off-by: Sebastiaan van Stijn <[email protected]>

FIXME manually gofmt test file

691a1d8

Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah force-pushed the bump_protobuf branch from 9f80970 to 691a1d8 Compare October 20, 2019 12:46

thaJeztah mentioned this pull request Oct 20, 2019

Improve information if IsStateDirty() fails #2909

Merged

thaJeztah closed this Oct 20, 2019

thaJeztah deleted the bump_protobuf branch October 20, 2019 12:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump protobuf 1.2.0, and re-generate #2837

Bump protobuf 1.2.0, and re-generate #2837

thaJeztah commented Mar 20, 2019

thaJeztah commented Mar 20, 2019 •

edited

Loading

thaJeztah commented Mar 20, 2019

thaJeztah commented Mar 20, 2019

thaJeztah commented Mar 20, 2019 •

edited

Loading

thaJeztah commented Oct 20, 2019

codecov bot commented Oct 20, 2019

Bump protobuf 1.2.0, and re-generate #2837

Bump protobuf 1.2.0, and re-generate #2837

Conversation

thaJeztah commented Mar 20, 2019

thaJeztah commented Mar 20, 2019 • edited Loading

thaJeztah commented Mar 20, 2019

thaJeztah commented Mar 20, 2019

thaJeztah commented Mar 20, 2019 • edited Loading

thaJeztah commented Oct 20, 2019

codecov bot commented Oct 20, 2019

Codecov Report

thaJeztah commented Mar 20, 2019 •

edited

Loading

thaJeztah commented Mar 20, 2019 •

edited

Loading