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

feat: add CPU/memory available capacity reporting in the InternalMemberCluster controller #815

Merged
merged 3 commits into from
May 14, 2024

Conversation

michaelawyu
Copy link
Contributor

Description of your changes

This PR adds the ability to report CPU/memory available capacity in the InternalMemberCluster controller.

This change reflects a decision earlier to make the available CPU/memory capacity a core Fleet property.

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

  • Integration tests

Special notes for your reviewer

Refactoring of the InternalMemberCluster controller tests is in progress and will be submitted separately.


// Do a sanity check to avoid inconsistencies.
if availableCPU.Cmp(resource.Quantity{}) < 0 {
availableCPU = resource.Quantity{}
Copy link
Contributor

Choose a reason for hiding this comment

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

when will this happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Ryan! The node list and the pod list does not happen at the same time, so it could happen that the available amount becomes negative.

@@ -138,6 +138,7 @@ var _ = Describe("Test Internal Member Cluster Controller", Serial, func() {
By("checking updated member cluster usage")
Expect(imc.Status.ResourceUsage.Allocatable).ShouldNot(BeNil())
Expect(imc.Status.ResourceUsage.Capacity).ShouldNot(BeNil())
Expect(imc.Status.ResourceUsage.Available).ShouldNot(BeNil())
Copy link
Contributor

Choose a reason for hiding this comment

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

check that the value is not zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Ryan! Actually a PR that refactors the tests on the InternalMemberCluster is in progress; would it be OK if this gets addressed in a separate PR? There will be a full check on the total/allocatable/available data.

@michaelawyu michaelawyu merged commit 7a9548d into Azure:main May 14, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants