Merge pull request #63 from ChrsMark/lenient-subsystems-checking

estesp · web-flow · commit 82cb49fc1779 · 2018-11-05T13:24:09.000-05:00
Makes Load function more lenient on subsystems' checking
diff --git a/cgroup.go b/cgroup.go
@@ -48,11 +48,12 @@ func New(hierarchy Hierarchy, path Path, resources *specs.LinuxResources) (Cgrou
 
 // Load will load an existing cgroup and allow it to be controlled
 func Load(hierarchy Hierarchy, path Path) (Cgroup, error) {
+	var activeSubsystems []Subsystem
 	subsystems, err := hierarchy()
 	if err != nil {
 		return nil, err
 	}
-	// check the the subsystems still exist
+	// check that the subsystems still exist, and keep only those that actually exist
 	for _, s := range pathers(subsystems) {
 		p, err := path(s.Name())
 		if err != nil {
@@ -63,14 +64,15 @@ func Load(hierarchy Hierarchy, path Path) (Cgroup, error) {
 		}
 		if _, err := os.Lstat(s.Path(p)); err != nil {
 			if os.IsNotExist(err) {
-				return nil, ErrCgroupDeleted
+				continue
 			}
 			return nil, err
 		}
+		activeSubsystems = append(activeSubsystems, s)
 	}
 	return &cgroup{
 		path:       path,
-		subsystems: subsystems,
+		subsystems: activeSubsystems,
 	}, nil
 }
 
diff --git a/cgroup_test.go b/cgroup_test.go
@@ -197,6 +197,20 @@ func checkTaskid(mock *mockCgroup, path string, expected int) error {
 	return nil
 }
 
+func mockNewNotInRdma(subsystems []Subsystem, path Path, resources *specs.LinuxResources) (Cgroup, error) {
+	for _, s := range subsystems {
+		if s.Name() != Rdma {
+			if err := initializeSubsystem(s, path, resources); err != nil {
+				return nil, err
+			}
+		}
+	}
+	return &cgroup{
+		path:       path,
+		subsystems: subsystems,
+	}, nil
+}
+
 func TestLoad(t *testing.T) {
 	mock, err := newMock()
 	if err != nil {
@@ -218,6 +232,36 @@ func TestLoad(t *testing.T) {
 	}
 }
 
+func TestLoadWithMissingSubsystems(t *testing.T) {
+	mock, err := newMock()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer mock.delete()
+	subsystems, err := mock.hierarchy()
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	control, err := mockNewNotInRdma(subsystems, StaticPath("test"), &specs.LinuxResources{})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	if control, err = Load(mock.hierarchy, StaticPath("test")); err != nil {
+		t.Error(err)
+		return
+	}
+	if control == nil {
+		t.Error("control is nil")
+		return
+	}
+	if len(control.Subsystems()) != len(subsystems) - 1 {
+		t.Error("wrong number of active subsystems")
+		return
+	}
+}
+
 func TestDelete(t *testing.T) {
 	mock, err := newMock()
 	if err != nil {

Original file line number	Diff line number	Diff line change
`@@ -48,11 +48,12 @@ func New(hierarchy Hierarchy, path Path, resources *specs.LinuxResources) (Cgrou`
`48`	`48`
`49`	`49`	`// Load will load an existing cgroup and allow it to be controlled`
`50`	`50`	`func Load(hierarchy Hierarchy, path Path) (Cgroup, error) {`
	`51`	`+ var activeSubsystems []Subsystem`
`51`	`52`	`subsystems, err := hierarchy()`
`52`	`53`	`if err != nil {`
`53`	`54`	`return nil, err`
`54`	`55`	`}`
`55`		`- // check the the subsystems still exist`
	`56`	`+ // check that the subsystems still exist, and keep only those that actually exist`
`56`	`57`	`for _, s := range pathers(subsystems) {`
`57`	`58`	`p, err := path(s.Name())`
`58`	`59`	`if err != nil {`
`@@ -63,14 +64,15 @@ func Load(hierarchy Hierarchy, path Path) (Cgroup, error) {`
`63`	`64`	`}`
`64`	`65`	`if _, err := os.Lstat(s.Path(p)); err != nil {`
`65`	`66`	`if os.IsNotExist(err) {`
`66`		`- return nil, ErrCgroupDeleted`
	`67`	`+ continue`
`67`	`68`	`}`
`68`	`69`	`return nil, err`
`69`	`70`	`}`
	`71`	`+ activeSubsystems = append(activeSubsystems, s)`
`70`	`72`	`}`
`71`	`73`	`return &cgroup{`
`72`	`74`	`path: path,`
`73`		`- subsystems: subsystems,`
	`75`	`+ subsystems: activeSubsystems,`
`74`	`76`	`}, nil`
`75`	`77`	`}`
`76`	`78`