Ensure controlapi Server always has an up to date allocator#2231
Ensure controlapi Server always has an up to date allocator#2231ijc wants to merge 1 commit intomoby:masterfrom
Conversation
Currently the allocator is only created when a node becomes the leader, which is (almost always) after the controlapi.NewServer call. Hence the server is always being passed a m.allocator which is nil. In addition the server wasn't actually remembering the allocator it was passed. Add a second layer of indirection to the allocator stashed by the controlapi Server, so it will always see the latest version of m.allocator. This stops moby#2230 from happening, but is really only a stopgap until a more satisfactory solution can be found. This regression was made by me in fb74191 ("Convert NetworkAllocator to an interface"). Add a call to CreateNetwork to TestManager intended to exercise the code path through validateNetwork to allocator.IsBuiltinNetworkDriver. Signed-off-by: Ian Campbell <ian.campbell@docker.com>
|
Not very happy with this long term, but will discuss potential proper solutions over on #2230. |
Codecov Report
@@ Coverage Diff @@
## master #2231 +/- ##
==========================================
- Coverage 60.16% 60.09% -0.07%
==========================================
Files 124 124
Lines 20156 20157 +1
==========================================
- Hits 12126 12114 -12
- Misses 6669 6674 +5
- Partials 1361 1369 +8 |
| // - Returns an error if the creation fails. | ||
| func (s *Server) CreateNetwork(ctx context.Context, request *api.CreateNetworkRequest) (*api.CreateNetworkResponse, error) { | ||
| if err := validateNetworkSpec(request.Spec, s.a, s.pg); err != nil { | ||
| if err := validateNetworkSpec(request.Spec, *(s.a), s.pg); err != nil { |
There was a problem hiding this comment.
Moot given the subsequent discussion, but I could have sworn that didn't compile. I just tried again and it is fine, no idea what I had done differently on my first try.
|
I'm not enthusiastic about this solution. It's not obvious to me that the pointer read can't race with I think we should move Perhaps a simple temporary solution would be to define an interface specifically for checking whether a driver is built-in, initializing that before everything else, and passing it into |
|
Short term I'll switch I'll then incorporate the |
|
Replacing with #2240 |
Currently the allocator is only created when a node becomes the leader, which
is (almost always) after the controlapi.NewServer call. Hence the server is
always being passed a m.allocator which is nil.
In addition the server wasn't actually remembering the allocator it was passed.
Add a second layer of indirection to the allocator stashed by the controlapi
Server, so it will always see the latest version of m.allocator.
This stops #2230 from happening, but is really only a stopgap until a more
satisfactory solution can be found.
This regression was made by me in fb74191 ("Convert NetworkAllocator to an
interface"). Add a call to CreateNetwork to TestManager intended to exercise
the code path through validateNetwork to allocator.IsBuiltinNetworkDriver.
Signed-off-by: Ian Campbell ian.campbell@docker.com