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

Do not set Memory limit on podman when cgroup is v1 #7028

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion pkg/dev/podmandev/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const (
portForwardingHelperImage = "quay.io/devfile/base-developer-image@sha256:27d5ce66a259decb84770ea0d1ce8058a806f39dfcfeed8387f9cf2f29e76480"
)

func createPodFromComponent(
func (o *DevClient) createPodFromComponent(
ctx context.Context,
debug bool,
buildCommand string,
Expand All @@ -55,6 +55,18 @@ func createPodFromComponent(
if err != nil {
return nil, nil, err
}

podmanCaps, err := o.podmanClient.GetCapabilities()
if err != nil {
return nil, nil, err
}

if !podmanCaps.Cgroupv2 {
for i := range podTemplate.Spec.Containers {
delete(podTemplate.Spec.Containers[i].Resources.Limits, corev1.ResourceMemory)
}
}

containers := podTemplate.Spec.Containers
if len(containers) == 0 {
return nil, nil, fmt.Errorf("no valid components found in the devfile")
Expand Down
55 changes: 47 additions & 8 deletions pkg/dev/podmandev/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import (
"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/v2/pkg/devfile/parser"
"github.com/devfile/library/v2/pkg/devfile/parser/data"
"github.com/golang/mock/gomock"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"

"github.com/redhat-developer/odo/pkg/api"
"github.com/redhat-developer/odo/pkg/labels"
"github.com/redhat-developer/odo/pkg/libdevfile/generator"
odocontext "github.com/redhat-developer/odo/pkg/odo/context"
"github.com/redhat-developer/odo/pkg/podman"
"github.com/redhat-developer/odo/pkg/version"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -143,11 +145,12 @@ func Test_createPodFromComponent(t *testing.T) {
customAddress string
}
tests := []struct {
name string
args args
wantPod func(basePod *corev1.Pod) *corev1.Pod
wantFwPorts []api.ForwardedPort
wantErr bool
name string
capabilities podman.Capabilities
args args
wantPod func(basePod *corev1.Pod) *corev1.Pod
wantFwPorts []api.ForwardedPort
wantErr bool
}{
{
name: "basic component without command / forwardLocalhost=false",
Expand Down Expand Up @@ -237,7 +240,10 @@ func Test_createPodFromComponent(t *testing.T) {
},
},
{
name: "basic component + memory limit / forwardLocalhost=false",
name: "basic component + memory limit / forwardLocalhost=false, cgroup=v2",
capabilities: podman.Capabilities{
Cgroupv2: true,
},
args: args{
devfileObj: func() parser.DevfileObj {
data, _ := data.NewDevfileData(string(data.APISchemaVersion200))
Expand All @@ -261,7 +267,34 @@ func Test_createPodFromComponent(t *testing.T) {
},
},
{
name: "basic component + memory limit / forwardLocalhost=true",
name: "basic component + memory limit / forwardLocalhost=false, cgroup=v1",
capabilities: podman.Capabilities{
Cgroupv2: false,
},
args: args{
devfileObj: func() parser.DevfileObj {
data, _ := data.NewDevfileData(string(data.APISchemaVersion200))
_ = data.AddCommands([]v1alpha2.Command{command})
cmp := baseComponent.DeepCopy()
cmp.Container.MemoryLimit = "1Gi"
_ = data.AddComponents([]v1alpha2.Component{*cmp})
return parser.DevfileObj{
Data: data,
}
},
componentName: devfileName,
appName: appName,
},
wantPod: func(basePod *corev1.Pod) *corev1.Pod {
pod := basePod.DeepCopy()
return pod
},
},
{
name: "basic component + memory limit / forwardLocalhost=true, cgroup=v2",
capabilities: podman.Capabilities{
Cgroupv2: true,
},
args: args{
devfileObj: func() parser.DevfileObj {
data, _ := data.NewDevfileData(string(data.APISchemaVersion200))
Expand Down Expand Up @@ -1412,7 +1445,13 @@ func Test_createPodFromComponent(t *testing.T) {
ctx = odocontext.WithApplication(ctx, tt.args.appName)
ctx = odocontext.WithComponentName(ctx, tt.args.componentName)
ctx = odocontext.WithWorkingDirectory(ctx, "/tmp/dir")
got, gotFwPorts, err := createPodFromComponent(
ctrl := gomock.NewController(t)
podmanClient := podman.NewMockClient(ctrl)
podmanClient.EXPECT().GetCapabilities().Return(tt.capabilities, nil)
client := NewDevClient(
nil, podmanClient, nil, nil, nil, nil, nil, nil,
)
got, gotFwPorts, err := client.createPodFromComponent(
ctx,
tt.args.debug,
tt.args.buildCommand,
Expand Down
2 changes: 1 addition & 1 deletion pkg/dev/podmandev/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (o *DevClient) deployPod(ctx context.Context, options dev.StartOptions, dev
spinner := log.Spinner("Deploying pod")
defer spinner.End(false)

pod, fwPorts, err := createPodFromComponent(
pod, fwPorts, err := o.createPodFromComponent(
ctx,
options.Debug,
options.BuildCommand,
Expand Down
17 changes: 17 additions & 0 deletions pkg/podman/capabilities.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package podman

type Capabilities struct {
Cgroupv2 bool
}

func (o *PodmanCli) GetCapabilities() (Capabilities, error) {
var result Capabilities
info, err := o.getInfo()
if err != nil {
return Capabilities{}, err
}
if info.Host.CgroupsVersion == "v2" {
result.Cgroupv2 = true
}
return result, nil
}
35 changes: 35 additions & 0 deletions pkg/podman/info.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package podman

import (
"encoding/json"
"fmt"
"os/exec"

"k8s.io/klog"
)

// podmanInfo originates from https://github.com/containers/podman/blob/main/libpod/define/info.go
type podmanInfo struct {
Host *HostInfo `json:"host"`
}

type HostInfo struct {
CgroupsVersion string `json:"cgroupVersion"`
}

func (o *PodmanCli) getInfo() (podmanInfo, error) {
cmd := exec.Command(o.podmanCmd, append(o.containerRunGlobalExtraArgs, "info", "--format", "json")...)
klog.V(3).Infof("executing %v", cmd.Args)
out, err := cmd.Output()
if err != nil {
if exiterr, ok := err.(*exec.ExitError); ok {
err = fmt.Errorf("%s: %s", err, string(exiterr.Stderr))
}
return podmanInfo{}, err
}

var result podmanInfo

err = json.Unmarshal(out, &result)
return result, err
}
3 changes: 3 additions & 0 deletions pkg/podman/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,7 @@ type Client interface {
ListAllComponents() ([]api.ComponentAbstract, error)

Version(ctx context.Context) (SystemVersionReport, error)

// GetCapabilities returns the capabilities of the underlying system
GetCapabilities() (Capabilities, error)
}
15 changes: 15 additions & 0 deletions pkg/podman/mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.