Check that Placement is not null before trying to access it #2950
Conversation
|
Hi @sgiacomel, could you please rebase this PR to see if the build still fails? I believe the build failure is not connected to this change and there were some commits to master fiddling with protos since this branch was created. Thanks in advance! |
|
Hi! I'd be happy to open a rebased version of this PR, in case @sgiacomel can't find the time to do it. |
Signed-off-by: Simone Giacomel <[email protected]>
Met the requirements if the constraints are empty
sgiacomel
force-pushed
the
2947-check-placement-not-null
branch
from
c479e01
to
9cb517e
Compare
Hi, thank you for poking, I just rebased it. @miklos-martin |
Codecov Report
@@ Coverage Diff @@
## master #2950 +/- ##
==========================================
- Coverage 61.77% 60.21% -1.57%
==========================================
Files 142 142
Lines 23005 20443 -2562
==========================================
- Hits 14211 12309 -1902
+ Misses 7303 6661 -642
+ Partials 1491 1473 -18 |
|
Nice, thanks |
|
@sgiacomel Maybe it's worth adding a test case in |
|
@dperny @thaJeztah Could you take a look? 🙂 |
If Placement is missing after updating a service with previous placement constraints in a swarm with multiple nodes, it sends the swarm in an unrecoverable state.
closes #2947
closes moby/moby#37883
- What I did
I created a swarm with 2 nodes. I created a service using the docker api with a placement constraint. I then updated the service passing a json load with no
Placementfield.moby/moby#37883 (comment)
- How I did it
I mainly used curl to issue docker api calls
- How to test it
I can provide more details
- Description for the changelog
Prevents nil pointer reference in nodeMatches when Placement is null
Signed-off-by: Simone Giacomel [email protected]