Skip to content

[20.10 backport] Fix setting swaplimit=true without checking#42479

Merged
thaJeztah merged 1 commit intomoby:20.10from
AkihiroSuda:cherrypick-42071
Jul 15, 2021
Merged

[20.10 backport] Fix setting swaplimit=true without checking#42479
thaJeztah merged 1 commit intomoby:20.10from
AkihiroSuda:cherrypick-42071

Conversation

@AkihiroSuda
Copy link
Copy Markdown
Member

Cherry-pick #42071


- What I did
fixes #41926

- How I did it
Added one condition to moby/pkg/sysinfo/cgroup2_linux.go

- How to verify it
Actually, I wanted to create unit tests for this one. The problem is certain files must be present on FS, so I see 2 solutions:

  1. use mocks (of os.Stat for example)
  2. since part of the path is passed as an argument, choose a different dir for tests (eg /tmp?)

I do not know what is the common practice in the project (1st PR), so I decided just to simply open a PR and ask.

- Description for the changelog

Fixes setting swaplimit=true without checking memory.swap.max

- A picture of a cute animal (not mandatory but encouraged)
Another time :)

Signed-off-by: Jakub Guzik <[email protected]>
(cherry picked from commit 7ef6ece)
Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda
Copy link
Copy Markdown
Member Author

@thaJeztah @kolyshkin PTAL

Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit a504476 into moby:20.10 Jul 15, 2021
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

}

info.MemoryLimit = true
info.SwapLimit = true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel like the old method is a lot faster 🤣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cgroup2 cgroup v2 kind/bugfix PR's that fix bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants