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

get mem on cgroupsv2 #2119

Merged
merged 2 commits into from
Aug 7, 2023
Merged

Conversation

KannarFr
Copy link
Contributor

@KannarFr KannarFr commented Jun 7, 2023

Description of your changes:

Fixes: #2118

@KannarFr KannarFr force-pushed the supportCGroupsV2MemoryGetter branch 2 times, most recently from d329bfd to 3c90753 Compare June 7, 2023 13:38
@KannarFr
Copy link
Contributor Author

KannarFr commented Jun 8, 2023

I checked the failing test but got

Identified problems
--
  | No identified problem
  | No problems were identified. If you know why this problem occurred, please add a suitable Cause for it.

Shall I run a bash linter or something?

@guits
Copy link
Collaborator

guits commented Jul 31, 2023

jenkins lint

@guits
Copy link
Collaborator

guits commented Jul 31, 2023


In /common_functions.sh line 525:
  if grep -q cgroup2 /proc/filesystems; then
  ^-- SC1009 (info): The mentioned syntax error was in this if expression.


In /common_functions.sh line 529:
  elif
  ^-- SC1049 (error): Did you forget the 'then' for this 'elif'?
  ^-- SC1073 (error): Couldn't parse this elif clause. Fix to allow more checks.


In /common_functions.sh line 533:
  fi
    ^-- SC1072 (error): Unexpected keyword/token. Fix any mentioned problems and try again.

@KannarFr KannarFr force-pushed the supportCGroupsV2MemoryGetter branch from 3c90753 to 3d05627 Compare July 31, 2023 19:27
src/daemon/common_functions.sh Outdated Show resolved Hide resolved
src/daemon/common_functions.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@guits guits left a comment

Choose a reason for hiding this comment

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

not related to this patch but if you want to address the warning SC2071 L124 you can apply this small change:

diff --git a/src/daemon/common_functions.sh b/src/daemon/common_functions.sh
index 1b679c6c..9e99046f 100755
--- a/src/daemon/common_functions.sh
+++ b/src/daemon/common_functions.sh
@@ -121,7 +121,7 @@ function dev_part {
       if [[ $(readlink -f "$option") == "$desired_partition" ]]; then
         local optprefixlen
         optprefixlen=$(prefix_length "$option" "$osd_device")
-        if [[ $optprefixlen > $pfxlen ]]; then
+        if [[ $optprefixlen -gt $pfxlen ]]; then
           link=$option
           pfxlen=$optprefixlen
         fi

Copy link
Collaborator

@guits guits left a comment

Choose a reason for hiding this comment

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

@KannarFr you need to add a sign off to your commit message

thanks!

@KannarFr KannarFr force-pushed the supportCGroupsV2MemoryGetter branch from 0a2c438 to 6583c0f Compare August 1, 2023 13:24
@KannarFr
Copy link
Contributor Author

KannarFr commented Aug 1, 2023

@guits thanks for the hints. I applied all of them. I also sign off it.

@guits
Copy link
Collaborator

guits commented Aug 1, 2023

@KannarFr Could you squash the first 3 commits? I think you should have only 2 commits in this PR.

get mem on cgroupsv2 and apply unrelated lint fix

Thanks!

@KannarFr
Copy link
Contributor Author

KannarFr commented Aug 1, 2023

@guits Damn I thought github will do it by itself. Done.

@KannarFr KannarFr force-pushed the supportCGroupsV2MemoryGetter branch from 6583c0f to 7cb22fb Compare August 1, 2023 14:02
This commit adds cgroup2 support in get_available_ram() function
defined in common_functions.sh

Signed-off-by: Alexandre DUVAL - @KannarFr <kannarfr@gmail.com>
This addresses a `SC2071` warning.

'>' operator is for string comparisons.
`-gt` should be used here. See [1]

[1] https://www.shellcheck.net/wiki/SC2071

Signed-off-by: Alexandre DUVAL - @KannarFr <kannarfr@gmail.com>
@guits guits force-pushed the supportCGroupsV2MemoryGetter branch from 7cb22fb to 9baede8 Compare August 3, 2023 11:28
@guits
Copy link
Collaborator

guits commented Aug 3, 2023

failure in container / arm64 is unrelated to this patch

@guits guits merged commit 27de8d8 into ceph:main Aug 7, 2023
5 of 6 checks passed
@KannarFr
Copy link
Contributor Author

KannarFr commented Aug 7, 2023

@guits can you make a release with this patch? :)

@guits
Copy link
Collaborator

guits commented Aug 8, 2023

@KannarFr which release are you looking for?

@KannarFr
Copy link
Contributor Author

KannarFr commented Aug 8, 2023

ceph/daemon

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.

get memory limit issues
2 participants