Issue fix: get wrong machine info on Arm64 guest by lubinszARM · Pull Request #2456 · google/cadvisor

lubinszARM · 2020-04-01T09:31:30Z

The latest kubernetes deployment on Arm64 VM-s(official ubuntu18.04) always fails.
Because k8s always get num_cores=0 from cAdvisor on Arm64 VM-s.

The reason is that, there is no cache info on Arm64 VM-s.
So the process of getMachineInfo was blocked by the step of getCacheInfo
on Arm64 guest.

And the good news is, we can get cache info on Arm64 hosts.

When this patch was merged, I will deliver a patch to update the version
of cAdvisor in kubernetes as soon as possible.

Signed-off-by: bblu bin.lu@arm.com

k8s-ci-robot · 2020-04-01T09:31:39Z

Hi @lubinszARM. Thanks for your PR.

I'm waiting for a google member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

lubinszARM · 2020-04-01T09:33:12Z

/assign @dashpole

lubinszARM · 2020-04-01T09:33:55Z

Hi @dashpole
Can you help to review it?
Thanks.

k8s-ci-robot · 2020-04-01T09:34:51Z

@lubinszARM: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/ok-to-test

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.

dims · 2020-04-01T10:44:49Z

@lubinszARM i'd rather see the full patch ... not sure why we need to merge this change quickly

dashpole · 2020-04-01T16:23:07Z

/ok-to-test

lubinszARM · 2020-04-02T02:31:30Z

i'd rather see the full patch ... not sure why we need to merge this change quickly

Hi,
Currently, our conformance tests were blocked due to this issue.
https://k8s-testgrid.appspot.com/conformance-arm#Periodic%20Arm64%20conformance%20test

