Fix incorrect CPU topology on single NUMA and multi socket platform.

[Creatone]

Fixes #2798

Signed-off-by: Paweł Szulik pawel.szulik@intel.com

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[Creatone]

/ok-to-test

[kashifest]

It seems physical package id is missing,
[sysinfo.go:406] Cannot read physical package id for /fakeSysfs/devices/system/node/node0/cpu1, physical_package_id file does not exist, err: file does not exist
are we calling GetCPUPhysicalPackageID at the correct location ?

[Creatone]

It seems physical package id is missing,
[sysinfo.go:406] Cannot read physical package id for /fakeSysfs/devices/system/node/node0/cpu1, physical_package_id file does not exist, err: file does not exist
are we calling GetCPUPhysicalPackageID at the correct location ?

This PR was draft at this time. Now it's ready for review.

[iwankgb]

LGTM

[iwankgb]

I have to un-LGTM it: it may cause yet another set of issues with kubelet (see comments in the linked issue).

[Creatone]

I have to un-LGTM it: it may cause yet another set of issues with kubelet (see comments in the linked issue).

Unfortunately, issues are already there. This PR fixes only one issue, the wrong threads to CPU matching.

[iwankgb]

I know but CPU manager must not be affected - keep in mind that it is enabled for all guaranteed pods by default.

[Insullone]

@Creatone We tested the fix and it can detect cpu topologies correctly. By the way, could you backport the fix to 0.38.x as well?

[Insullone]

Just clarify: when I said "We tested the fix and it can detect cpu topologies correctly", I meant it fixed the specific issue "wrong threads to CPU matching".

[bobbypage]

Thanks @Creatone for the fix. Can you please explain what the issue was / how this fixes it, I'm not super clear? Is the issue due to missing check of core.SocketID == physicalPackageID?

[Creatone]

@bobbypage Imagine you have a platform with one NUMA node and 4 sockets with one CPU in. In this case, every CPU has core_id = 0, so cAdvisor was putting them in the same core in topology.
Ex.

"topology": [
   {
     "node_id": 0,
     "memory": 50628362240,
     "hugepages": [
       {
         "page_size": 1048576,
         "num_pages": 8
       }
     ],
     "cores": [
       {
         "core_id": 0,
         "thread_ids": [
           0,
           1,
           2,
           3,
         ],
         "caches": null,
         "socket_id": 3
       }
     ],
     "caches": null
   }
 ]

And this is wrong. We need to differentiate these CPUs. It's needed to consist of psychical_package_id in our topology discovery. This parameter describes on which socket is CPU.
This is the output that we need:

"topology": [
  {
    "node_id": 0,
    "memory": 50628362240,
    "hugepages": [
      {
        "page_size": 1048576,
        "num_pages": 8
      }
    ],
    "cores": [
      {
        "core_id": 0,
        "thread_ids": [
          0
        ],
        "caches": null,
        "socket_id": 0
      },
      {
        "core_id": 1,
        "thread_ids": [
          1
        ],
        "caches": null,
        "socket_id": 1
      },
      {
        "core_id": 2,
        "thread_ids": [
          2
        ],
        "caches": null,
        "socket_id": 2
      },
      {
        "core_id": 3,
        "thread_ids": [
          3
        ],
        "caches": null,
        "socket_id": 3
      },
    ],
    "caches": null
  }
]

[hanamantagoudvk]

@Creatone @bobbypage : Is the review ok ? If review is done, it needs to be merged.

[bobbypage]

got it, thanks @Creatone for the fix. LGTM.