Skip to content
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.

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

Add s390/s390x CPU topology #3516

Merged
merged 2 commits into from
Jun 27, 2024
Merged

Add s390/s390x CPU topology #3516

merged 2 commits into from
Jun 27, 2024

Conversation

madeelibm
Copy link
Contributor

s390x CPU topology is missing in cadvisor which made kubelet service returns error when enabling CPU Manager. This patch adds missing CPU topology with which CPU Manager can also be enabled on s390x platform.
Please for review and if there are concerns let us know.

@k8s-ci-robot
Copy link
Collaborator

Hi @madeelibm. 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.

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.

@madeelibm
Copy link
Contributor Author

madeelibm commented Apr 24, 2024

@bobbypage We are adding support for s390x again with the PR: #3521

@madeelibm
Copy link
Contributor Author

Can someone help to merge this PR?

utils/sysfs/sysfs.go Outdated Show resolved Hide resolved
utils/sysfs/sysfs.go Outdated Show resolved Hide resolved
@madeelibm madeelibm force-pushed the s390x-topology branch 2 times, most recently from 96992df to 7872211 Compare May 29, 2024 10:55
@haircommander
Copy link
Contributor

/ok-to-test

@k8s-ci-robot
Copy link
Collaborator

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

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-sigs/prow repository.

@mrunalp
Copy link
Collaborator

mrunalp commented May 29, 2024

/ok-to-test

@haircommander
Copy link
Contributor

you have some test failures that need fixing @madeelibm

in the meantime @iwankgb @bobbypage PTAL

Copy link
Collaborator

@iwankgb iwankgb left a comment

Choose a reason for hiding this comment

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

LGTM besides returning err instead of explicit nil in two places.

@madeelibm
Copy link
Contributor Author

/retest

1 similar comment
@madeelibm
Copy link
Contributor Author

/retest

@madeelibm madeelibm force-pushed the s390x-topology branch 2 times, most recently from 428a005 to 1929598 Compare June 12, 2024 07:08
@madeelibm
Copy link
Contributor Author

/retest

@madeelibm
Copy link
Contributor Author

All tests are passing now on x86 and Z.

@haircommander
Copy link
Contributor

@iwankgb PTAL

machine/topology_test.go Outdated Show resolved Hide resolved
@madeelibm madeelibm force-pushed the s390x-topology branch 2 times, most recently from ff38143 to 3eb1be1 Compare June 13, 2024 08:34
@iwankgb iwankgb mentioned this pull request Jun 15, 2024
@iwankgb
Copy link
Collaborator

iwankgb commented Jun 22, 2024

@madeelibm, can you merge master, please?

@iwankgb
Copy link
Collaborator

iwankgb commented Jun 27, 2024

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants