Do not display the resource limits warning message #18052
Conversation
pkg/specgen/generate/validate.go
Outdated
| warnings = append(warnings, "Resource limits are not supported and ignored on cgroups V1 rootless systems") | ||
| } | ||
|
|
||
| if s.ResourceLimits == nil { | ||
| if s.ResourceLimits == resourceNil { |
There was a problem hiding this comment.
I find the code very hard to read because we're mixing the condition checks with setting s.ResourceLimits in case it's nil.
Can we rewrite it and return []string{"Resource limits are not supported and ignored on cgroups V1 rootless systems"}, nil immediately?
We should also move down the declarations of sysInfo and warnings.
There was a problem hiding this comment.
yes, please rewrite it because it is too difficult to follow now.
Just return the warning immediately, no need to set s.ResourceLimits to a dummy value
sstosh
force-pushed
the
resource-rootless
branch
2 times, most recently
from
91a8606 to
a95c9a4
Compare
sstosh
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
sstosh
force-pushed
the
resource-rootless
branch
from
a95c9a4 to
2b3f0ef
Compare
pkg/specgen/generate/validate.go
Outdated
| ) | ||
|
|
||
| // Verify resource limits are sanely set when running on cgroup v1. | ||
| func verifyContainerResourcesCgroupV1(s *specgen.SpecGenerator) ([]string, error) { | ||
| var resourceNil *spec.LinuxResources |
There was a problem hiding this comment.
Is this variable still needed? Does the != nil check not work?
There was a problem hiding this comment.
s.ResourceLimits is not nil when the container created by podman kube play.
s.ResourceLimits
(dlv) p s.ResourceLimits
*github.com/opencontainers/runtime-spec/specs-go.LinuxResources {
Devices: []github.com/opencontainers/runtime-spec/specs-go.LinuxDeviceCgroup len: 0, cap: 0, nil,
Memory: *github.com/opencontainers/runtime-spec/specs-go.LinuxMemory nil,
CPU: *github.com/opencontainers/runtime-spec/specs-go.LinuxCPU nil,
Pids: *github.com/opencontainers/runtime-spec/specs-go.LinuxPids nil,
BlockIO: *github.com/opencontainers/runtime-spec/specs-go.LinuxBlockIO nil,
HugepageLimits: []github.com/opencontainers/runtime-spec/specs-go.LinuxHugepageLimit len: 0, cap: 0, nil,
Network: *github.com/opencontainers/runtime-spec/specs-go.LinuxNetwork nil,
Rdma: map[string]github.com/opencontainers/runtime-spec/specs-go.LinuxRdma nil,
Unified: map[string]string nil,}
I think s.ResourceLimits is equal to var resourceNil *spec.LinuxResources.
sstosh
force-pushed
the
resource-rootless
branch
3 times, most recently
from
23db1ef to
af71543
Compare
sstosh
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
If resource limits is not set, do not display the following warning message: `Resource limits are not supported and ignored on cgroups V1 rootless systems` Ref: containers#17582 Signed-off-by: Toshiki Sonoda <[email protected]>
sstosh
force-pushed
the
resource-rootless
branch
from
af71543 to
4f5f89c
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, sstosh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/lgtm |
This is a regression for containers#18052. When podman ignores the resource limits, s.ResourceLimits needs to be nil. [NO NEW TESTS NEEDED] Signed-off-by: Toshiki Sonoda <[email protected]>
github-actions
bot
added
the
locked - please file new issue/PR
If resource limits is not set, do not display the following warning message:
Resource limits are not supported and ignored on cgroups V1 rootless systemsRef: #17582
Does this PR introduce a user-facing change?