Fix JSON marshaling of runsc cgroup configuration.

PiperOrigin-RevId: 470357748

runsc/cgroup/cgroup.go

@@ -41,9 +41,7 @@ import (
 const (
 	cgroupv1FsName = "cgroup"
 	cgroupv2FsName = "cgroup2"
-)
 
-const (
 	// procRoot is the procfs root this module uses.
 	procRoot = "/proc"
 
@@ -415,67 +413,67 @@ func new(pid, cgroupsPath string, useSystemd bool) (Cgroup, error) {
 
 // CgroupJSON is a wrapper for Cgroup that can be encoded to JSON.
 type CgroupJSON struct {
-	Cgroup     Cgroup `json:"cgroup"`
-	UseSystemd bool   `json:"useSystemd"`
+	Cgroup Cgroup
 }
 
 type cgroupJSONv1 struct {
-	Cgroup *cgroupV1 `json:"cgroup"`
+	Cgroup *cgroupV1 `json:"cgroupv1"`
 }
 
 type cgroupJSONv2 struct {
-	Cgroup *cgroupV2 `json:"cgroup"`
+	Cgroup *cgroupV2 `json:"cgroupv2"`
 }
 
 type cgroupJSONSystemd struct {
-	Cgroup *cgroupSystemd `json:"cgroup"`
+	Cgroup *cgroupSystemd `json:"cgroupsystemd"`
+}
+
+type cgroupJSONUnknown struct {
+	Cgroup interface{} `json:"cgroupunknown"`
 }
 
 // UnmarshalJSON implements json.Unmarshaler.UnmarshalJSON
 func (c *CgroupJSON) UnmarshalJSON(data []byte) error {
-	if c.UseSystemd {
-		systemd := cgroupJSONSystemd{}
-		if err := json.Unmarshal(data, &systemd); err != nil {
-			return err
-		}
-		if systemd.Cgroup != nil {
-			c.Cgroup = systemd.Cgroup
-		}
-		return nil
-	}
-
-	if IsOnlyV2() {
-		v2 := cgroupJSONv2{}
-		err := json.Unmarshal(data, &v2)
-		if v2.Cgroup != nil {
-			c.Cgroup = v2.Cgroup
-		}
+	m := map[string]json.RawMessage{}
+	if err := json.Unmarshal(data, &m); err != nil {
 		return err
 	}
-	v1 := cgroupJSONv1{}
-	err := json.Unmarshal(data, &v1)
-	if v1.Cgroup != nil {
-		c.Cgroup = v1.Cgroup
+
+	var cg Cgroup
+	if rm, ok := m["cgroupv1"]; ok {
+		cg = &cgroupV1{}
+		if err := json.Unmarshal(rm, cg); err != nil {
+			return err
+		}
+	} else if rm, ok := m["cgroupv2"]; ok {
+		cg = &cgroupV2{}
+		if err := json.Unmarshal(rm, cg); err != nil {
+			return err
+		}
+	} else if rm, ok := m["cgroupsystemd"]; ok {
+		cg = &cgroupSystemd{}
+		if err := json.Unmarshal(rm, cg); err != nil {
+			return err
+		}
 	}
-	return err
+	c.Cgroup = cg
+	return nil
 }
 
 // MarshalJSON implements json.Marshaler.MarshalJSON
 func (c *CgroupJSON) MarshalJSON() ([]byte, error) {
 	if c.Cgroup == nil {
-		v1 := cgroupJSONv1{}
-		return json.Marshal(&v1)
+		return json.Marshal(cgroupJSONUnknown{})
 	}
-	if IsOnlyV2() {
-		if c.UseSystemd {
-			systemd := cgroupJSONSystemd{Cgroup: c.Cgroup.(*cgroupSystemd)}
-			return json.Marshal(&systemd)
-		}
-		v2 := cgroupJSONv2{Cgroup: c.Cgroup.(*cgroupV2)}
-		return json.Marshal(&v2)
+	switch c.Cgroup.(type) {
+	case *cgroupV1:
+		return json.Marshal(cgroupJSONv1{Cgroup: c.Cgroup.(*cgroupV1)})
+	case *cgroupV2:
+		return json.Marshal(cgroupJSONv2{Cgroup: c.Cgroup.(*cgroupV2)})
+	case *cgroupSystemd:
+		return json.Marshal(cgroupJSONSystemd{Cgroup: c.Cgroup.(*cgroupSystemd)})
 	}
-	v1 := cgroupJSONv1{Cgroup: c.Cgroup.(*cgroupV1)}
-	return json.Marshal(&v1)
+	return nil, nil
 }
 
 // Install creates and configures cgroups according to 'res'. If cgroup path

runsc/cgroup/cgroup_test.go

@@ -15,6 +15,7 @@
 package cgroup
 
 import (
+	"encoding/json"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -905,3 +906,55 @@ func TestOptional(t *testing.T) {
 		})
 	}
 }
+
+func TestJSON(t *testing.T) {
+	for _, tc := range []struct {
+		cg Cgroup
+	}{
+		{
+			cg: &cgroupV1{
+				Name:    "foobar",
+				Parents: map[string]string{"hello": "world"},
+				Own:     map[string]bool{"parent": true},
+			},
+		},
+		{
+			cg: &cgroupV2{
+				Mountpoint:  "foobar",
+				Path:        "a/path/here",
+				Controllers: []string{"test", "controllers"},
+				Own:         []string{"I", "own", "this"},
+			},
+		},
+		{
+			cg: CreateMockSystemdCgroup(),
+		},
+		{
+			cg: nil,
+		},
+	} {
+		in := &CgroupJSON{Cgroup: tc.cg}
+		data, err := json.Marshal(in)
+		if err != nil {
+			t.Fatalf("could not serialize %v to JSON: %v", in, err)
+		}
+		out := &CgroupJSON{}
+		if err := json.Unmarshal(data, out); err != nil {
+			t.Fatalf("could not deserialize %v from JSON: %v", data, err)
+		}
+		switch tc.cg.(type) {
+		case *cgroupSystemd:
+			if _, ok := out.Cgroup.(*cgroupSystemd); !ok {
+				t.Errorf("cgroup incorrectly deserialized from JSON: got %v, want %v", out.Cgroup, tc.cg)
+			}
+		case *cgroupV1:
+			if _, ok := out.Cgroup.(*cgroupV1); !ok {
+				t.Errorf("cgroup incorrectly deserialized from JSON: got %v, want %v", out.Cgroup, tc.cg)
+			}
+		case *cgroupV2:
+			if _, ok := out.Cgroup.(*cgroupV2); !ok {
+				t.Errorf("cgroup incorrectly deserialized from JSON: got %v, want %v", out.Cgroup, tc.cg)
+			}
+		}
+	}
+}

runsc/cgroup/systemd.go

@@ -291,3 +291,17 @@ func newProp(name string, units interface{}) systemdDbus.Property {
 		Value: dbus.MakeVariant(units),
 	}
 }
+
+// CreateMockSystemdCgroup returns a mock Cgroup configured for systemd. This
+// is useful for testing.
+func CreateMockSystemdCgroup() Cgroup {
+	return &cgroupSystemd{
+		Name:        "test",
+		ScopePrefix: "runsc",
+		Parent:      "system.slice",
+		cgroupV2: cgroupV2{
+			Mountpoint: "/sys/fs/cgroup",
+			Path:       "/a/random/path",
+		},
+	}
+}

runsc/container/BUILD

@@ -80,6 +80,7 @@ go_test(
         "//pkg/unet",
         "//pkg/urpc",
         "//runsc/boot",
+        "//runsc/cgroup",
         "//runsc/config",
         "//runsc/flag",
         "//runsc/specutils",

runsc/container/container_test.go

@@ -42,6 +42,7 @@ import (
 	"gvisor.dev/gvisor/pkg/sync"
 	"gvisor.dev/gvisor/pkg/test/testutil"
 	"gvisor.dev/gvisor/pkg/urpc"
+	"gvisor.dev/gvisor/runsc/cgroup"
 	"gvisor.dev/gvisor/runsc/config"
 	"gvisor.dev/gvisor/runsc/flag"
 	"gvisor.dev/gvisor/runsc/specutils"
@@ -2675,3 +2676,40 @@ func TestProfile(t *testing.T) {
 		}
 	}
 }
+
+// TestSaveSystemdCgroup emulates a sandbox saving while configured with the
+// systemd cgroup driver.
+func TestSaveSystemdCgroup(t *testing.T) {
+	spec, conf := sleepSpecConf(t)
+	_, bundleDir, cleanup, err := testutil.SetupContainer(spec, conf)
+	if err != nil {
+		t.Fatalf("error setting up container: %v", err)
+	}
+	defer cleanup()
+
+	// Create and start the container.
+	args := Args{
+		ID:        testutil.RandomContainerID(),
+		Spec:      spec,
+		BundleDir: bundleDir,
+	}
+	cont, err := New(conf, args)
+	if err != nil {
+		t.Fatalf("error creating container: %v", err)
+	}
+	defer cont.Destroy()
+
+	cont.CompatCgroup = cgroup.CgroupJSON{Cgroup: cgroup.CreateMockSystemdCgroup()}
+	if err := cont.Saver.lock(); err != nil {
+		t.Fatalf("cannot lock container metadata file: %v", err)
+	}
+	if err := cont.saveLocked(); err != nil {
+		t.Fatalf("error saving cgroup: %v", err)
+	}
+	cont.Saver.unlock()
+	loadCont := Container{}
+	cont.Saver.load(&loadCont)
+	if !reflect.DeepEqual(cont.CompatCgroup, loadCont.CompatCgroup) {
+		t.Errorf("CompatCgroup not properly saved: want %v, got %v", cont.CompatCgroup, loadCont.CompatCgroup)
+	}
+}

runsc/sandbox/sandbox.go

@@ -179,8 +179,7 @@ func New(conf *config.Config, args *Args) (*Sandbox, error) {
 	s := &Sandbox{
 		ID: args.ID,
 		CgroupJSON: cgroup.CgroupJSON{
-			Cgroup:     args.Cgroup,
-			UseSystemd: conf.SystemdCgroup,
+			Cgroup: args.Cgroup,
 		},
 		UID: -1, // prevent usage before it's set.
 		GID: -1, // prevent usage before it's set.