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.

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

salt: add allocated resources for OS and kubelet #4134

Merged
merged 1 commit into from
Sep 13, 2023
Merged

salt: add allocated resources for OS and kubelet #4134

merged 1 commit into from
Sep 13, 2023

Conversation

ezekiel-alexrod
Copy link
Contributor

Component:

salt

Context:

To avoid exhausting memory and CPU resources on the server, we want to set a minimum amount of memory and CPU for the operating system and kubelet.

We will follow this document Allocatable resources and especially Google recommandation as they are the state of art of kubernetes.

We will add some “memory and CPU” because we are using salt and mdadm and they are vital for the cluster.

Summary:

I allow myself to add this link as it is the
Official documentation of Google

Added kubeReserved and systemReserved to KubeletConfiguration following the Google's recommandations.

@ezekiel-alexrod ezekiel-alexrod requested a review from a team as a code owner August 30, 2023 17:09
@bert-e

This comment was marked as outdated.

Sign in to view
@bert-e

This comment was marked as outdated.

Sign in to view
@bert-e

This comment was marked as outdated.

Sign in to view
@ezekiel-alexrod ezekiel-alexrod force-pushed the improvement/minimum-memory-cpu-for-os-kubelet branch from cbf1852 to bd627d7 Compare September 6, 2023 14:43
@bert-e

This comment was marked as outdated.

Sign in to view
@ezekiel-alexrod ezekiel-alexrod force-pushed the improvement/minimum-memory-cpu-for-os-kubelet branch from bd627d7 to 43b08b8 Compare September 7, 2023 15:52
@bert-e

This comment was marked as outdated.

Sign in to view
TeddyAndrieux
TeddyAndrieux requested changes Sep 8, 2023
View reviewed changes
Copy link
Collaborator

@TeddyAndrieux TeddyAndrieux left a comment

Choose a reason for hiding this comment

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

If you may also add a changelog entry about it

salt/metalk8s/kubernetes/kubelet/standalone.sls Outdated Show resolved Hide resolved
salt/metalk8s/kubernetes/kubelet/standalone.sls Outdated Show resolved Hide resolved
salt/metalk8s/kubernetes/kubelet/standalone.sls Outdated Show resolved Hide resolved
@bert-e

This comment was marked as outdated.

Sign in to view
@ezekiel-alexrod ezekiel-alexrod force-pushed the improvement/minimum-memory-cpu-for-os-kubelet branch from 43b08b8 to 662e89a Compare September 12, 2023 08:32
@bert-e

This comment was marked as outdated.

Sign in to view
TeddyAndrieux
TeddyAndrieux requested changes Sep 12, 2023
View reviewed changes
salt/_modules/metalk8s_os.py Outdated Show resolved Hide resolved
salt/_modules/metalk8s_os.py Outdated Show resolved Hide resolved
salt/metalk8s/kubernetes/kubelet/standalone.sls Outdated Show resolved Hide resolved
salt/_modules/metalk8s_os.py Outdated Show resolved Hide resolved
salt/_modules/metalk8s_os.py Outdated Show resolved Hide resolved
salt/tests/unit/formulas/fixtures/salt.py Show resolved Hide resolved
salt/tests/unit/formulas/fixtures/salt.py Show resolved Hide resolved
@ezekiel-alexrod ezekiel-alexrod force-pushed the improvement/minimum-memory-cpu-for-os-kubelet branch 4 times, most recently from 4249609 to 9a88f78 Compare September 13, 2023 07:14
@bert-e
Copy link
Contributor

bert-e commented Sep 13, 2023

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

TeddyAndrieux
TeddyAndrieux approved these changes Sep 13, 2023
View reviewed changes
Copy link
Collaborator

@TeddyAndrieux TeddyAndrieux left a comment

Choose a reason for hiding this comment

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

LGTM good job 👍

salt/tests/unit/modules/files/test_metalk8s_os.yaml
result:
cpu: 1350m
memory: 27894Mi
# 10 - 3 cores and 14.2 GB of RAM
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice 😄

@bert-e
Copy link
Contributor

bert-e commented Sep 13, 2023

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@ezekiel-alexrod
Copy link
Contributor Author

/approve

@ezekiel-alexrod ezekiel-alexrod force-pushed the improvement/minimum-memory-cpu-for-os-kubelet branch from 9a88f78 to 698e79e Compare September 13, 2023 08:20
@bert-e
Copy link
Contributor

bert-e commented Sep 13, 2023

Build failed

The build for commit did not succeed in branch improvement/minimum-memory-cpu-for-os-kubelet.

The following options are set: approve

@bert-e
Copy link
Contributor

bert-e commented Sep 13, 2023

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

  • ✔️ development/126.0

The following branches will NOT be impacted:

  • development/123.0
  • development/124.0
  • development/124.1
  • development/125.0
  • development/2.0
  • development/2.1
  • development/2.10
  • development/2.11
  • development/2.2
  • development/2.3
  • development/2.4
  • development/2.5
  • development/2.6
  • development/2.7
  • development/2.8
  • development/2.9

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

  • Any commit you add on the source branch will trigger a new cycle after the
    current queue is merged.
  • Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

@bert-e
Copy link
Contributor

bert-e commented Sep 13, 2023

I have successfully merged the changeset of this pull request
into targetted development branches:

  • ✔️ development/126.0

The following branches have NOT changed:

  • development/123.0
  • development/124.0
  • development/124.1
  • development/125.0
  • development/2.0
  • development/2.1
  • development/2.10
  • development/2.11
  • development/2.2
  • development/2.3
  • development/2.4
  • development/2.5
  • development/2.6
  • development/2.7
  • development/2.8
  • development/2.9

Please check the status of the associated issue None.

Goodbye ezekiel-alexrod.

@bert-e bert-e merged commit b1f81ed into development/126.0 Sep 13, 2023
27 checks passed
@bert-e bert-e deleted the improvement/minimum-memory-cpu-for-os-kubelet branch September 13, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TeddyAndrieux TeddyAndrieux TeddyAndrieux approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ezekiel-alexrod @bert-e @TeddyAndrieux