Automatically set or not set resource quota through the cluster #59

pycgo · 2024-01-05T08:38:10Z

CRI-O 1.28 no longer supports CGOUP1 for CPU allocation when use centos7, In order to be compatible, we should remove the CPU restrictions in the script.
#53

kvaps · 2024-01-05T11:16:04Z

The requests/limits options were introduced by @arothste-blk in #38
We should decide which defaults are better, but keep both options available.

pycgo · 2024-01-08T10:12:28Z

I don’t think the CPU limit is necessary. The actual amount of CPU used is very small and it should be compatible with most environments. Most operating systems, especially older operating systems, have cgoup values of 1 instead of 2.
If you do not modify this tool, upgrading the system where k8s is located will be a big job.

arothste-blk · 2024-01-08T14:16:29Z

My clusters require requests and limits for both CPU and memory on every pod whether I like it or not. That was why I added them. Feel free to adjust them as u see fit. Perhaps u can add a mechanism for removing/adding them as required by users so that I can continue to use them and those who don’t won’t. Drew

…

________________________________ From: pycgo ***@***.***> Sent: Monday, January 8, 2024 5:12:38 AM To: kvaps/kubectl-node-shell ***@***.***> Cc: Rothstein, Drew ***@***.***>; Mention ***@***.***> Subject: Re: [kvaps/kubectl-node-shell] delele cpu limit fix timeout (PR #59) External Email: Use caution with links and attachments I don’t think the CPU limit is necessary. The actual amount of CPU used is very small and it should be compatible with most environments. Most operating systems, especially older operating systems, have cgoup values of 1 instead of 2. If you do not modify this tool, upgrading the system where k8s is located will be a big job. — Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https://github.com/kvaps/kubectl-node-shell/pull/59*issuecomment-1880709308__;Iw!!KSjYCgUGsB4!ZtLxiwiNbF534S8wKGUIpuN9w6XbTHDEronuUYrk220KNLItw4kPT2KONt5B9uW8zrGu-lKU9F-ZLLZEGGGVSYx2OA$>, or unsubscribe<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AMWEUSSSQKHWLAH6MP5LNQTYNPBBNAVCNFSM6AAAAABBODA7FWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBQG4YDSMZQHA__;!!KSjYCgUGsB4!ZtLxiwiNbF534S8wKGUIpuN9w6XbTHDEronuUYrk220KNLItw4kPT2KONt5B9uW8zrGu-lKU9F-ZLLZEGGGSTSctbA$>. You are receiving this because you were mentioned.Message ID: ***@***.***> This message may contain information that is confidential or privileged. If you are not the intended recipient, please advise the sender immediately and delete this message. See http://www.blackrock.com/corporate/compliance/email-disclaimers for further information. Please refer to http://www.blackrock.com/corporate/compliance/privacy-policy for more information about BlackRock’s Privacy Policy. For a list of BlackRock's office addresses worldwide, see http://www.blackrock.com/corporate/about-us/contacts-locations. © 2024 BlackRock, Inc. All rights reserved.

kvaps · 2024-01-08T15:12:33Z

@arothste-blk is there any way to implement any sort of auto-detection for resources requirements?
Could you check something like:

if kubectl run --image test test --dry-run 2>&1 | grep -q 'resources.requests: Required value' then
  container_cpu="100m"
  container_memory="256Mi"
fi

in your cluster please

pycgo · 2024-01-10T02:22:14Z

in k8s create this yaml to test it

apiVersion: v1
kind: ResourceQuota
metadata:
  name: mem-cpu-example
spec:
  hard:
    requests.cpu: 150m
    requests.memory: 200Mi
    limits.cpu: 2
    limits.memory: 4Gi

[root@master ~]# kubectl run --image test test --dry-run
W0110 10:21:43.697889  104817 helpers.go:692] --dry-run is deprecated and can be replaced with --dry-run=client.
pod/test created (dry run)
[root@master ~]# echo $?
0
[root@master ~]# kubectl run --image test test
Error from server (Forbidden): pods "test" is forbidden: failed quota: mem-cpu-example: must specify limits.cpu for: test; limits.memory for: test; requests.cpu for: test; requests.memory for: test
[root@master ~]# echo $?
1
[root@master ~]#

pycgo · 2024-01-12T09:19:36Z

we can use this to test and It won't really be created, but it can be verified

kubectl run test --image test --dry-run=server

[root@master ~]# kubectl run test --image test --dry-run=server
Error from server (Forbidden): pods "test" is forbidden: failed quota: mem-cpu-example: must specify limits.cpu for: test; limits.memory for: test; requests.cpu for: test; requests.memory for: test

@arothste-blk can you test this in your cluster?

kvaps · 2024-01-12T22:46:39Z

I think the best option is to:

test if resource specification is required

if not: do not specify any resources
if yes: specify both CPU and MEM

I could implement this by myself, but I have no free time right now. Sorry.

If you want this asap, feel free to add this check into your PR :)

pycgo · 2024-01-17T10:31:57Z

@kvaps I've changed it, take a look. thank you.

kvaps

Amazing job, please apply review suggestions and here we go

kvaps · 2024-01-17T11:17:17Z

kubectl-node_shell

@@ -140,6 +140,20 @@ else
  fi
 fi

+# test if resource specification is required
+set +e


Suggested change

set +e

if do not set this,when grep nothting ,the shell will exit ,can you test it in no limit cluster ?
I can not find other way to fix it.

kvaps · 2024-01-17T11:17:25Z

kubectl-node_shell

+  resources_json='"resources": {}'
+
+fi
+set -e


Suggested change

set -e

kubectl-node_shell

Co-authored-by: Andrei Kvapil <[email protected]>

pycgo · 2024-01-18T02:16:48Z

@kvaps I fix it ,and test ok,take a look again.thank you.

kvaps

Ah, I forgot we use set -e (I confused it with set -x)
But as I can see, you already found elegant solution!

LGTM

kvaps · 2024-01-18T03:44:05Z

kubectl-node_shell

+          "limits":   { "cpu": "'${container_cpu}'", "memory": "'${container_memory}'" },
+          "requests": { "cpu": "'${container_cpu}'", "memory": "'${container_memory}'" }
+        }'
+$kubectl run test --image test --dry-run=server 2>&1 | grep 'failed quota' --quiet || resources_json='"resources": {}'


Very tricky, bravo👏

delele cpu limit

e8163a9

Update kubectl-node_shell

2b4df33

pycgo changed the title ~~delele cpu limit fix timeout~~ Automatically set or not set resource quota through the cluster Jan 17, 2024

kvaps reviewed Jan 17, 2024

View reviewed changes

pycgo and others added 2 commits January 18, 2024 09:57

change grep message

a2ea0cb

Co-authored-by: Andrei Kvapil <[email protected]>

set resource if or not depend on k8s quota

457fce0

kvaps approved these changes Jan 18, 2024

View reviewed changes

kvaps merged commit ef2c556 into kvaps:master Jan 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically set or not set resource quota through the cluster #59

Automatically set or not set resource quota through the cluster #59

pycgo commented Jan 5, 2024 •

edited

Loading

kvaps commented Jan 5, 2024

pycgo commented Jan 8, 2024

arothste-blk commented Jan 8, 2024 via email

kvaps commented Jan 8, 2024 •

edited

Loading

pycgo commented Jan 10, 2024 •

edited

Loading

pycgo commented Jan 12, 2024

kvaps commented Jan 12, 2024 •

edited

Loading

pycgo commented Jan 17, 2024

kvaps left a comment •

edited

Loading

kvaps Jan 17, 2024

pycgo Jan 18, 2024 •

edited

Loading

kvaps Jan 17, 2024

pycgo commented Jan 18, 2024

kvaps left a comment •

edited

Loading

kvaps Jan 18, 2024

Automatically set or not set resource quota through the cluster #59

Automatically set or not set resource quota through the cluster #59

Conversation

pycgo commented Jan 5, 2024 • edited Loading

kvaps commented Jan 5, 2024

pycgo commented Jan 8, 2024

arothste-blk commented Jan 8, 2024 via email

kvaps commented Jan 8, 2024 • edited Loading

pycgo commented Jan 10, 2024 • edited Loading

pycgo commented Jan 12, 2024

kvaps commented Jan 12, 2024 • edited Loading

pycgo commented Jan 17, 2024

kvaps left a comment • edited Loading

Choose a reason for hiding this comment

kvaps Jan 17, 2024

Choose a reason for hiding this comment

pycgo Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

kvaps Jan 17, 2024

Choose a reason for hiding this comment

pycgo commented Jan 18, 2024

kvaps left a comment • edited Loading

Choose a reason for hiding this comment

kvaps Jan 18, 2024

Choose a reason for hiding this comment

pycgo commented Jan 5, 2024 •

edited

Loading

kvaps commented Jan 8, 2024 •

edited

Loading

pycgo commented Jan 10, 2024 •

edited

Loading

kvaps commented Jan 12, 2024 •

edited

Loading

kvaps left a comment •

edited

Loading

pycgo Jan 18, 2024 •

edited

Loading

kvaps left a comment •

edited

Loading