diff --git a/api/handlers/container/container.go b/api/handlers/container/container.go index e0fb1218..2c51b99a 100644 --- a/api/handlers/container/container.go +++ b/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) diff --git a/api/handlers/container/stop.go b/api/handlers/container/stop.go index 9d28c69b..6739747d 100644 --- a/api/handlers/container/stop.go +++ b/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 +} diff --git a/e2e/tests/container_restart.go b/e2e/tests/container_restart.go index 57793bbf..883e65ca 100644 --- a/e2e/tests/container_restart.go +++ b/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" + diff --git a/internal/backend/container.go b/internal/backend/container.go index a87f7188..05b730a5 100644 --- a/internal/backend/container.go +++ b/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) @@ -58,8 +57,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) { diff --git a/internal/service/container/restart.go b/internal/service/container/restart.go index b5e1d4eb..92f76ff3 100644 --- a/internal/service/container/restart.go +++ b/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 } diff --git a/internal/service/container/restart_test.go b/internal/service/container/restart_test.go index 269f6385..067ec7f2 100644 --- a/internal/service/container/restart_test.go +++ b/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), diff --git a/internal/service/container/stop.go b/internal/service/container/stop.go index 0800ed18..edf64810 100644 --- a/internal/service/container/stop.go +++ b/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 } diff --git a/internal/service/container/stop_test.go b/internal/service/container/stop_test.go index 34ccc434..1b442b49 100644 --- a/internal/service/container/stop_test.go +++ b/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)) }) }) diff --git a/mocks/mocks_backend/nerdctlcontainersvc.go b/mocks/mocks_backend/nerdctlcontainersvc.go index 2313e953..6040c323 100644 --- a/mocks/mocks_backend/nerdctlcontainersvc.go +++ b/mocks/mocks_backend/nerdctlcontainersvc.go @@ -9,7 +9,6 @@ import ( io "io" os "os" reflect "reflect" - time "time" client "github.com/containerd/containerd/v2/client" types "github.com/containerd/nerdctl/v2/pkg/api/types" @@ -264,7 +263,7 @@ func (mr *MockNerdctlContainerSvcMockRecorder) StartContainer(arg0, arg1, arg2 i } // StopContainer mocks base method. -func (m *MockNerdctlContainerSvc) StopContainer(arg0 context.Context, arg1 client.Container, arg2 *time.Duration) error { +func (m *MockNerdctlContainerSvc) StopContainer(arg0 context.Context, arg1 string, arg2 types.ContainerStopOptions) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "StopContainer", arg0, arg1, arg2) ret0, _ := ret[0].(error) diff --git a/mocks/mocks_container/containersvc.go b/mocks/mocks_container/containersvc.go index a84cd9d3..b38499ae 100644 --- a/mocks/mocks_container/containersvc.go +++ b/mocks/mocks_container/containersvc.go @@ -8,7 +8,6 @@ import ( context "context" io "io" reflect "reflect" - time "time" types "github.com/containerd/nerdctl/v2/pkg/api/types" gomock "github.com/golang/mock/gomock" @@ -256,7 +255,7 @@ func (mr *MockServiceMockRecorder) Stats(arg0, arg1 interface{}) *gomock.Call { } // Stop mocks base method. -func (m *MockService) Stop(arg0 context.Context, arg1 string, arg2 *time.Duration) error { +func (m *MockService) Stop(arg0 context.Context, arg1 string, arg2 types.ContainerStopOptions) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Stop", arg0, arg1, arg2) ret0, _ := ret[0].(error)