Use GlobalMemoryStatusEx to get total physical memory on Windows node

[JiangtianLi]

What this PR does / why we need it:
This PR fixes issue #57110 due to failure in getting total physical memory on some Windows VM such as in VMWare Fusion or Virtualbox. This change uses GlobalMemoryStatusEx instead of GetPhysicallyInstalledSystemMemory to retrieve total physical memory on Windows node. The amount obtained this way is also closer in parity with reading MemTotal from /proc/meminfo on Linux node.
(thanks to @martinivanov and @marono for the help)

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #57110

Special notes for your reviewer:

Release note:

Use a more reliable way to get total physical memory on windows nodes

[dims]

/release-note-none
/ok-to-test

[JiangtianLi]

@marono @trondhindenes Can you help to do sanity check to run kubelet? I don't have VMware fusion/Mac and VirtualBox/Linux handy. I've verified with HyperV/Windows.

[marono]

@JiangtianLi I can do VMware Fusion tomorrow

[marono]

@JiangtianLi delighted to confirm kubelet.exe starts on VMWare Fusion (MAC) running Windows Server 1709, with your changes ...

Thanks and well done!

[JiangtianLi]

@marono Thanks so much for the help!

[alinbalutoiu]

This was previously also failing on Windows Server RTM. Confirmed that it works on Windows Server RTM and 1709 now on VMware Workstation. The change looks good to me, good job!

I am still thinking if kubelet should fail with an error if it couldn't read the memory. On the Linux node from what I see, they ignore the error if it couldn't read the memory. https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/dockershim/cm/container_manager_linux.go#L104-L107
It will always return nil as error (https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/dockershim/cm/container_manager_linux.go#L122)

Should the error be ignored if kubelet couldn't fetch the memory?

[JiangtianLi]

@alinbalutoiu Thanks for the confirmation and good suggestion! As to handling reading memory capacity failure, what I read from code is that reading memory capacity on Linux calls cadvisor: https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/cadvisor/cadvisor_linux.go#L117

cadvisor gets machine info in https://github.com/google/cadvisor/blob/master/manager/manager.go#L220 and https://github.com/google/cadvisor/blob/master/machine/info.go#L107

And if there is an error in https://github.com/google/cadvisor/blob/master/machine/machine.go#L86-L97, the error is surfaced up and not ignored.

What you pointed out seems to be ignoring reading memory capacity error in setting memory limit when creating cgroup manager.

Please feel free to correct me and let me know your thoughts.

[JiangtianLi]

Kindly ping @dashpole @tallclair

[dashpole]

cc @kubernetes/sig-windows-bugs
We could use someone with some context on windows nodes to review and lgtm

[michmike]

@bsteciuk can you please review?

[michmike]

@alinbalutoiu as well

[alinbalutoiu]

Thanks @JiangtianLi for the clarifications.

The changes looks good to me.

[dashpole]

/lgtm

[JiangtianLi]

/test pull-kubernetes-cross

[JiangtianLi]

I restarted the tests and it is green (I think there was flakiness in cross test before and it is skipped this time)
/cc @michmike

[JiangtianLi]

/retest

[JiangtianLi]

/cc @dashpole @kubernetes/sig-windows-bugs @michmike again. Need approve after rebase.

[k8s-ci-robot]

@JiangtianLi: Reiterating the mentions to trigger a notification:
@kubernetes/sig-windows-bugs

In response to this:

/cc @dashpole @kubernetes/sig-windows-bugs @michmike again. Need approve after rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[JiangtianLi]

/assign @tallclair

[michmike]

/lgtm

[michmike]

/assign @tallclair

[aserdean]

Acked-by: Alin Gabriel Serdean [email protected]

I tested the changes using VMware Workstation 12.

/lgtm
/ping @tallclair

[k8s-ci-robot]

@aserdean: changing LGTM is restricted to assignees, and only kubernetes org members may be assigned issues.

In response to this:

Acked-by: Alin Gabriel Serdean [email protected]

I tested the changes using VMware Workstation 12.

/lgtm
/ping @tallclair

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[michmike]

@tallclair can you please look into this and approve

[tallclair]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aserdean, dashpole, JiangtianLi, michmike, tallclair

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

• pkg/kubelet/OWNERS [tallclair]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[fejta-bot]

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

[k8s-github-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[k8s-github-robot]

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

[feiskyer]

1.9 cherry-pick PR created at #59455