Inspect output on service without labels is an empty map instead of null, fixes #24631

[adshmh]

Added code to output an empty map, instead of null, by the inspect command for labels of a service that has no labels. Fixes #24631 (also changes 'null' output to an empty object for other types with labels: secret, node, task, network, swarm)

Signed-off-by: Arash Deshmeh adeshmeh@ca.ibm.com

- What I did
The output of inspect command for labels of a service that has no labels is currently 'null', which is inconsistent with other objects, e.g. inspect output for labels of containers without labels is an empty object: {}. This PR fixes this inconsistency by changing the output to '{}' when the service has no labels.

- How I did it
Added code which replaces nil labels maps on service objects with an empty map, when retrieved by the daemon.

- How to verify it
Create a service:
$ docker service create --name nolabels nginx:alpine

Run inspect on the service, with Labels specified in format string.

• Without this PR:
  $ docker service inspect --format '{{ json .Spec.Labels }}' nolabels
  null

• With this PR included:
  $ docker service inspect --format '{{ json .Spec.Labels }}' nolabels
  {}

- Description for the changelog
The inspect command outputs an empty json object, i.e. {}, instead of null, for labels of a service that has no labels

- A picture of a cute animal (not mandatory but encouraged)

[vdemeester]

LGTM 🐯

[aaronlehmann]

I don't think the omitempty here is related to #24631. I don't think there's any harm in continuing to omit the Labels field when it's empty.

[adshmh]

Thank you for the review. I removed the omitempty because my tests show 'null' as output with omitempty left as is, while including all other changes of the PR. (I think the empty labels map is lost in marshalling/unmarshaling due to omitempty. The test steps are listed in the PR description). I did not find any client-side code that makes changes to the retrieved object, so avoided adding such code (e.g. in client/service_inspect.go). Is fixing the nil labels map on the client-side the preferred approach? (in that case, I can update the PR accordingly)

[aaronlehmann]

Thanks for confirming that this makes a difference.

I honestly don't know the right solution. It doesn't feel intuitively right to me to force the field to be marshalled in all cases so that the client avoids outputting null for a specific query. My sense is that it would be better for the client to make sure the map is initialized after unmarshalling, but I'm not sure how we handle these cases in general. I'll step back and let other maintainers offer opinions.

[aaronlehmann]

This should not just be done for Service. There are other other objects that have Annotations and they need to be consistent.

[adshmh]

@aaronlehmann Thank you for the review. I have updated the PR to include all object types using Annotations.

[LK4D4]

I think we had not-nil slices and maps all around the place already.
LGTM