From 758ee9a403b07ebda22c6eb3d166410fc2d33779 Mon Sep 17 00:00:00 2001
From: Samuel Ortiz <sameo@linux.intel.com>
Date: Thu, 14 Sep 2017 12:16:55 +0200
Subject: [PATCH] oci: Follow the OCI spec for cgroup paths

Getting an absolute path while not having a cgroup mount
from the OCI spec is a valid case. In that case the path
should be interpreted as a relative one to the system
cgroup mount point.

Fixes #552

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>
---
 create_test.go | 25 +++++++++++++++----------
 oci.go         | 10 +++-------
 oci_test.go    | 10 +++++-----
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/create_test.go b/create_test.go
index 369b9fa2..3ee4773e 100644
--- a/create_test.go
+++ b/create_test.go
@@ -467,7 +467,7 @@ func TestCreateContainerInvalid(t *testing.T) {
 	}
 }
 
-func TestCreateProcessCgroupsPathFail(t *testing.T) {
+func TestCreateProcessCgroupsPathSuccessful(t *testing.T) {
 	assert := assert.New(t)
 
 	pod := &vcMock.Pod{
@@ -522,33 +522,38 @@ func TestCreateProcessCgroupsPathFail(t *testing.T) {
 		Limit: &limit,
 	}
 
-	// Set an absolute (and invalid) path
-	spec.Linux.CgroupsPath = "/this/is/not/a/valid/cgroup/path"
+	// Set an absolute path
+	spec.Linux.CgroupsPath = "/this/is/a/cgroup/path"
 
 	var mounts []specs.Mount
 	foundMount := false
 
+	// Replace the standard cgroup destination with a temporary one.
 	for _, mount := range spec.Mounts {
 		if mount.Type == "cgroup" {
 			foundMount = true
-		} else {
-			mounts = append(mounts, mount)
+			cgroupDir, err := ioutil.TempDir("", "cgroup")
+			assert.NoError(err)
+
+			defer os.RemoveAll(cgroupDir)
+			mount.Destination = cgroupDir
 		}
+
+		mounts = append(mounts, mount)
 	}
 
 	assert.True(foundMount)
 
-	// Remove the cgroup mount
+	// Replace mounts with the newly created one.
 	spec.Mounts = mounts
 
 	// Rewrite the file
 	err = writeOCIConfigFile(spec, ociConfigFile)
 	assert.NoError(err)
 
-	for detach := range []bool{true, false} {
-		err := create(testContainerID, bundlePath, testConsole, pidFilePath, true, runtimeConfig)
-		assert.Error(err, "%+v", detach)
-		assert.False(vcMock.IsMockError(err))
+	for _, detach := range []bool{true, false} {
+		err := create(testContainerID, bundlePath, testConsole, pidFilePath, detach, runtimeConfig)
+		assert.NoError(err, "detached: %+v", detach)
 	}
 }
 
diff --git a/oci.go b/oci.go
index 3206bb12..abade61e 100644
--- a/oci.go
+++ b/oci.go
@@ -228,13 +228,9 @@ func processCgroupsPathForResource(ociSpec oci.CompatOCISpec, resource string, i
 	}
 
 	if !cgroupMountFound {
-		if isPod {
-			return "", fmt.Errorf("cgroupsPath %q is absolute, cgroup mount MUST exist",
-				ociSpec.Linux.CgroupsPath)
-		}
-
-		// In case of container (CRI-O), if the mount point is not
-		// provided, we assume this is a relative path.
+		// According to the OCI spec, an absolute path should be
+		// interpreted as relative to the system cgroup mount point
+		// when there is no cgroup mount point.
 		return filepath.Join(cgroupsDirPath, resource, ociSpec.Linux.CgroupsPath), nil
 	}
 
diff --git a/oci_test.go b/oci_test.go
index 4a2d822e..4c1a889f 100644
--- a/oci_test.go
+++ b/oci_test.go
@@ -344,9 +344,9 @@ func TestProcessCgroupsPathRelativePathSuccessful(t *testing.T) {
 	}
 }
 
-func TestProcessCgroupsPathAbsoluteNoCgroupMountFailure(t *testing.T) {
-	assert := assert.New(t)
+func TestProcessCgroupsPathAbsoluteNoCgroupMountSuccessful(t *testing.T) {
 	absoluteCgroupsPath := "/absolute/cgroups/path"
+	cgroupsDirPath = "/foo/runtime/base"
 
 	ociSpec := oci.CompatOCISpec{}
 
@@ -357,9 +357,9 @@ func TestProcessCgroupsPathAbsoluteNoCgroupMountFailure(t *testing.T) {
 	for _, d := range cgroupTestData {
 		ociSpec.Linux.Resources = d.linuxSpec
 
-		_, err := processCgroupsPath(ociSpec, true)
-		assert.Error(err, "This test should fail because no cgroup mount provided (%+v)", d)
-		assert.False(vcMock.IsMockError(err))
+		p := filepath.Join(cgroupsDirPath, d.resource, absoluteCgroupsPath)
+
+		testProcessCgroupsPath(t, ociSpec, []string{p})
 	}
 }