Thanks for the quick review, @thaJeztah , much appreciated :)

In addition; we should consider having a different way to obtain this info, because the /info endpoint is far from lightweight to call

That endpoint seems to be the one, central, place where we gather info on hosts (and is tied directly to the types.Info struct, that is also the one, central place for this); so I'm not sure where else it would make sense to do this; could you please elaborate a bit on where you'd rather have this? Or are you suggesting we should start breaking up types.Info into multiple structs?

I'm a bit concerned about using this for scheduling as we'll now end up with two vectors to schedule (image manifest / platform as advertised by engine info), which may not provide the same information (e.g. Architecture may also have to be taken into account, as well as isolation (hyper-v / process isolation).

Agreed that there is food for thought here, as there is a lot of info we could use to match images & nodes that we don't really make use of just yet (we pretty much only use the architecture for now); however, that seems to be a discussion more relevant on the upcoming swarmkit change. This patch here is only concerned with providing more info to swarmkit.
We've been discussing that scheduling change with @wsong @anshulpundir and @dhiltgen , notably on moby/swarmkit#2770, and this is the best we've come up with. Happy to brain storm more with with you any time though :)

That endpoint seems to be the one, central, place where we gather info on hosts (and is tied directly to the types.Info struct, that is also the one, central place for this)

Yes, and I think that's a problem, because the endpoint just about touches every subsystem of dockerd (gathering information about containers (and their state), images, which plugins are available, low-level information from the storage-driver). Some of those have caused issues in the past (e.g. due to locks when determining container state).

I think we need a way to get more fine-grained information out of the daemon, especially if this endpoint is gonna be used more.

Agreed that there is food for thought here, as there is a lot of info we could use to match images & nodes that we don't really make use of just yet

I think the key discussion here is (also see #34875) that we need to finish the standardisation of the "platform" information on images, and set the same information on the daemon. I think the last discussion ended with;

• review/define the fields that are included in the platform
• make sure that there's a well-defined format to serialize the platform in a string
• the platform (and string representation) should allow optional fields (e.g. OS version, and OS features may not be present on all platforms)
• when parsing the string, those fields can be ignored by a scheduler (and when matching (multi-arch) images with the daemon/runtime)

Related issues;

• Add support for Windows version filtering on pull #35090 Add support for Windows version filtering on pull
• Support running ARM images without variant information containerd/containerd#2293 Support running ARM images without variant information
• ARM Variant Matching containerd/containerd#2133 ARM Variant Matching
• Discuss: How to handle multi-arch images when platforms support multi-arch runtimes? containerd/containerd#2029 Discuss: How to handle multi-arch images when platforms support multi-arch runtimes?
• (slightly related Utilize os.features for plugin images with linux library requirements containerd/containerd#2594 Utilize os.features for plugin images with linux library requirements)

There was a lengthy discussion on the original PR that added the OS and Architecture; #13921

This patch here is only concerned with providing more info to swarmkit.

Correct; it's probably fine to show this information for informational purposes; in which case the field should carry a big disclaimer that it's meant as-such, and that its content (or format) is not part of the API specification. Similar to what we added to the DriverStatus field;

however (glancing over the discussion on the swarmkit issue), IIUC, the purpose of this change is to influence scheduling decisions (manually or automatically). If the information used here does not match the direction that image-selection will use (the discussion on #34875), there is a risk;

• user specifies constraint to run service on Windows version Foobar.X.Y.Z nodes
• service fails to deploy, because manifest selection uses a different algorithm
• or service fails to deploy because the image is not compatible
• confusion..

Note that for scheduling, another approach would be to set up engine (or node-) labels, and set constraints based on those labels.

@thaJeztah thanks for your very interesting & extensive explanation, I understand much better where you're coming from now.

May I suggest then that while the specs solidify on the image manifest side of things as to how to normalize how to report arch & OS info, we still get this in with a big caveat in the doc pointing out this field is temporary and subject to change without notice? Then when that's normalized I'll come back and change both this and swarmkit accordingly.

I think we need a way to get more fine-grained information out of the daemon, especially if this endpoint is gonna be used more.

Agreed on this, too. We should start doing this as part of the long-term fix too, when it's clear how we want to report host info.

Addressing the rest of your comments ASAP.

Codecov Report

❗ No coverage uploaded for pull request base (master@1d5748d). Click here to learn what that means.
The diff coverage is 63.63%.

@@            Coverage Diff            @@
##             master   #38349   +/-   ##
=========================================
  Coverage          ?   37.06%           
=========================================
  Files             ?      611           
  Lines             ?    45548           
  Branches          ?        0           
=========================================
  Hits              ?    16881           
  Misses            ?    26384           
  Partials          ?     2283

Discussing if we can come up with alternatives for this use-case, but will have to do a bit more thinking about that

Discussing in the maintainers meeting (again 😅 );

• can you add this to the prometheus metrics?
• can you cache the results?

Other than that, we're ok with this change

Great, thanks @thaJeztah ! :)

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "wk8/os_version" git@github.com:wk8/moby.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354462472
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@thaJeztah : amended as requested, could you please have another look? Thanks!

@thaJeztah : amended as requested per @cpuguy83 . Thanks.