Expand /info: Expose OSType (GOOS), Architecture (GOARCH) by olleolleolle · Pull Request #13921 · moby/moby

olleolleolle · 2015-06-13T07:43:36Z

This change amends the diagnostic output of /info, to add two more keys. This supports Swarm's effort of helping users target multi-architecture Swarm clusters. See #13634.

These keys were introduced:

OSType (runtime.GOOS)
Architecture (runtime.GOARCH)

✨ to @runcom, who helped out a lot with this.

olleolleolle · 2015-06-13T08:44:09Z

(I asked in IRC for a re-run on janky.)

runcom · 2015-06-13T09:24:01Z

@olleolleolle does swarm need only Platform and the other two fields as well or does it need only os platform? (from what I've read seems like only Platform is needed but I may have missed something)

runcom · 2015-06-13T09:25:49Z

Also platform seems really redunant as it can be composed of GOOS + GOARCH, wdyt?

olleolleolle · 2015-06-13T09:28:16Z

My understanding is: These two would be the minimal for Swarm's current needs:

OperatingSystemType: runtime.GOOS,
Architecture: runtime.GOARCH,

runcom · 2015-06-13T09:29:28Z

@olleolleolle yup and in case the concat would be done elsewhere I suppose (because only swarm may need the underscore between the two), let's see what others have to say about this :)

olleolleolle · 2015-06-13T09:31:21Z

I agree! I will run over the existing commit with the reduced one.

runcom · 2015-06-13T09:32:04Z

ping @vieux @aluzzardi

tiborvass · 2015-06-13T16:40:20Z

I am not a fan of OperatingSystemType :S not sure what would be the best. Platform with GOOS/GOARCH could work I guess. What do others think?

runcom · 2015-06-13T17:59:52Z

Naming apart, just GOOS and GOARCH is OK for me as I've already said. So ppl (swarm) can play with them

estesp · 2015-06-15T17:55:45Z

I think one of the "problems" here is that GOOS is really what we should be calling "Operating System" in current "/info" results. What we return today as operating system has nothing to do with whether the OS is Linux or Windows, but simply whether we can find a nice "PRETTY_NAME" from a common file in Linux distros (which from a recent issue/bug has been found to be missing on some distros), or in the Windows case, the release number/variant of Windows from the registry.

I think GOOS should become OperatingSystem in the info structure, and the pretty name/OS variant parsing result should go in a new field OS Variant/Version or something like that. GOOS is stable and easily parseable and well-defined, whereas the distro format for PRETTY_NAME is free-form and up to how the distro wants to make a value that is (quote) A pretty operating system name in a format suitable for presentation to the user. May or may not contain a release code name or OS version of some kind, as suitable. (see http://www.freedesktop.org/software/systemd/man/os-release.html)

aluzzardi · 2015-06-15T20:38:08Z

I agree with @tiborvass: Not a fan or OperatingSystemType.

I think Platform is pretty standard (arch + OS pair).

tiborvass · 2015-06-15T20:47:47Z

Agree with @estesp too... should we change what OperatingSystem is, now? Or should we just use Platform ?

duglin · 2015-06-17T18:57:55Z

Given I don't think we can change what's there today because it will probably break someone, I would recommend:

deprecate OperatingSystem
add OS and map it to GOOS
add OSVariant and map it to today's OperatingSystem, aka PRETTY_NAME
add Architecture per the current PR, GOARCH

icecrime · 2015-07-07T23:42:44Z

Not fond of the name OSVariant, but LGTM!

calavera · 2015-07-07T23:46:12Z

deprecate OperatingSystem

why don't we version those fields by the api version? that's what we normally do.

runcom · 2015-07-14T20:30:32Z

I agree with @calavera, we could version this and not expose the new fields to old api
Wdyt @icecrime @calavera

abronan · 2015-07-14T20:44:26Z

+1 with @calavera and @runcom. Also not fond of the name OSVariant.

tiborvass · 2015-07-22T23:22:03Z

Ping @olleolleolle please rebase and modify according to #13921 (comment)

Let us know if you don't have time, we can always carry your commit.

thaJeztah · 2015-09-27T19:12:44Z

ping @tiborvass @estesp @duglin PTAL

Thanks so much @olleolleolle, apologies for all the bike shedding ❤️

olleolleolle · 2015-09-27T19:44:45Z

Hi all. Back again, getting ready to introduce the new folder pkg/parser/architecture/ and use that in daemon/info.go.

Detecting runtime architecture

...on Windows. Following the leads given by @tiborvass, I came upon counter-indications.

In the docs for the SYSTEM_INFO struct for Windows, a contributed comment presented the value found there as not-runtime (quite similar to previous discussion):

wProcessorArchitecture documentation is wrong
The documentation for the wProcessorArchitecture is wrong. It is in fact not the "processor architecture of the installed operating system" but rather is the process architecture the app was built for. An x86 process running on x64 will get a value of PROCESSOR_ARCHITECTURE_INTEL.

Also, the SDK defines a number of PROCESSOR_ARCHITECTURE values that aren't present in this documentation.

duglin · 2015-09-27T20:00:42Z

I like the two fields, if we can just get GOARCH to return the right thing.
I think GOOS does it correctly.

olleolleolle · 2015-09-27T20:51:01Z

Added a few incomplete files in order to show the plan.

jessfraz · 2015-10-01T18:05:16Z

+1 to @tiborvass comment in #13921 (comment)

tiborvass · 2015-10-01T18:11:13Z

Collective review:

We all agree with #13921 (comment) and would just add an ommission: the name of the field for the architecture should be Architecture like it is in the code right now.

@olleolleolle if you have better alternatives for how to actually detect Architecture at runtime for those different platforms we're all ears, but if not, we agree on the proposal mentioned above.

Thanks!!

olleolleolle · 2015-10-01T18:13:34Z

Thanks, great, then I can go forward with the simplest possible thing. Glad to hear it.

(Now removed the WIP commit I had created while misunderstanding Tibor's plan outline.)

olleolleolle · 2015-10-01T18:18:31Z

@tiborvass You can remove the GitHub label impact/deprecation.

tiborvass · 2015-10-01T20:53:55Z

@olleolleolle I just edited my proposal above for the windows function. It should be using https://msdn.microsoft.com/en-us/library/windows/desktop/ms724381(v=vs.85).aspx

tiborvass · 2015-10-15T00:45:12Z

@olleolleolle any updates? Would you prefer someone carries this PR?

olleolleolle · 2015-10-15T05:03:42Z

Ah, no updates - and yes please do carry it, anyone.

(Good process, that!)

On 15 Oct 2015, at 02:46, Tibor Vass [email protected] wrote:

@olleolleolle any updates? Would you prefer someone carries this PR?

—
Reply to this email directly or view it on GitHub.

thaJeztah · 2015-10-27T00:20:19Z

Talked with @vdemeester earlier and he is planning on carrying this PR tomorrow. Thanks so much again for being patient with us @olleolleolle

icecrime · 2015-10-27T22:21:45Z

Ping @vdemeester: was that carried?

vdemeester · 2015-10-27T22:22:47Z

@icecrime in the process (writing some stuff on pkg/architecture taking account @tiborvass comments). PR to come tomorrow I think 😉

icecrime · 2015-10-27T22:46:13Z

@vdemeester 🎉

vdemeester · 2015-10-30T23:20:15Z

Closing this as carrying in #17478.

It was not immediately clear why we were not using runtime.GOARCH for these (with a conversion to other formats, such as x86_64). These docs are based on comments that were posted when implementing this package; - moby#13921 (comment) - moby#13921 (comment) Some links were now redirecting to a new location, so updated them to not depend on the redirect. While at it, also updated a call to logrus to use structured formatting (WithError()). Signed-off-by: Sebastiaan van Stijn <[email protected]>

GordonTheTurtle added the status/0-triage label Jun 13, 2015

olleolleolle force-pushed the feature/host-arch-output-13634 branch 2 times, most recently from 199088b to ccf408f Compare June 13, 2015 08:07

olleolleolle changed the title ~~WIP /info: Architecture, OperatingSystemType, Platform~~ Expand /info: OperatingSystemType, Architecture, Platform Jun 13, 2015

runcom added status/2-code-review and removed status/0-triage labels Jun 13, 2015

olleolleolle force-pushed the feature/host-arch-output-13634 branch from ccf408f to 24f6d19 Compare June 13, 2015 09:33

olleolleolle changed the title ~~Expand /info: OperatingSystemType, Architecture, Platform~~ Expand /info: OperatingSystemType, Architecture Jun 13, 2015

olleolleolle force-pushed the feature/host-arch-output-13634 branch 2 times, most recently from 97d727f to adb626c Compare June 17, 2015 21:07

olleolleolle force-pushed the feature/host-arch-output-13634 branch from adb626c to c89ae46 Compare July 23, 2015 21:07

olleolleolle changed the title ~~Expand /info: OperatingSystemType, Architecture~~ Expand /info: Expose GOOS, GOARCH as OS, Architecture Jul 23, 2015

olleolleolle changed the title ~~Expand /info: Expose OSType (GOOS), Architecture (run-time architecture)~~ WIP: Expand /info: Expose OSType (GOOS), Architecture (run-time architecture) Sep 27, 2015

tiborvass added status/2-code-review and removed status/1-design-review status/needs-attention Calls for a collective discussion during a review session labels Oct 1, 2015

olleolleolle force-pushed the feature/host-arch-output-13634 branch from 431fa60 to 96184fc Compare October 1, 2015 18:15

olleolleolle changed the title ~~WIP: Expand /info: Expose OSType (GOOS), Architecture (run-time architecture)~~ Expand /info: Expose OSType (GOOS), Architecture (run-time architecture) Oct 1, 2015

olleolleolle changed the title ~~Expand /info: Expose OSType (GOOS), Architecture (run-time architecture)~~ Expand /info: Expose OSType (GOOS), Architecture (GOARCH) Oct 1, 2015

vdemeester removed the impact/deprecation label Oct 1, 2015

vdemeester mentioned this pull request Oct 29, 2015

Carry#13921 : Expand /info: Expose OSType (GOOS), Architecture (GOARCH) #17478

Merged

vdemeester closed this Oct 30, 2015

olleolleolle deleted the feature/host-arch-output-13634 branch September 27, 2017 15:57

thaJeztah mentioned this pull request Dec 12, 2018

Adding OS version info to nodes' Info struct and to the system info's API #38349

Merged

thaJeztah mentioned this pull request Apr 8, 2023

pkg/platform: cleanup, and deprecate OSType #45298

Merged

Conversation

olleolleolle commented Jun 13, 2015

Uh oh!

olleolleolle commented Jun 13, 2015

Uh oh!

runcom commented Jun 13, 2015

Uh oh!

runcom commented Jun 13, 2015

Uh oh!

olleolleolle commented Jun 13, 2015

Uh oh!

runcom commented Jun 13, 2015

Uh oh!

olleolleolle commented Jun 13, 2015

Uh oh!

runcom commented Jun 13, 2015

Uh oh!

tiborvass commented Jun 13, 2015

Uh oh!

runcom commented Jun 13, 2015

Uh oh!

estesp commented Jun 15, 2015

Uh oh!

aluzzardi commented Jun 15, 2015

Uh oh!

tiborvass commented Jun 15, 2015

Uh oh!

duglin commented Jun 17, 2015

Uh oh!

icecrime commented Jul 7, 2015

Uh oh!

calavera commented Jul 7, 2015

Uh oh!

runcom commented Jul 14, 2015

Uh oh!

abronan commented Jul 14, 2015

Uh oh!

tiborvass commented Jul 22, 2015

Uh oh!

thaJeztah commented Sep 27, 2015

Uh oh!

olleolleolle commented Sep 27, 2015

Detecting runtime architecture

Uh oh!

duglin commented Sep 27, 2015

Uh oh!

olleolleolle commented Sep 27, 2015

Uh oh!

jessfraz commented Oct 1, 2015

Uh oh!

tiborvass commented Oct 1, 2015

Uh oh!

olleolleolle commented Oct 1, 2015

Uh oh!

olleolleolle commented Oct 1, 2015

Uh oh!

tiborvass commented Oct 1, 2015

Uh oh!

tiborvass commented Oct 15, 2015

Uh oh!

olleolleolle commented Oct 15, 2015

Uh oh!

thaJeztah commented Oct 27, 2015

Uh oh!

icecrime commented Oct 27, 2015

Uh oh!

vdemeester commented Oct 27, 2015

Uh oh!

icecrime commented Oct 27, 2015

Uh oh!

vdemeester commented Oct 30, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants