Merge pull request #306 from xibz/jailer-cgroup-path-clean

Adds cgroup path to CreateVMResponse

Author: xibz

firecracker-control/local.go

@@ -78,7 +78,7 @@ func newLocal(ic *plugin.InitContext) (*local, error) {
 // CreateVM creates new Firecracker VM instance. It creates a runtime shim for the VM and the forwards
 // the CreateVM request to that shim. If there is already a VM created with the provided VMID, then
 // AlreadyExists is returned.
-func (s *local) CreateVM(requestCtx context.Context, req *proto.CreateVMRequest) (*empty.Empty, error) {
+func (s *local) CreateVM(requestCtx context.Context, req *proto.CreateVMRequest) (*proto.CreateVMResponse, error) {
 	var err error
 
 	id := req.GetVMID()

firecracker-control/service.go

@@ -67,7 +67,7 @@ func (s *service) RegisterTTRPC(server *ttrpc.Server) error {
 	return nil
 }
 
-func (s *service) CreateVM(ctx context.Context, req *proto.CreateVMRequest) (*empty.Empty, error) {
+func (s *service) CreateVM(ctx context.Context, req *proto.CreateVMRequest) (*proto.CreateVMResponse, error) {
 	log.G(ctx).Debugf("create VM request: %+v", req)
 	return s.local.CreateVM(ctx, req)
 }

proto/firecracker.pb.go

@@ -36,6 +36,14 @@ message CreateVMRequest {
     JailerConfig JailerConfig = 10;
 }
 
+message CreateVMResponse {
+    string VMID = 1;
+    string SocketPath = 2;
+    string LogFifoPath = 3;
+    string MetricsFifoPath = 4;
+    string CgroupPath = 5;
+}
+
 message StopVMRequest {
     string VMID = 1;
     uint32 TimeoutSeconds = 2;
@@ -47,10 +55,10 @@ message GetVMInfoRequest {
 
 message GetVMInfoResponse {
     string VMID = 1;
-    uint32 ContextID = 2;
-    string SocketPath = 3;
-    string LogFifoPath = 4;
-    string MetricsFifoPath = 5;
+    string SocketPath = 2;
+    string LogFifoPath = 3;
+    string MetricsFifoPath = 4;
+    string CgroupPath = 5;
 }
 
 message SetVMMetadataRequest {

proto/firecracker.proto

@@ -8,7 +8,7 @@ option go_package = "fccontrol";
 
 service Firecracker {
     // Runs new Firecracker VM instance
-    rpc CreateVM(CreateVMRequest) returns (google.protobuf.Empty);
+    rpc CreateVM(CreateVMRequest) returns (CreateVMResponse);
 
     // Stops existing Firecracker instance by VM ID
     rpc StopVM(StopVMRequest) returns (google.protobuf.Empty);

proto/service/fccontrol/fccontrol.proto

@@ -58,6 +58,10 @@ type jailer interface {
 	StubDrivesOptions() []stubDrivesOpt
 }
 
+type cgroupPather interface {
+	CgroupPath() string
+}
+
 // newJailer is used to construct a jailer from the CreateVM request. If no
 // request or jailer config was provided, then the noopJailer will be returned.
 func newJailer(
@@ -73,5 +77,13 @@ func newJailer(
 	}
 
 	l := logger.WithField("jailer", "runc")
-	return newRuncJailer(ctx, l, ociBundlePath, service.config.JailerConfig.RuncBinaryPath, jailerUID, jailerGID)
+	return newRuncJailer(
+		ctx,
+		l,
+		service.vmID,
+		ociBundlePath,
+		service.config.JailerConfig.RuncBinaryPath,
+		jailerUID,
+		jailerGID,
+	)
 }

proto/service/fccontrol/ttrpc/fccontrol.pb.go

@@ -50,22 +50,29 @@ type runcJailer struct {
 	runcBinaryPath string
 	uid            uint32
 	gid            uint32
+	vmID           string
 	configSpec     specs.Spec
 }
 
 const firecrackerFileName = "firecracker"
 
-func newRuncJailer(ctx context.Context, logger *logrus.Entry, ociBundlePath, runcBinPath string, uid, gid uint32) (*runcJailer, error) {
-	l := logger.WithField("ociBundlePath", ociBundlePath).
-		WithField("runcBinaryPath", runcBinPath)
-
+func newRuncJailer(
+	ctx context.Context,
+	logger *logrus.Entry,
+	vmID string,
+	ociBundlePath string,
+	runcBinaryPath string,
+	uid uint32,
+	gid uint32) (*runcJailer, error) {
+	l := logger.WithField("ociBundlePath", ociBundlePath).WithField("runcBinaryPath", runcBinaryPath)
 	j := &runcJailer{
 		ctx:            ctx,
 		logger:         l,
 		ociBundlePath:  ociBundlePath,
-		runcBinaryPath: runcBinPath,
+		runcBinaryPath: runcBinaryPath,
 		uid:            uid,
 		gid:            gid,
+		vmID:           vmID,
 	}
 
 	spec := specs.Spec{}
@@ -418,6 +425,10 @@ func (j *runcJailer) overwriteConfig(cfg *Config, machineConfig *firecracker.Con
 	return nil
 }
 
+func (j runcJailer) CgroupPath() string {
+	return filepath.Join("/firecracker-containerd", j.vmID)
+}
+
 // setDefaultConfigValues will process the spec file provided and allow any
 // empty/zero values to be replaced with default values.
 func (j *runcJailer) setDefaultConfigValues(cfg *Config, socketPath string, spec specs.Spec) specs.Spec {
@@ -436,6 +447,10 @@ func (j *runcJailer) setDefaultConfigValues(cfg *Config, socketPath string, spec
 		spec.Process.Args = cmd.Args
 	}
 
+	cgroupPath := j.CgroupPath()
+	j.logger.WithField("CgroupPath", cgroupPath).Debug("using cgroup path")
+	spec.Linux.CgroupsPath = cgroupPath
+
 	return spec
 }
 

runtime/jailer.go

@@ -53,7 +53,8 @@ func TestBuildJailedRootHandler_Isolated(t *testing.T) {
 	defer firecrackerFd.Close()
 
 	l := logrus.NewEntry(logrus.New())
-	jailer, err := newRuncJailer(context.Background(), l, dir, "bin-path", 123, 456)
+	vmID := "foo"
+	jailer, err := newRuncJailer(context.Background(), l, vmID, dir, "path/to/runc", 123, 456)
 	require.NoError(t, err, "failed to create runc jailer")
 
 	cfg := Config{
@@ -72,7 +73,6 @@ func TestBuildJailedRootHandler_Isolated(t *testing.T) {
 			},
 		},
 	}
-	vmID := "foo"
 	handler := jailer.BuildJailedRootHandler(&cfg, &machineConfig, vmID)
 
 	machine := firecracker.Machine{

runtime/runc_jailer.go

@@ -410,12 +410,13 @@ func (s *service) waitVMReady() error {
 
 // CreateVM will attempt to create the VM as specified in the provided request, but only on the first request
 // received. Any subsequent requests will be ignored and get an AlreadyExists error response.
-func (s *service) CreateVM(requestCtx context.Context, request *proto.CreateVMRequest) (*empty.Empty, error) {
+func (s *service) CreateVM(requestCtx context.Context, request *proto.CreateVMRequest) (*proto.CreateVMResponse, error) {
 	defer logPanicAndDie(s.logger)
 
 	var (
 		err       error
 		createRan bool
+		resp      proto.CreateVMResponse
 	)
 
 	s.vmStartOnce.Do(func() {
@@ -445,7 +446,11 @@ func (s *service) CreateVM(requestCtx context.Context, request *proto.CreateVMRe
 
 	// let all the other methods know that the VM is ready for tasks
 	close(s.vmReady)
-	return &empty.Empty{}, nil
+
+	if c, ok := s.jailer.(cgroupPather); ok {
+		resp.CgroupPath = c.CgroupPath()
+	}
+	return &resp, nil
 }
 
 func (s *service) publishVMStart() error {

runtime/runc_jailer_test.go

@@ -274,9 +274,10 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 
 			rootfsPath := defaultVMRootfsPath
 
+			vmIDStr := strconv.Itoa(vmID)
 			fcClient := fccontrol.NewFirecrackerClient(pluginClient.Client())
 			req := &proto.CreateVMRequest{
-				VMID: strconv.Itoa(vmID),
+				VMID: vmIDStr,
 				MachineCfg: &proto.FirecrackerMachineConfiguration{
 					MemSizeMib: 512,
 				},
@@ -296,7 +297,7 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 				JailerConfig:   jailerConfig,
 			}
 
-			_, err = fcClient.CreateVM(ctx, req)
+			resp, err := fcClient.CreateVM(ctx, req)
 			require.NoError(t, err, "failed to create vm")
 
 			var containerWg sync.WaitGroup
@@ -403,9 +404,13 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 
 						jailer := &runcJailer{
 							ociBundlePath: string(shimDir),
+							vmID:          vmIDStr,
 						}
 						_, err = os.Stat(jailer.RootPath())
 						require.NoError(t, err, "failed to stat root path of jailer")
+						_, err = os.Stat(filepath.Join("/sys/fs/cgroup/cpu", resp.CgroupPath))
+						require.NoError(t, err, "failed to stat cgroup path of jailer")
+						assert.Equal(t, filepath.Join("/firecracker-containerd", vmIDStr), resp.CgroupPath)
 					}
 
 					// Verify each exec had the same stdout and use that value as the mount namespace that will be compared