Skip to content

e2e: llc: initial tests for cpu allocation #1308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

e2e: llc: initial tests for cpu allocation #1308

wants to merge 1 commit into from

Conversation

ffromani
Copy link
Contributor

bootstrap the tests which actually run pods and check the CPU (and thus the L3/LLC) allocation.

Start with the trivial sanity tests, adding a good chunk of required infra utilities along the way.

@ffromani
Copy link
Contributor Author

/hold

still tuning the tests

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 11, 2025
@ffromani
Copy link
Contributor Author

/retest

@mrniranjan
Copy link
Contributor

@ffromani i ran the tests on my system and i got this error:

  [FAILED] Unexpected error:
      <*json.UnmarshalTypeError | 0xc00061c230>: 
      json: cannot unmarshal string into Go struct field Info.topology.architecture of type topology.Architecture
      {
          Value: "string",
          Type: <*reflect.rtype | 0x1a4b7e0>{
              t: {Size_: 0x8, PtrBytes: 0x0, Hash: 3381447132, TFlag: 15, Align_: 8, FieldAlign_: 8, Kind_: 2, Equal: 0x408c60, GCData: 0, Str: 235607, PtrToThis: 1884672},
          },
          Offset: 25432,
          Struct: "Info",
          Field: "topology.architecture",
      }
  occurred

From the machineinfo json of my system , i see that topology.Architecture is "numa" string

@ffromani
Copy link
Contributor Author

ffromani commented Apr 7, 2025

@ffromani i ran the tests on my system and i got this error:

  [FAILED] Unexpected error:
      <*json.UnmarshalTypeError | 0xc00061c230>: 
      json: cannot unmarshal string into Go struct field Info.topology.architecture of type topology.Architecture
      {
          Value: "string",
          Type: <*reflect.rtype | 0x1a4b7e0>{
              t: {Size_: 0x8, PtrBytes: 0x0, Hash: 3381447132, TFlag: 15, Align_: 8, FieldAlign_: 8, Kind_: 2, Equal: 0x408c60, GCData: 0, Str: 235607, PtrToThis: 1884672},
          },
          Offset: 25432,
          Struct: "Info",
          Field: "topology.architecture",
      }
  occurred

From the machineinfo json of my system , i see that topology.Architecture is "numa" string

how did you run? I can't recall a similar error

@ffromani
Copy link
Contributor Author

ffromani commented Apr 8, 2025

/retest

@ffromani
Copy link
Contributor Author

ffromani commented Apr 8, 2025

@ffromani i ran the tests on my system and i got this error:

  [FAILED] Unexpected error:
      <*json.UnmarshalTypeError | 0xc00061c230>: 
      json: cannot unmarshal string into Go struct field Info.topology.architecture of type topology.Architecture
      {
          Value: "string",
          Type: <*reflect.rtype | 0x1a4b7e0>{
              t: {Size_: 0x8, PtrBytes: 0x0, Hash: 3381447132, TFlag: 15, Align_: 8, FieldAlign_: 8, Kind_: 2, Equal: 0x408c60, GCData: 0, Str: 235607, PtrToThis: 1884672},
          },
          Offset: 25432,
          Struct: "Info",
          Field: "topology.architecture",
      }
  occurred

From the machineinfo json of my system , i see that topology.Architecture is "numa" string

how did you run? I can't recall a similar error

also, the current tests are a working demo about how to fetch and consume machineinfo JSON data

@mrniranjan
Copy link
Contributor

@ffromani i ran the tests on my system and i got this error:

  [FAILED] Unexpected error:
      <*json.UnmarshalTypeError | 0xc00061c230>: 
      json: cannot unmarshal string into Go struct field Info.topology.architecture of type topology.Architecture
      {
          Value: "string",
          Type: <*reflect.rtype | 0x1a4b7e0>{
              t: {Size_: 0x8, PtrBytes: 0x0, Hash: 3381447132, TFlag: 15, Align_: 8, FieldAlign_: 8, Kind_: 2, Equal: 0x408c60, GCData: 0, Str: 235607, PtrToThis: 1884672},
          },
          Offset: 25432,
          Struct: "Info",
          Field: "topology.architecture",
      }
  occurred

From the machineinfo json of my system , i see that topology.Architecture is "numa" string

how did you run? I can't recall a similar error

also, the current tests are a working demo about how to fetch and consume machineinfo JSON data
i ran the tests as below:

export IMAGE_REGISTRY=quay.io/openshift-kni/
export CNF_TESTS_IMAGE=cnf-tests:latest
ginkgo  -v --require-suite 13_llc/

@ffromani
Copy link
Contributor Author

ffromani commented Apr 9, 2025

we need to bump ghw to 0.9.0 at least (jaypipes/ghw#283). I recall some convos and some challenges to update though.

@ffromani
Copy link
Contributor Author

ffromani commented Apr 9, 2025

we need to bump ghw to 0.9.0 at least (jaypipes/ghw#283). I recall some convos and some challenges to update though.

rebasing against the tip of the main branch (where we moved to ghw 0.15) should suffice

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Apr 9, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ffromani

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 9, 2025
@mrniranjan
Copy link
Contributor

@ffromani llc tests are failing in updating-profile lane because the cnf-tests image that it's using doesn't have machineinfo , currently only the latest cnf-tests image from quay has the required binaries

@ffromani
Copy link
Contributor Author

/test okd-scos-e2e-aws-ovn

@ffromani
Copy link
Contributor Author

/test e2e-aws-ovn-techpreview

@ffromani
e2e: llc: initial tests for cpu allocation
ae03972 
bootstrap the tests which actually run pods and check
the CPU (and thus the L3/LLC) allocation.

Start with the trivial sanity tests, adding a good chunk of
required infra utilities along the way.

Signed-off-by: Francesco Romani <[email protected]>
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Apr 11, 2025

@ffromani: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-hypershift-pao ae03972 link true /test e2e-hypershift-pao
ci/prow/e2e-hypershift ae03972 link true /test e2e-hypershift
ci/prow/okd-scos-e2e-aws-ovn ae03972 link false /test okd-scos-e2e-aws-ovn

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

mrniranjan
mrniranjan reviewed Apr 24, 2025
View reviewed changes
test/e2e/performanceprofile/functests/13_llc/llc.go
deleteTestPod(ctx, testPod)
})

It("should align containers which request less than a L3 group size exclusive CPUs", func(ctx context.Context) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add test case id 77727

mrniranjan
mrniranjan reviewed Apr 24, 2025
View reviewed changes
test/e2e/performanceprofile/functests/13_llc/llc.go
Expect(ok).To(BeTrue(), "pod has not L3-aligned CPUs") // TODO log what?
})

It("cannot align containers which request more than a L3 group size exclusive CPUs", func(ctx context.Context) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you test_id: 81669

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrniranjan mrniranjan mrniranjan left review comments

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ffromani @mrniranjan