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 documentation specifically for OpenShift Virtualization users #93

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Barakmor1
Copy link
Member

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Release note:

NONE

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Sep 17, 2024
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from barakmor1. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

Copy link

@enp0s3 enp0s3 left a comment

Choose a reason for hiding this comment

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

@Barakmor1 Hi, great detailed work! I have some questions below

* `vmiCalcConfigName` - Defines how resource counting is managed for VM-associated pods. The available options are:
* `VmiPodUsage` - Counts compute resources for VM-associated pods in the same way as native resource quotas, while excluding migration-related resources.
* `VirtualResources` - Counts compute resources based on the VM's specifications, using the VM's RAM size for memory and CPU threads for processing.
* `DedicatedVirtualResources` (default) - Similar to VirtualResources, but separates resource tracking for VM-associated pods by adding a `/vmi` suffix to CPU and memory resource names (e.g., `requests.cpu/vmi`, `limits.memory/vmi`), distinguishing them from other pods.
Copy link

Choose a reason for hiding this comment

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

To me its not clear what is the added value of having the DedicatedVirtualResources option

Copy link
Member Author

Choose a reason for hiding this comment

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

The DedicatedVirtualResources option is helpful for users who want to run both pods and virtual machines in the same namespace. This option allows you to set separate resource limits for VMs (ram and virtual cpu) and pods (native resources), making it easier to manage their respective quotas.

The VirtualResources option is useful when you only have VMs in the namespace and prefer to use the standard resource names (requests.cpu, limits.memory) for quota management, without the need to differentiate between pods and VMs.

docs/cnv-configuration.md Outdated Show resolved Hide resolved
Comment on lines 112 to 113
non-schedulable resources are supported by both AAQ quota and AAQ cluster
quota by managing a k8s resource quota behind the scenes.
Copy link

Choose a reason for hiding this comment

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

Does it mean that AAQ opeator creates ResourceQuota and ClusterResourceQouta?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the AAQ operator creates ResourceQuota and ClusterResourceQuota to enforce limits on non-schedulable resources like PVCs or ClusterRoles. Since these can’t be managed with scheduling gates, AAQ uses native Kubernetes quotas as an alternative. This way, users don’t need to manually create separate native quotas for non-schedulable resources and AAQ quotas for pods.


For non-compute resources, the quota acts like the standard ResourceQuota.

***Warning***: Pods that set `.spec.nodeName` are not allowed to use in any namespace where AAQ Operates.
Copy link

Choose a reason for hiding this comment

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

Does it mean that pod creation will be rejected at admission stage?

Copy link
Member Author

Choose a reason for hiding this comment

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

docs/cnv-configuration.md Outdated Show resolved Hide resolved
Copy link
Contributor

@iholder101 iholder101 left a comment

Choose a reason for hiding this comment

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

@Barakmor1 Looks good! See comments below

docs/cnv-configuration.md Outdated Show resolved Hide resolved
docs/cnv-configuration.md Outdated Show resolved Hide resolved
docs/cnv-configuration.md Outdated Show resolved Hide resolved
docs/cnv-configuration.md Outdated Show resolved Hide resolved
docs/cnv-configuration.md Outdated Show resolved Hide resolved
Comment on lines 52 to 55
requests.memory: 1Gi
limits.memory: 1Gi
requests.cpu/vmi: "1"
requests.memory/vmi: 1Gi
Copy link
Contributor

Choose a reason for hiding this comment

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

In most situations, wouldn't we set requests.memory above requests.memory/vmi? i.e. wouldn't it be more realistic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Please remember that requests.memory/vmi and requests.memory are counted separately. requests.memory is for pods not associated with virtual machines, while requests.memory/vmi is only for pods associated with virtual machines, where we count the VM's RAM.

So, when we set requests.memory: 1Gi and requests.memory/vmi: 1Gi, it means that pods not associated with virtual machines can use up to 1Gi, and relevant VMs can use up to 1Gi of RAM.

I hope this answers your question.

docs/cnv-configuration.md Outdated Show resolved Hide resolved
docs/cnv-configuration.md Outdated Show resolved Hide resolved

The primary purpose of these managed quotas is to set unified quotas for bot native and extended
accounting resources, eliminating the need to manage both ResourceQuota and AAQ quota, or
ClusterResourceQuota and AAQ cluster quota separately.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you can briefly mention that the aware quota is also better because it's not admission based?

docs/cnv-configuration.md Outdated Show resolved Hide resolved
Signed-off-by: bmordeha <bmordeha@redhat.com>
@Barakmor1 Barakmor1 force-pushed the cnvdoc branch 2 times, most recently from c485531 to 3fa2ef8 Compare October 8, 2024 11:41
Add information about the events created by AAQ

Signed-off-by: bmordeha <bmordeha@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. release-note-none Denotes a PR that doesn't merit a release note. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants