kill shim functionality

firecracker update

Signed-off-by: Plamen Petrov <[email protected]>

Author: plamenmpetrov

Makefile

@@ -278,8 +278,8 @@ demo-network: install-cni-bins $(FCNET_CONFIG)
 # Firecracker submodule
 ##########################
 .PHONY: firecracker
-firecracker: $(FIRECRACKER_BIN) $(JAILER_BIN)
-
+firecracker:
+	_submodules/firecracker/tools/devtool build --release
 .PHONY: install-firecracker
 install-firecracker: firecracker
 	install -D -o root -g root -m755 -t $(INSTALLROOT)/bin $(FIRECRACKER_BIN)

_submodules/firecracker

@@ -71,6 +71,8 @@ type local struct {
 
 	processesMu sync.Mutex
 	processes   map[string]int32
+
+	fcControlSocket *net.UnixListener
 }
 
 func newLocal(ic *plugin.InitContext) (*local, error) {
@@ -243,7 +245,7 @@ func (s *local) StopVM(requestCtx context.Context, req *proto.StopVMRequest) (*e
 	defer client.Close()
 
 	resp, shimErr := client.StopVM(requestCtx, req)
-	waitErr := s.waitForShimToExit(requestCtx, req.VMID)
+	waitErr := s.waitForShimToExit(requestCtx, req.VMID, false)
 
 	// Assuming the shim is returning containerd's error code, return the error as is if possible.
 	if waitErr == nil {
@@ -252,7 +254,7 @@ func (s *local) StopVM(requestCtx context.Context, req *proto.StopVMRequest) (*e
 	return resp, multierror.Append(shimErr, waitErr).ErrorOrNil()
 }
 
-func (s *local) waitForShimToExit(ctx context.Context, vmID string) error {
+func (s *local) waitForShimToExit(ctx context.Context, vmID string, killShim bool) error {
 	socketAddr, err := fcShim.SocketAddress(ctx, vmID)
 	if err != nil {
 		return err
@@ -267,6 +269,17 @@ func (s *local) waitForShimToExit(ctx context.Context, vmID string) error {
 	}
 	defer delete(s.processes, socketAddr)
 
+	if killShim {
+		s.logger.Debug("Killing shim")
+
+		if err := syscall.Kill(int(pid), 9); err != nil {
+			s.logger.WithError(err).Error("Failed to kill shim process")
+			return err
+		}
+
+		return nil
+	}
+
 	return internal.WaitForPidToExit(ctx, stopVMInterval, pid)
 }
 
@@ -464,6 +477,150 @@ func setShimOOMScore(shimPid int) error {
 	return nil
 }
 
+func (s *local) loadShim(ctx context.Context, ns, vmID, containerdAddress string) (*exec.Cmd, error) {
+	logger := s.logger.WithField("vmID", vmID)
+	logger.Debug("Loading shim")
+
+	shimSocketAddress, err := fcShim.SocketAddress(ctx, vmID)
+	if err != nil {
+		err = errors.Wrap(err, "failed to obtain shim socket address")
+		s.logger.WithError(err).Error()
+		return nil, err
+	}
+
+	shimSocket, err := shim.NewSocket(shimSocketAddress)
+	if isEADDRINUSE(err) {
+		return nil, status.Errorf(codes.AlreadyExists, "VM with ID %q already exists (socket: %q)", vmID, shimSocketAddress)
+	} else if err != nil {
+		err = errors.Wrapf(err, "failed to open shim socket at address %q", shimSocketAddress)
+		s.logger.WithError(err).Error()
+		return nil, err
+	}
+
+	// If we're here, there is no pre-existing shim for this VMID, so we spawn a new one
+	defer shimSocket.Close()
+	if err := os.Mkdir(s.config.ShimBaseDir, 0700); err != nil && !os.IsExist(err) {
+		s.logger.WithError(err).Error()
+		return nil, errors.Wrapf(err, "failed to make shim base directory: %s", s.config.ShimBaseDir)
+	}
+
+	shimDir, err := vm.ShimDir(s.config.ShimBaseDir, ns, vmID)
+	if err != nil {
+		err = errors.Wrapf(err, "failed to build shim path")
+		s.logger.WithError(err).Error()
+		return nil, err
+	}
+
+	err = shimDir.Mkdir()
+	if err != nil {
+		err = errors.Wrapf(err, "failed to create VM dir %q", shimDir.RootPath())
+		s.logger.WithError(err).Error()
+		return nil, err
+	}
+
+	fcSocketAddress, err := fcShim.FCControlSocketAddress(ctx, vmID)
+	if err != nil {
+		err = errors.Wrap(err, "failed to obtain shim socket address")
+		s.logger.WithError(err).Error()
+		return nil, err
+	}
+
+	fcSocket, err := shim.NewSocket(fcSocketAddress)
+	if err != nil {
+		err = errors.Wrapf(err, "failed to open fccontrol socket at address %q", fcSocketAddress)
+		s.logger.WithError(err).Error()
+		return nil, err
+	}
+
+	s.fcControlSocket = fcSocket
+
+	args := []string{
+		"-namespace", ns,
+		"-address", containerdAddress,
+	}
+
+	cmd := exec.Command(internal.ShimBinaryName, args...)
+
+	// note: The working dir of the shim has an effect on the length of the path
+	// needed to specify various unix sockets that the shim uses to communicate
+	// with the firecracker VMM and guest agent within. The length of that path
+	// has a relatively low limit (usually 108 chars), so modifying the working
+	// dir should be done with caution. See internal/vm/dir.go for the path
+	// definitions.
+	cmd.Dir = shimDir.RootPath()
+
+	shimSocketFile, err := shimSocket.File()
+	if err != nil {
+		err = errors.Wrap(err, "failed to get shim socket fd")
+		logger.WithError(err).Error()
+		return nil, err
+	}
+
+	fcSocketFile, err := fcSocket.File()
+	if err != nil {
+		err = errors.Wrap(err, "failed to get shim fccontrol socket fd")
+		logger.WithError(err).Error()
+		return nil, err
+	}
+
+	cmd.ExtraFiles = append(cmd.ExtraFiles, shimSocketFile, fcSocketFile)
+	fcSocketFDNum := 2 + len(cmd.ExtraFiles) // "2 +" because ExtraFiles come after stderr (fd #2)
+
+	ttrpc := containerdAddress + ".ttrpc"
+	cmd.Env = append(os.Environ(),
+		fmt.Sprintf("%s=%s", ttrpcAddressEnv, ttrpc),
+		fmt.Sprintf("%s=%s", internal.VMIDEnvVarKey, vmID),
+		fmt.Sprintf("%s=%s", internal.FCSocketFDEnvKey, strconv.Itoa(fcSocketFDNum))) // TODO remove after containerd is updated to expose ttrpc server to shim
+
+	cmd.SysProcAttr = &syscall.SysProcAttr{
+		Setpgid: true,
+	}
+
+	// shim stderr is just raw text, so pass it through our logrus formatter first
+	cmd.Stderr = logger.WithField("shim_stream", "stderr").WriterLevel(logrus.ErrorLevel)
+	// shim stdout on the other hand is already formatted by logrus, so pass that transparently through to containerd logs
+	cmd.Stdout = logger.Logger.Out
+
+	logger.Debugf("starting %s", internal.ShimBinaryName)
+
+	err = cmd.Start()
+	if err != nil {
+		err = errors.Wrap(err, "failed to start shim child process")
+		logger.WithError(err).Error()
+		return nil, err
+	}
+
+	// make sure to wait after start
+	go func() {
+		if err := cmd.Wait(); err != nil {
+			if exitErr, ok := err.(*exec.ExitError); ok {
+				// shim is usually terminated by cancelling the context
+				logger.WithError(exitErr).Debug("shim has been terminated")
+			} else {
+				logger.WithError(err).Error("shim has been unexpectedly terminated")
+			}
+		}
+
+		// Close all Unix abstract sockets.
+		if err := shimSocketFile.Close(); err != nil {
+			logger.WithError(err).Errorf("failed to close %q", shimSocketFile.Name())
+		}
+		if err := fcSocketFile.Close(); err != nil {
+			logger.WithError(err).Errorf("failed to close %q", fcSocketFile.Name())
+		}
+	}()
+
+	err = setShimOOMScore(cmd.Process.Pid)
+	if err != nil {
+		logger.WithError(err).Error()
+		return nil, err
+	}
+
+	s.addShim(shimSocketAddress, cmd)
+
+	return cmd, nil
+}
+
 // PauseVM Pauses a VM
 func (s *local) PauseVM(ctx context.Context, req *proto.PauseVMRequest) (*empty.Empty, error) {
 	client, err := s.shimFirecrackerClient(ctx, req.VMID)
@@ -520,6 +677,18 @@ func (s *local) CreateSnapshot(ctx context.Context, req *proto.CreateSnapshotReq
 
 // LoadSnapshot Loads a snapshot of a VM
 func (s *local) LoadSnapshot(ctx context.Context, req *proto.LoadSnapshotRequest) (*empty.Empty, error) {
+	ns, err := namespaces.NamespaceRequired(ctx)
+	if err != nil {
+		err = errors.Wrap(err, "error retrieving namespace of request")
+		s.logger.WithError(err).Error()
+		return nil, err
+	}
+
+	_, err = s.loadShim(ctx, ns, req.VMID, s.containerdAddress)
+	if err != nil {
+		return nil, err
+	}
+
 	client, err := s.shimFirecrackerClient(ctx, req.VMID)
 	if err != nil {
 		return nil, err
@@ -553,5 +722,36 @@ func (s *local) Offload(ctx context.Context, req *proto.OffloadRequest) (*empty.
 		return nil, err
 	}
 
+	s.fcControlSocket.Close()
+
+	shimSocketAddress, err := fcShim.SocketAddress(ctx, req.VMID)
+	if err != nil {
+		err = errors.Wrap(err, "failed to obtain shim socket address")
+		s.logger.WithError(err).Error()
+		return nil, err
+	}
+	removeErr := os.RemoveAll(shimSocketAddress)
+	if removeErr != nil {
+		s.logger.Errorf("failed to remove shim socket addr file: %v", removeErr)
+		return nil, err
+	}
+
+	fcSocketAddress, err := fcShim.FCControlSocketAddress(ctx, req.VMID)
+	if err != nil {
+		s.logger.Error("failed to get FC socket address")
+		return nil, err
+	}
+	removeErr = os.RemoveAll(fcSocketAddress)
+	if removeErr != nil {
+		s.logger.Errorf("failed to remove fc socket addr file: %v", removeErr)
+		return nil, err
+	}
+
+	waitErr := s.waitForShimToExit(ctx, req.VMID, true)
+	if waitErr != nil {
+		s.logger.Error("failed to wait for shim to exit on offload")
+		return nil, waitErr
+	}
+
 	return resp, nil
 }

firecracker-control/local.go

@@ -811,24 +811,6 @@ func (s *service) LoadSnapshot(ctx context.Context, req *proto.LoadSnapshotReque
 		return nil, err
 	}
 
-	// Workaround for https://github.com/firecracker-microvm/firecracker/issues/1974
-	patchDriveReq, err := formPatchDriveReq("MN2HE43UOVRDA", s.taskDrivePathOnHost)
-	if err != nil {
-		s.logger.WithError(err).Error("Failed to create patch drive request")
-		return nil, err
-	}
-
-	resp, err = s.httpControlClient.Do(patchDriveReq)
-	if err != nil {
-		s.logger.WithError(err).Error("Failed to send patch drive request")
-		return nil, err
-	}
-	if !strings.Contains(resp.Status, "204") {
-		s.logger.WithError(err).Error("Failed to patch drive")
-		s.logger.WithError(err).Errorf("Status of request: %s", resp.Status)
-		return nil, err
-	}
-
 	return &empty.Empty{}, nil
 }
 
@@ -1643,31 +1625,6 @@ func formCreateSnapReq(snapshotPath, memPath string) (*http.Request, error) {
 	return req, nil
 }
 
-func formPatchDriveReq(driveID, pathOnHost string) (*http.Request, error) {
-	var req *http.Request
-
-	data := map[string]string{
-		"drive_id":     driveID,
-		"path_on_host": pathOnHost,
-	}
-	json, err := json.Marshal(data)
-	if err != nil {
-		logrus.WithError(err).Error("Failed to marshal json data")
-		return nil, err
-	}
-
-	req, err = http.NewRequest("PATCH", fmt.Sprintf("http+unix://firecracker/drives/%s", driveID), bytes.NewBuffer(json))
-	if err != nil {
-		logrus.WithError(err).Error("Failed to create new HTTP request in formPauseReq")
-		return nil, err
-	}
-
-	req.Header.Add("accept", "application/json")
-	req.Header.Add("Content-Type", "application/json")
-
-	return req, nil
-}
-
 func (s *service) startFirecrackerProcess() error {
 	firecPath, err := exec.LookPath("firecracker")
 	if err != nil {