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

Swap support #1584

Closed
kannon92 opened this issue Feb 12, 2024 · 7 comments · Fixed by #1585
Closed

Swap support #1584

kannon92 opened this issue Feb 12, 2024 · 7 comments · Fixed by #1585
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@kannon92
Copy link
Contributor

What would you like to be added:
I am working on kubernetes/enhancements#2400 which adds swap support. I think that this project could be a good fit for labeling nodes if they have swap enabled.
Why is this needed:
Swap is an important characteristic of a node and it is usually turned off. We are introducing ways for heterogenous nodes for swap so we should have a label for a swap enabled node.

I think the implementation could just check swapon and verify that swap is enabled on the node.

@kannon92 kannon92 added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 12, 2024
@marquiz
Copy link
Contributor

marquiz commented Feb 12, 2024

Thanks @kannon92 for bringing this up. I think this would be a really good enhancement for NFD. We cannot do swapon as we run NFD essentially in an empty/scratch container. We could read /proc/swaps, we'd need to add that (this file only, not the whole procfs) to the volumes/mounts in deployment files. Do you know any other methods for determining if swap is on (like from sysfs which is already mounted inside the NFD container)?

@kannon92
Copy link
Contributor Author

So I’m happy to contribute!

so as I posted this I realize /proc/swaps is probably the correct path.

I was thinking I could add this to memory in source/memory.

So would I open /proc/swaps and detect if swap is enabled by parsing that file?

And would swap be considered an attribute or a feature?

@marquiz
Copy link
Contributor

marquiz commented Feb 13, 2024

So I’m happy to contribute!

@kannon92 🎉

so as I posted this I realize /proc/swaps is probably the correct path.

Yes, I'm not aware of anything better, either

I was thinking I could add this to memory in source/memory.

👍 I think that would be the logical place for it

So would I open /proc/swaps and detect if swap is enabled by parsing that file?

That would be it

And would swap be considered an attribute or a feature?

I think swap would be an "attribute feature" under the memory source, with one "element" or "property" telling if it's on or not. So we'd have a label like feature.node.kubernetes.io/memory-swap.on=<true/false>, and correspondingly memory.swap.on "hidden feature" for custom rules to use as input.

We could target NFD v0.16 which we've been planning to release before the end of March.
/milestone release-0.16

@k8s-ci-robot
Copy link
Contributor

@marquiz: The provided milestone is not valid for this repository. Milestones in this repository: [v0.16, v0.17]

Use /milestone clear to clear the milestone.

In response to this:

So I’m happy to contribute!

@kannon92 🎉

so as I posted this I realize /proc/swaps is probably the correct path.

Yes, I'm not aware of anything better, either

I was thinking I could add this to memory in source/memory.

👍 I think that would be the logical place for it

So would I open /proc/swaps and detect if swap is enabled by parsing that file?

That would be it

And would swap be considered an attribute or a feature?

I think swap would be an "attribute feature" under the memory source, with one "element" or "property" telling if it's on or not. So we'd have a label like feature.node.kubernetes.io/memory-swap.on=<true/false>, and correspondingly memory.swap.on "hidden feature" for custom rules to use as input.

We could target NFD v0.16 which we've been planning to release before the end of March.
/milestone release-0.16

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.

@marquiz
Copy link
Contributor

marquiz commented Feb 13, 2024

/milestone v0.16

@k8s-ci-robot k8s-ci-robot added this to the v0.16 milestone Feb 13, 2024
@kannon92
Copy link
Contributor Author

we'd need to add that (this file only, not the whole procfs) to the volumes/mounts in deployment files.

Can you point me where you expect this change to be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants