Update and validation of generated files#44697
Conversation
cc61c26 to
52430e3
Compare
libnetwork/service_linux.go
Outdated
| ingressProxyTbl = make(map[string]io.Closer) | ||
| portConfigMu sync.Mutex | ||
| portConfigTbl = make(map[PortConfig]int) | ||
| portConfigTbl = make(map[*PortConfig]int) |
There was a problem hiding this comment.
This changes the map keys from matching by-value (i.e. the (name, protocol, target_port, published_port) tuple) to matching by reference identity. PortConfig{} == PortConfig{} but &PortConfig{} != &PortConfig{}. The by-value behaviour is clearly the intended one, so this change will break things. And annoyingly there isn't any guarantee that protobuf serialization is deterministic so that wouldn't work either. While there may be tuning knobs in gogo protobuf to stop it from emitting the extra XXX_ fields in PortConfig, that'd be only a stopgap measure as gogo protobuf is officially deprecated. Unfortunately, the only forward-compatible solution I can see is to define a parallel struct type by hand for use as map keys.
52430e3 to
6e5e0bb
Compare
corhere
left a comment
There was a problem hiding this comment.
Nice find with gogofaster! We can figure out a future-proof solution later, when we tackle replacing gogo protobuf with something maintained.
6e5e0bb to
0744f93
Compare
0744f93 to
aa94798
Compare
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Adds a Dockerfile and make targets to update and validate generated files (proto, seccomp default profile) Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
gogofaster is identical as gogofast but removes XXX_unrecognized Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
There was a problem hiding this comment.
- Note that this file is being removed in builder/remotecontext: remove CachableSource, NewCachableSource #45627
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
let's bring this one in first, in case we still need the updated builder/remotecontext/tarsum.pb.go for whatever reason before we remove it.
There was a problem hiding this comment.
nit: for other Dockerfiles in this repository, we use Dockerfile.<something> as convention; should we stick to a single convention?
fixes #40176
- What I did
Adds a Dockerfile and Make targets to update and validate generated files via
go:generate(proto, seccomp default profile)- How I did it
Needed some changes in proto definitions, specially ones for libnetwork pkg that were not updated after being moved to this repo.
I use a dedicated Dockerfile as the main one is quite huge.
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)