Because our conformance tests were deployed in Arm64 VM-s, and the guest kernel (official ubuntu18.04） doesn't show the cache information.

iwankgb · 2020-04-02T14:47:49Z

Why have you decided to ignore errors on all the platforms rather then on ARM only?

dashpole · 2020-04-02T16:03:43Z

+
+		// On some Linux platforms(such as Arm64 guest kernel), cache info may not exist.
+		// So, we should ignore error here.
+		_ = addCacheInfo(sysFs, &node)


Can we klog.Warningf here at least? It will log frequently on Arm, but this is just meant to fix the conformance test temporarily.

lubinszARM · 2020-04-03T02:08:14Z

Why have you decided to ignore errors on all the platforms rather then on ARM only?

Not all ARM platforms, just only some ARM VM-s, such as official ubuntu-18.04 virtual machine.
cAdvisor can work fine on Arm host and some other ARM VM-s, such as Amazon EC2 A1.
Our conformance test on Amazon EC2 A1 was not blocked, please see:
https://k8s-testgrid.appspot.com/conformance-arm#Periodic%20Arm64%20conformance%20test%20on%20AWS

lubinszARM · 2020-04-08T10:00:31Z

Hi @dashpole
Any idea?

dashpole · 2020-04-08T17:21:03Z

I believe this was fixed by #2471

lubinszARM · 2020-04-09T04:49:50Z

I believe this was fixed by #2471

Hi @dashpole ,
I have compiled the code with your patch.
Please see the log:
root@ubuntu-03:/go/src/github.com/google/cadvisor# git log | grep kk-fix-cpus-without-nodes
Merge pull request #2471 from katarzyna-z/kk-fix-cpus-without-nodes

The issue is not gone.

In #2471, if no "/sys/devices/system/node/" was found in Linux, it will call getCPUTopology().
But in getCPUTopology(), it also needs to get cache info. And it will have the same issue.
So I also skip the cache-missing-issue in getCPUTopology.

Please see the machine info as reference:

{"num_cores":0,"num_physical_cores":2,"num_sockets":1,"cpu_frequency_khz":0,"memory_capacity":8329531392,"memory_by_type":{},"nvm":{"memory_mode_capacity":0,"app direct_mode_capacity":0,"avg_power_budget":0},"hugepages":[{"page_size":1048576,"num_pages":0},{"page_size":2048,"num_pages":0},{"page_size":32768,"num_pages":0},{"page_size":64,"num_pages":0}],"machine_id":"af15b5a662c84009a24a3a245edf00a0","system_uuid":"62f5f880-9aa0-4f6a-b468-7287cc9fd704","boot_id":"174f24ab-23a6-4445-8b88-b1cd812c27bd","filesystems":[{"device":"/run/user/1000","capacity":832950272,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"tmpfs","capacity":832950272,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"/run","capacity":832954368,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"/dev/mapper/ubuntu--01--vg-root","capacity":45747568640,"type":"vfs","inodes":2853424,"has_inodes":true},{"device":"/dev/shm","capacity":4164763648,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"/run/lock","capacity":5242880,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"/sys/fs/cgroup","capacity":4164763648,"type":"vfs","inodes":1016788,"has_inodes":true}],"disk_map":{"253:0":{"name":"dm-0","major":253,"minor":0,"size":46749712384,"scheduler":"none"},"253:1":{"name":"dm-1","major":253,"minor":1,"size":1027604480,"scheduler":"none"},"8:0":{"name":"sda","major":8,"minor":0,"size":48318382080,"scheduler":"mq-deadline"}},"network_devices":[{"name":"enp1s0","mac_address":"52:54:00:e1:49:c6","speed":-1,"mtu":1500}],"topology":null,"cloud_provider":"Unknown","instance_type":"Unknown","instance_id":"None"}

After applying with my patch, the machinInfo will be normal:

{"num_cores":2,"num_physical_cores":2,"num_sockets":1,"cpu_frequency_khz":0,"memory_capacity":8329531392,"memory_by_type":{},"nvm":{"memory_mode_capacity":0,"app direct_mode_capacity":0,"avg_power_budget":0},"hugepages":[{"page_size":1048576,"num_pages":0},{"page_size":2048,"num_pages":0},{"page_size":32768,"num_pages":0},{"page_size":64,"num_pages":0}],"machine_id":"af15b5a662c84009a24a3a245edf00a0","system_uuid":"62f5f880-9aa0-4f6a-b468-7287cc9fd704","boot_id":"174f24ab-23a6-4445-8b88-b1cd812c27bd","filesystems":[{"device":"/dev/shm","capacity":4164763648,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"/run/lock","capacity":5242880,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"/sys/fs/cgroup","capacity":4164763648,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"/run/user/1000","capacity":832950272,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"tmpfs","capacity":832950272,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"/run","capacity":832954368,"type":"vfs","inodes":1016788,"has_inodes":true},{"device":"/dev/mapper/ubuntu--01--vg-root","capacity":45747568640,"type":"vfs","inodes":2853424,"has_inodes":true}],"disk_map":{"253:0":{"name":"dm-0","major":253,"minor":0,"size":46749712384,"scheduler":"none"},"253:1":{"name":"dm-1","major":253,"minor":1,"size":1027604480,"scheduler":"none"},"8:0":{"name":"sda","major":8,"minor":0,"size":48318382080,"scheduler":"mq-deadline"}},"network_devices":[{"name":"enp1s0","mac_address":"52:54:00:e1:49:c6","speed":-1,"mtu":1500}],"topology":[{"node_id":0,"memory":8329531392,"hugepages":[{"page_size":1048576,"num_pages":0},{"page_size":2048,"num_pages":0},{"page_size":32768,"num_pages":0},{"page_size":64,"num_pages":0}],"cores":[{"core_id":0,"thread_ids":[0],"caches":null},{"core_id":1,"thread_ids":[1],"caches":null}],"caches":null}],"cloud_provider":"Unknown","instance_type":"Unknown","instance_id":"None"}

lubinszARM · 2020-04-09T04:50:34Z

I also added a test case of 'TestGetNodesInfoWithoutCacheInfo' for this issue.

The latest kubernetes deployment on Arm64 VM-s always fails. Because k8s always get num_cores=0 from cAdvisor on Arm64 VM-s. The reason is that, there is no cache info on Arm64 VM-s. And the good news is that, we can get cache info on Arm64 hosts. When this patch was merged, I will deliver a patch to update the version of cAdvisor in kubernetes as soon as possible. Signed-off-by: bblu <bin.lu@arm.com>

dashpole

lgtm. Thanks for the unit test!

dashpole · 2020-04-09T16:03:47Z

cc @katarzyna-z
I'm going to go ahead with this mitigation for now to unblock the kubernetes conformance tests. This includes a unit test to verify we don't break them again.

lubinszARM · 2020-04-10T03:26:39Z

Hi @dims
Can we update the version of cAdvisor in k8s side?
Thanks.

dims · 2020-04-10T10:42:50Z

@lubinszARM go for it

lubinszARM · 2020-04-13T07:47:01Z

Hi @dashpole
Can you add a new tag for cadvisor, such as v0.36.1?
So that, I can use this new tag in kubernetes.

Thanks.

dims · 2020-04-13T10:18:41Z

@lubinszARM there's a few more things that need to get in. we typically do this just before code freeze of kubernetes to do a final sync.

alexellis · 2020-05-20T08:14:58Z

 		err = addCacheInfo(sysFs, &node)
 		if err != nil {
-			return nil, 0, err
+			klog.Warningf("Found node without cache information, nodeDir: %s", nodeDir)


Will this appear in the syslog / journal by default? If so, that may be a concern and I would encourage using a lower log level, or a flag to suppress it.

I came here via an issue on the k3s repo after having a log fill up with this message. Turns out, this code gets hit a fair bit.

@alexellis
Unfortunately, this is still in the syslog.
I will resubmit a PR to solve your troubles

k8s-ci-robot added the needs-ok-to-test label Apr 1, 2020

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Apr 1, 2020

lubinszARM force-pushed the pr_cacheinfo branch 2 times, most recently from 0256340 to 4fcdf9a Compare April 2, 2020 05:01

dashpole reviewed Apr 2, 2020

View reviewed changes

lubinszARM force-pushed the pr_cacheinfo branch from 4fcdf9a to 0bcf452 Compare April 3, 2020 02:20

lubinszARM force-pushed the pr_cacheinfo branch 2 times, most recently from 3eea895 to bdf54ad Compare April 9, 2020 04:42

lubinszARM force-pushed the pr_cacheinfo branch 2 times, most recently from d81b408 to 63b5186 Compare April 9, 2020 06:00

lubinszARM force-pushed the pr_cacheinfo branch from 63b5186 to 7f1ea45 Compare April 9, 2020 06:02

dashpole approved these changes Apr 9, 2020

View reviewed changes

dashpole merged commit e35a0d5 into google:master Apr 9, 2020

This was referenced Apr 20, 2020

the tag of cadvisor in kubernetes #2500

Closed

Conformance image v1.18.0 fails on arm64 kubernetes/kubernetes#89554

Closed

lubinsz mentioned this pull request May 20, 2020

machine.go reaching code on aarch64 it shouldn't #2229

Closed

alexellis reviewed May 20, 2020

View reviewed changes

Conversation

lubinszARM commented Apr 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Apr 1, 2020

Uh oh!

lubinszARM commented Apr 1, 2020

Uh oh!

lubinszARM commented Apr 1, 2020

Uh oh!

k8s-ci-robot commented Apr 1, 2020

Uh oh!

dims commented Apr 1, 2020

Uh oh!

dashpole commented Apr 1, 2020

Uh oh!

lubinszARM commented Apr 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iwankgb commented Apr 2, 2020

Uh oh!

dashpole Apr 2, 2020

Choose a reason for hiding this comment

Uh oh!

lubinszARM Apr 3, 2020

Choose a reason for hiding this comment

Uh oh!

lubinszARM commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lubinszARM commented Apr 8, 2020

Uh oh!

dashpole commented Apr 8, 2020

Uh oh!

lubinszARM commented Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lubinszARM commented Apr 9, 2020

Uh oh!

dashpole left a comment

Choose a reason for hiding this comment

Uh oh!

dashpole commented Apr 9, 2020

Uh oh!

lubinszARM commented Apr 10, 2020

Uh oh!

dims commented Apr 10, 2020

Uh oh!

lubinszARM commented Apr 13, 2020

Uh oh!

dims commented Apr 13, 2020

Uh oh!

alexellis May 20, 2020

Choose a reason for hiding this comment

Uh oh!

lubinsz May 20, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

lubinszARM commented Apr 1, 2020 •

edited

Loading

lubinszARM commented Apr 2, 2020 •

edited

Loading

lubinszARM commented Apr 3, 2020 •

edited

Loading

lubinszARM commented Apr 9, 2020 •

edited

Loading