From 51c079c83a2e5c388b19d201b5ad3886e2f9cfd1 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Mon, 21 Aug 2023 13:56:03 +0200 Subject: [PATCH] Do not set Memory limit on podman when cgroup is v1 (#7028) --- pkg/dev/podmandev/pod.go | 14 ++++++++- pkg/dev/podmandev/pod_test.go | 55 +++++++++++++++++++++++++++++----- pkg/dev/podmandev/reconcile.go | 2 +- pkg/podman/capabilities.go | 17 +++++++++++ pkg/podman/info.go | 35 ++++++++++++++++++++++ pkg/podman/interface.go | 3 ++ pkg/podman/mock.go | 15 ++++++++++ 7 files changed, 131 insertions(+), 10 deletions(-) create mode 100644 pkg/podman/capabilities.go create mode 100644 pkg/podman/info.go diff --git a/pkg/dev/podmandev/pod.go b/pkg/dev/podmandev/pod.go index 281a7a4ca..21dd93442 100644 --- a/pkg/dev/podmandev/pod.go +++ b/pkg/dev/podmandev/pod.go @@ -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, @@ -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") diff --git a/pkg/dev/podmandev/pod_test.go b/pkg/dev/podmandev/pod_test.go index e0a4479e8..a4b804f23 100644 --- a/pkg/dev/podmandev/pod_test.go +++ b/pkg/dev/podmandev/pod_test.go @@ -7,6 +7,7 @@ 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" @@ -14,6 +15,7 @@ import ( "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" @@ -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", @@ -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)) @@ -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)) @@ -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, diff --git a/pkg/dev/podmandev/reconcile.go b/pkg/dev/podmandev/reconcile.go index e53f4df93..ae6a11881 100644 --- a/pkg/dev/podmandev/reconcile.go +++ b/pkg/dev/podmandev/reconcile.go @@ -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, diff --git a/pkg/podman/capabilities.go b/pkg/podman/capabilities.go new file mode 100644 index 000000000..a746686d9 --- /dev/null +++ b/pkg/podman/capabilities.go @@ -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 +} diff --git a/pkg/podman/info.go b/pkg/podman/info.go new file mode 100644 index 000000000..73d371710 --- /dev/null +++ b/pkg/podman/info.go @@ -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 +} diff --git a/pkg/podman/interface.go b/pkg/podman/interface.go index 22b720032..c88cf75b9 100644 --- a/pkg/podman/interface.go +++ b/pkg/podman/interface.go @@ -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) } diff --git a/pkg/podman/mock.go b/pkg/podman/mock.go index 55bd798b2..4ddcc10bc 100644 --- a/pkg/podman/mock.go +++ b/pkg/podman/mock.go @@ -97,6 +97,21 @@ func (mr *MockClientMockRecorder) GetAllResourcesFromSelector(selector, ns inter return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllResourcesFromSelector", reflect.TypeOf((*MockClient)(nil).GetAllResourcesFromSelector), selector, ns) } +// GetCapabilities mocks base method. +func (m *MockClient) GetCapabilities() (Capabilities, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetCapabilities") + ret0, _ := ret[0].(Capabilities) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetCapabilities indicates an expected call of GetCapabilities. +func (mr *MockClientMockRecorder) GetCapabilities() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCapabilities", reflect.TypeOf((*MockClient)(nil).GetCapabilities)) +} + // GetPodLogs mocks base method. func (m *MockClient) GetPodLogs(podName, containerName string, followLog bool) (io.ReadCloser, error) { m.ctrl.T.Helper()