Adding compatible platforms to service spec#33239
Conversation
client/service_create.go
Outdated
There was a problem hiding this comment.
This is changing the pointer, not the value
client/service_create.go
Outdated
|
Tests? |
|
@aaronlehmann I think my commit was overwritten, so this is stale. Updating it. Where should the tests ideally be put for this? |
client/service_create_test.go |
b338a13 to
6ff0852
Compare
5a1236e to
624bd2e
Compare
|
@aaronlehmann updated PR and added a unit test (the unit test is somewhat of a hack to test the platform response at the API level, but hopefully it is okay). |
|
Thanks. Could you please extend the unit test to check that the correct platform information is being passed to the |
Signed-off-by: Nishant Totla <[email protected]>
624bd2e to
587d07c
Compare
|
@aaronlehmann done. |
|
I'm not seeing it. The request body to |
client/service_create_test.go
Outdated
There was a problem hiding this comment.
This comparison won't work. They are both slices.
92e56c7 to
c2c891a
Compare
Signed-off-by: Nishant Totla <[email protected]>
c2c891a to
7d4b8fb
Compare
|
LGTM |
|
LGTM, not sure if we should sort the platforms array but shouldn't be a blocker |
|
Should be fine like this, sorting is usually for display |
|
@tiborvass we could do it as a follow-up, it's not critical right now. |
Signed-off-by: Nishant Totla [email protected]
This is a follow-up to #32388 (Related to: #31348).
It extracts platform information that is already present after the call to the
/distribution/jsonendpoint (#32061), and adds it to the service spec. This platform information is used as a Swarm scheduling filter, to prevent tasks for running on nodes where they shouldn't run.CLI changes in docker/cli#30 need to be merged before this actually is enabled end to end. We'll also have to merge #33228, which includes a SwarmKit bug fix that is required for smooth functioning of this feature.