Merge pull request #83 from coderbirju/updateContainerStop

feat: add signal option to containerStop

Author: coderbirju

api/handlers/container/container.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"io"
 	"net/http"
-	"time"
 
 	ncTypes "github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/containerd/nerdctl/v2/pkg/config"
@@ -23,7 +22,7 @@ type Service interface {
 	Remove(ctx context.Context, cid string, force, removeVolumes bool) error
 	Wait(ctx context.Context, cid string, options ncTypes.ContainerWaitOptions) error
 	Start(ctx context.Context, cid string, options ncTypes.ContainerStartOptions) error
-	Stop(ctx context.Context, cid string, timeout *time.Duration) error
+	Stop(ctx context.Context, cid string, option ncTypes.ContainerStopOptions) error
 	Restart(ctx context.Context, cid string, options ncTypes.ContainerRestartOptions) error
 	Create(ctx context.Context, image string, cmd []string, createOpt ncTypes.ContainerCreateOptions, netOpt ncTypes.NetworkOptions) (string, error)
 	Inspect(ctx context.Context, cid string, size bool) (*types.Container, error)

api/handlers/container/stop.go

@@ -5,10 +5,12 @@ package container
 
 import (
 	"net/http"
+	"os"
 	"strconv"
 	"time"
 
 	"github.com/containerd/containerd/v2/pkg/namespaces"
+	ncTypes "github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/gorilla/mux"
 	"github.com/sirupsen/logrus"
 
@@ -24,8 +26,30 @@ func (h *handler) stop(w http.ResponseWriter, r *http.Request) {
 	}
 	timeout := time.Second * time.Duration(t)
 
+	signal := getSignal(r)
+	if signal == "" {
+		signal = "SIGTERM" // Docker/nerdctl default
+		logrus.Infof("Setting default %s to stop container", signal)
+	}
+
+	// discard unwanted logs by writing to /dev/null
+	devNull, err := os.OpenFile("/dev/null", os.O_WRONLY, 0600)
+	if err != nil {
+		response.JSON(w, http.StatusBadRequest, response.NewErrorFromMsg("failed to open /dev/null"))
+		return
+	}
+	defer devNull.Close()
+
 	ctx := namespaces.WithNamespace(r.Context(), h.Config.Namespace)
-	err = h.service.Stop(ctx, cid, &timeout)
+	globalOpt := ncTypes.GlobalCommandOptions(*h.Config)
+	stopOpts := ncTypes.ContainerStopOptions{
+		Stdout:   devNull,
+		Stderr:   devNull,
+		Timeout:  &timeout,
+		Signal:   signal,
+		GOptions: globalOpt,
+	}
+	err = h.service.Stop(ctx, cid, stopOpts)
 	// map the error into http status code and send response.
 	if err != nil {
 		var code int
@@ -44,3 +68,12 @@ func (h *handler) stop(w http.ResponseWriter, r *http.Request) {
 	// successfully stopped. Send no content status.
 	response.Status(w, http.StatusNoContent)
 }
+
+func getSignal(r *http.Request) string {
+	signal := r.URL.Query().Get("signal")
+	if signal == "" {
+		// If "signal" is not present, check for "s"
+		signal = r.URL.Query().Get("s")
+	}
+	return signal
+}

e2e/tests/container_restart.go

@@ -52,6 +52,9 @@ func ContainerRestart(opt *option.Option) {
 			Expect(err).Should(BeNil())
 			Expect(res.StatusCode).Should(Equal(http.StatusNoContent))
 
+			// Add sleep to ensure container has time to restart and execute the date command
+			time.Sleep(2 * time.Second)
+
 			logsRelativeUrl := fmt.Sprintf("/containers/%s/logs", testContainerName)
 			opts := "?stdout=1" +
 				"&stderr=0" +

internal/backend/container.go

@@ -11,7 +11,6 @@ import (
 	"io"
 	"os"
 	"os/exec"
-	"time"
 
 	containerd "github.com/containerd/containerd/v2/client"
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
@@ -28,7 +27,7 @@ import (
 type NerdctlContainerSvc interface {
 	RemoveContainer(ctx context.Context, c containerd.Container, force bool, removeAnonVolumes bool) error
 	StartContainer(ctx context.Context, cid string, options types.ContainerStartOptions) error
-	StopContainer(ctx context.Context, container containerd.Container, timeout *time.Duration) error
+	StopContainer(ctx context.Context, cid string, options types.ContainerStopOptions) error
 	CreateContainer(ctx context.Context, args []string, netManager containerutil.NetworkOptionsManager, options types.ContainerCreateOptions) (containerd.Container, func(), error)
 	InspectContainer(ctx context.Context, c containerd.Container, size bool) (*dockercompat.Container, error)
 	InspectNetNS(ctx context.Context, pid int) (*native.NetNS, error)
@@ -59,8 +58,8 @@ func (w *NerdctlWrapper) StartContainer(ctx context.Context, cid string, options
 }
 
 // StopContainer wrapper function to call nerdctl function to stop a container.
-func (*NerdctlWrapper) StopContainer(ctx context.Context, container containerd.Container, timeout *time.Duration) error {
-	return containerutil.Stop(ctx, container, timeout, "")
+func (w *NerdctlWrapper) StopContainer(ctx context.Context, cid string, options types.ContainerStopOptions) error {
+	return container.Stop(ctx, w.clientWrapper.client, []string{cid}, options)
 }
 
 func (w *NerdctlWrapper) CreateContainer(ctx context.Context, args []string, netManager containerutil.NetworkOptionsManager, options types.ContainerCreateOptions) (containerd.Container, func(), error) {

internal/service/container/restart.go

@@ -5,6 +5,7 @@ package container
 
 import (
 	"context"
+	"io"
 
 	"github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/runfinch/finch-daemon/pkg/errdefs"
@@ -16,10 +17,18 @@ func (s *service) Restart(ctx context.Context, cid string, options types.Contain
 		return err
 	}
 
+	stopOptions := types.ContainerStopOptions{
+		Stdout:   io.Discard,
+		Stderr:   io.Discard,
+		Timeout:  options.Timeout,
+		Signal:   "SIGTERM",
+		GOptions: options.GOption,
+	}
+
 	// restart the container and if error occurs then return error otherwise return nil
 	// swallow IsNotModified error on StopContainer for already stopped container, simply call StartContainer
 	s.logger.Debugf("restarting container: %s", cid)
-	if err := s.nctlContainerSvc.StopContainer(ctx, con, options.Timeout); err != nil && !errdefs.IsNotModified(err) {
+	if err := s.nctlContainerSvc.StopContainer(ctx, con.ID(), stopOptions); err != nil && !errdefs.IsNotModified(err) {
 		s.logger.Errorf("Failed to stop container: %s. Error: %v", cid, err)
 		return err
 	}

internal/service/container/restart_test.go

@@ -51,15 +51,17 @@ var _ = Describe("Container Restart API ", func() {
 		options = ncTypes.ContainerRestartOptions{
 			Timeout: &timeout,
 		}
+
 	})
 	Context("service", func() {
 		It("should not return any error when Running", func() {
 			// set up the mock to return a container that is in running state
+
 			cdClient.EXPECT().SearchContainer(gomock.Any(), cid).Return(
 				[]containerd.Container{con}, nil).AnyTimes()
 			//mock the nerdctl client to mock the restart container was successful without any error.
 			ncClient.EXPECT().StartContainer(ctx, gomock.Any(), gomock.Any()).Return(nil)
-			ncClient.EXPECT().StopContainer(ctx, con, &timeout).Return(nil)
+			ncClient.EXPECT().StopContainer(ctx, con.ID(), gomock.Any()).Return(nil)
 			gomock.InOrder(
 				logger.EXPECT().Debugf("restarting container: %s", cid),
 				logger.EXPECT().Debugf("successfully restarted: %s", cid),
@@ -73,7 +75,7 @@ var _ = Describe("Container Restart API ", func() {
 			cdClient.EXPECT().SearchContainer(gomock.Any(), cid).Return(
 				[]containerd.Container{con}, nil).AnyTimes()
 			//mock the nerdctl client to mock the restart container was successful without any error.
-			ncClient.EXPECT().StopContainer(ctx, con, &timeout).Return(errdefs.NewNotModified(fmt.Errorf("err")))
+			ncClient.EXPECT().StopContainer(ctx, con.ID(), gomock.Any()).Return(errdefs.NewNotModified(fmt.Errorf("err")))
 			ncClient.EXPECT().StartContainer(ctx, gomock.Any(), gomock.Any()).Return(nil)
 			gomock.InOrder(
 				logger.EXPECT().Debugf("restarting container: %s", cid),
@@ -88,7 +90,6 @@ var _ = Describe("Container Restart API ", func() {
 			cdClient.EXPECT().SearchContainer(gomock.Any(), gomock.Any()).Return(
 				[]containerd.Container{}, nil)
 			logger.EXPECT().Debugf("no such container: %s", gomock.Any())
-
 			// service should return NotFound error
 			err := service.Restart(ctx, cid, options)
 			Expect(errdefs.IsNotFound(err)).Should(BeTrue())
@@ -109,7 +110,7 @@ var _ = Describe("Container Restart API ", func() {
 				[]containerd.Container{con}, nil).AnyTimes()
 
 			expectedErr := fmt.Errorf("nerdctl error")
-			ncClient.EXPECT().StopContainer(ctx, con, &timeout).Return(nil)
+			ncClient.EXPECT().StopContainer(ctx, con.ID(), gomock.Any()).Return(nil)
 			ncClient.EXPECT().StartContainer(ctx, gomock.Any(), gomock.Any()).Return(expectedErr)
 			gomock.InOrder(
 				logger.EXPECT().Debugf("restarting container: %s", cid),

internal/service/container/stop.go

@@ -6,15 +6,15 @@ package container
 import (
 	"context"
 	"fmt"
-	"time"
 
 	containerd "github.com/containerd/containerd/v2/client"
+	ncTypes "github.com/containerd/nerdctl/v2/pkg/api/types"
 
 	"github.com/runfinch/finch-daemon/pkg/errdefs"
 )
 
 // Stop function stops a running container. It returns nil when it successfully stops the container.
-func (s *service) Stop(ctx context.Context, cid string, timeout *time.Duration) error {
+func (s *service) Stop(ctx context.Context, cid string, options ncTypes.ContainerStopOptions) error {
 	con, err := s.getContainer(ctx, cid)
 	if err != nil {
 		return err
@@ -23,7 +23,7 @@ func (s *service) Stop(ctx context.Context, cid string, timeout *time.Duration)
 	if s.isContainerStopped(ctx, con) {
 		return errdefs.NewNotModified(fmt.Errorf("container is already stopped: %s", cid))
 	}
-	if err = s.nctlContainerSvc.StopContainer(ctx, con, timeout); err != nil {
+	if err = s.nctlContainerSvc.StopContainer(ctx, con.ID(), options); err != nil {
 		s.logger.Errorf("Failed to stop container: %s. Error: %v", cid, err)
 		return err
 	}

internal/service/container/stop_test.go

@@ -12,6 +12,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	ncTypes "github.com/containerd/nerdctl/v2/pkg/api/types"
 	"github.com/runfinch/finch-daemon/api/handlers/container"
 	"github.com/runfinch/finch-daemon/mocks/mocks_archive"
 	"github.com/runfinch/finch-daemon/mocks/mocks_backend"
@@ -32,6 +33,7 @@ var _ = Describe("Container Stop API ", func() {
 		cid          string
 		tarExtractor *mocks_archive.MockTarExtractor
 		service      container.Service
+		stopOptions  ncTypes.ContainerStopOptions
 	)
 	BeforeEach(func() {
 		ctx = context.Background()
@@ -43,6 +45,7 @@ var _ = Describe("Container Stop API ", func() {
 		con = mocks_container.NewMockContainer(mockCtrl)
 		con.EXPECT().ID().Return(cid).AnyTimes()
 		tarExtractor = mocks_archive.NewMockTarExtractor(mockCtrl)
+		stopOptions = ncTypes.ContainerStopOptions{}
 
 		service = NewService(cdClient, mockNerdctlService{ncClient, nil}, logger, nil, nil, tarExtractor)
 	})
@@ -53,10 +56,10 @@ var _ = Describe("Container Stop API ", func() {
 			cdClient.EXPECT().SearchContainer(gomock.Any(), gomock.Any()).Return(
 				[]containerd.Container{con}, nil)
 
-			ncClient.EXPECT().StopContainer(ctx, con, gomock.Any())
+			ncClient.EXPECT().StopContainer(ctx, con.ID(), gomock.Any())
 			logger.EXPECT().Debugf("successfully stopped: %s", cid)
 
-			err := service.Stop(ctx, cid, nil)
+			err := service.Stop(ctx, cid, stopOptions)
 			Expect(err).Should(BeNil())
 		})
 		It("should return not found error", func() {
@@ -66,7 +69,7 @@ var _ = Describe("Container Stop API ", func() {
 			logger.EXPECT().Debugf("no such container: %s", cid)
 
 			// service should return NotFound error
-			err := service.Stop(ctx, cid, nil)
+			err := service.Stop(ctx, cid, stopOptions)
 			Expect(errdefs.IsNotFound(err)).Should(BeTrue())
 		})
 		It("should return multiple containers found error", func() {
@@ -76,7 +79,7 @@ var _ = Describe("Container Stop API ", func() {
 			logger.EXPECT().Debugf("multiple IDs found with provided prefix: %s, total containers found: %d", cid, 2)
 
 			// service should return error
-			err := service.Stop(ctx, cid, nil)
+			err := service.Stop(ctx, cid, stopOptions)
 			Expect(err).Should(Not(BeNil()))
 		})
 		It("should return not modified error as container is stopped already", func() {
@@ -86,7 +89,7 @@ var _ = Describe("Container Stop API ", func() {
 				[]containerd.Container{con}, nil)
 
 			// service should return not modified error.
-			err := service.Stop(ctx, cid, nil)
+			err := service.Stop(ctx, cid, stopOptions)
 			Expect(errdefs.IsNotModified(err)).Should(BeTrue())
 		})
 		It("should return not modified error as container is not running", func() {
@@ -96,7 +99,7 @@ var _ = Describe("Container Stop API ", func() {
 				[]containerd.Container{con}, nil)
 
 			// service should return not modified error.
-			err := service.Stop(ctx, cid, nil)
+			err := service.Stop(ctx, cid, stopOptions)
 			Expect(errdefs.IsNotModified(err)).Should(BeTrue())
 		})
 		It("should fail due to nerdctl client error", func() {
@@ -106,11 +109,11 @@ var _ = Describe("Container Stop API ", func() {
 				[]containerd.Container{con}, nil)
 
 			expectedErr := fmt.Errorf("nerdctl error")
-			ncClient.EXPECT().StopContainer(ctx, con, gomock.Any()).Return(expectedErr)
+			ncClient.EXPECT().StopContainer(ctx, con.ID(), gomock.Any()).Return(expectedErr)
 			logger.EXPECT().Errorf("Failed to stop container: %s. Error: %v", cid, expectedErr)
 
 			// service should return not modified error.
-			err := service.Stop(ctx, cid, nil)
+			err := service.Stop(ctx, cid, stopOptions)
 			Expect(err).Should(Equal(expectedErr))
 		})
 	})