Skip to content

Adds cgroup path to CreateVMResponse #306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion firecracker-control/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion firecracker-control/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
180 changes: 126 additions & 54 deletions proto/firecracker.pb.go

Large diffs are not rendered by default.

16 changes: 12 additions & 4 deletions proto/firecracker.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion proto/service/fccontrol/fccontrol.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
31 changes: 16 additions & 15 deletions proto/service/fccontrol/ttrpc/fccontrol.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 13 additions & 1 deletion runtime/jailer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
)
}
25 changes: 20 additions & 5 deletions runtime/runc_jailer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions runtime/runc_jailer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -72,7 +73,6 @@ func TestBuildJailedRootHandler_Isolated(t *testing.T) {
},
},
}
vmID := "foo"
handler := jailer.BuildJailedRootHandler(&cfg, &machineConfig, vmID)

machine := firecracker.Machine{
Expand Down
9 changes: 7 additions & 2 deletions runtime/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {
Expand Down
9 changes: 7 additions & 2 deletions runtime/service_integ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down