Ported changes from internal 'implement container restart API' commit

Signed-off-by: Cezar Rata <ceradev@amazon.com>

api/handlers/container/container.go

@@ -24,6 +24,7 @@ type Service interface {
 	Wait(ctx context.Context, cid string, condition string) (code int64, err error)
 	Start(ctx context.Context, cid string) error
 	Stop(ctx context.Context, cid string, timeout *time.Duration) error
+	Restart(ctx context.Context, cid string, timeout time.Duration) error
 	Create(ctx context.Context, image string, cmd []string, createOpt ncTypes.ContainerCreateOptions, netOpt ncTypes.NetworkOptions) (string, error)
 	Inspect(ctx context.Context, cid string) (*types.Container, error)
 	WriteFilesAsTarArchive(filePath string, writer io.Writer, slashDot bool) error
@@ -44,6 +45,7 @@ func RegisterHandlers(r types.VersionedRouter, service Service, conf *config.Con
 	r.HandleFunc("/{id:.*}", h.remove, http.MethodDelete)
 	r.HandleFunc("/{id:.*}/start", h.start, http.MethodPost)
 	r.HandleFunc("/{id:.*}/stop", h.stop, http.MethodPost)
+	r.HandleFunc("/{id:.*}/restart", h.restart, http.MethodPost)
 	r.HandleFunc("/{id:.*}/remove", h.remove, http.MethodPost)
 	r.HandleFunc("/{id:.*}/wait", h.wait, http.MethodPost)
 	r.HandleFunc("/create", h.create, http.MethodPost)

api/handlers/container/container_test.go

@@ -76,6 +76,15 @@ var _ = Describe("Container API", func() {
 			Expect(rr).Should(HaveHTTPStatus(http.StatusInternalServerError))
 			Expect(rr.Body).Should(MatchJSON(`{"message": "error from stop api"}`))
 		})
+		It("should call container restart method", func() {
+			// setup mocks
+			service.EXPECT().Restart(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("error from restart api"))
+			req, _ = http.NewRequest(http.MethodPost, "/containers/123/restart", nil)
+			// call the API to check if it returns the error generated from restart method
+			router.ServeHTTP(rr, req)
+			Expect(rr).Should(HaveHTTPStatus(http.StatusInternalServerError))
+			Expect(rr.Body).Should(MatchJSON(`{"message": "error from restart api"}`))
+		})
 		It("should call container create method", func() {
 			// setup mocks
 			body := []byte(`{"Image": "test-image"}`)

api/handlers/container/restart.go

@@ -0,0 +1,42 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package container
+
+import (
+	"net/http"
+	"strconv"
+	"time"
+
+	"github.com/containerd/containerd/namespaces"
+	"github.com/gorilla/mux"
+	"github.com/runfinch/finch-daemon/api/response"
+	"github.com/runfinch/finch-daemon/pkg/errdefs"
+)
+
+func (h *handler) restart(w http.ResponseWriter, r *http.Request) {
+	cid := mux.Vars(r)["id"]
+	t, err := strconv.ParseInt(r.URL.Query().Get("t"), 10, 64)
+	if err != nil {
+		t = 10 // Docker/nerdctl default
+	}
+	timeout := time.Second * time.Duration(t)
+
+	ctx := namespaces.WithNamespace(r.Context(), h.Config.Namespace)
+	err = h.service.Restart(ctx, cid, timeout)
+	// map the error into http status code and send response.
+	if err != nil {
+		var code int
+		switch {
+		case errdefs.IsNotFound(err):
+			code = http.StatusNotFound
+		default:
+			code = http.StatusInternalServerError
+		}
+		h.logger.Debugf("Restart container API responding with error code. Status code %d, Message: %s", code, err.Error())
+		response.SendErrorResponse(w, code, err)
+		return
+	}
+	// successfully restarted the container. Send no content status.
+	response.Status(w, http.StatusNoContent)
+}

api/handlers/container/restart_test.go

@@ -0,0 +1,76 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package container
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+
+	"github.com/containerd/nerdctl/pkg/config"
+	"github.com/golang/mock/gomock"
+	"github.com/gorilla/mux"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"github.com/runfinch/finch-daemon/mocks/mocks_container"
+	"github.com/runfinch/finch-daemon/mocks/mocks_logger"
+	"github.com/runfinch/finch-daemon/pkg/errdefs"
+)
+
+var _ = Describe("Container Restart API ", func() {
+	var (
+		mockCtrl *gomock.Controller
+		logger   *mocks_logger.Logger
+		service  *mocks_container.MockService
+		h        *handler
+		rr       *httptest.ResponseRecorder
+		req      *http.Request
+	)
+	BeforeEach(func() {
+		mockCtrl = gomock.NewController(GinkgoT())
+		defer mockCtrl.Finish()
+		logger = mocks_logger.NewLogger(mockCtrl)
+		service = mocks_container.NewMockService(mockCtrl)
+		c := config.Config{}
+		h = newHandler(service, &c, logger)
+		rr = httptest.NewRecorder()
+		req, _ = http.NewRequest(http.MethodPost, "/containers/123/restart", nil)
+		req = mux.SetURLVars(req, map[string]string{"id": "123"})
+	})
+	Context("handler", func() {
+		It("should return 204 as success response", func() {
+			// service mock returns nil to mimic handler started the container successfully.
+			service.EXPECT().Restart(gomock.Any(), "123", gomock.Any()).Return(nil)
+
+			//handler should return success message with 204 status code.
+			h.restart(rr, req)
+			Expect(rr).Should(HaveHTTPStatus(http.StatusNoContent))
+		})
+
+		It("should return 404 not found response", func() {
+			// service mock returns not found error to mimic user trying to start container that does not exist
+			service.EXPECT().Restart(gomock.Any(), "123", gomock.Any()).Return(
+				errdefs.NewNotFound(fmt.Errorf("container not found")))
+			logger.EXPECT().Debugf("Restart container API responding with error code. Status code %d, Message: %s", 404, "container not found")
+
+			//handler should return 404 status code with an error msg.
+			h.restart(rr, req)
+			Expect(rr).Should(HaveHTTPStatus(http.StatusNotFound))
+			Expect(rr.Body).Should(MatchJSON(`{"message": "container not found"}`))
+		})
+		It("should return 500 internal error response", func() {
+			// service mock return error to mimic a user trying to start a container with an id that has
+			// multiple containers with same prefix.
+			service.EXPECT().Restart(gomock.Any(), "123", gomock.Any()).Return(
+				fmt.Errorf("multiple IDs found with provided prefix"))
+			logger.EXPECT().Debugf("Restart container API responding with error code. Status code %d, Message: %s", 500, "multiple IDs found with provided prefix")
+
+			//handler should return 500 status code with an error msg.
+			h.restart(rr, req)
+			Expect(rr).Should(HaveHTTPStatus(http.StatusInternalServerError))
+			Expect(rr.Body).Should(MatchJSON(`{"message": "multiple IDs found with provided prefix"}`))
+		})
+	})
+})

e2e/e2e_test.go

@@ -40,6 +40,7 @@ func TestRun(t *testing.T) {
 		tests.ContainerCreate(opt)
 		tests.ContainerStart(opt)
 		tests.ContainerStop(opt)
+		tests.ContainerRestart(opt)
 		tests.ContainerRemove(opt)
 		tests.ContainerList(opt)
 		tests.ContainerRename(opt)

e2e/tests/container_restart.go

@@ -0,0 +1,106 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package tests
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/runfinch/common-tests/command"
+	"github.com/runfinch/common-tests/option"
+	"github.com/runfinch/finch-daemon/api/response"
+	"github.com/runfinch/finch-daemon/e2e/client"
+)
+
+// ContainerRestart tests the `POST containers/{id}/restart` API.
+func ContainerRestart(opt *option.Option) {
+	Describe("restart a container", func() {
+		var (
+			uClient *http.Client
+			version string
+		)
+		BeforeEach(func() {
+			command.Run(opt, "run", "-d", "--name", testContainerName, defaultImage,
+				"/bin/sh", "-c", `date; sleep infinity`)
+			// create a custom client to use http over unix sockets
+			uClient = client.NewClient(GetDockerHostUrl())
+			// get the docker api version that will be tested
+			version = GetDockerApiVersion()
+		})
+		AfterEach(func() {
+			command.RemoveAll(opt)
+		})
+
+		It("should start and restart the container", func() {
+			containerShouldBeRunning(opt, testContainerName)
+
+			before := time.Now().Round(0)
+
+			restartRelativeUrl := fmt.Sprintf("/containers/%s/restart", testContainerName)
+			res, err := uClient.Post(client.ConvertToFinchUrl(version, restartRelativeUrl), "application/json", nil)
+			Expect(err).Should(BeNil())
+			Expect(res.StatusCode).Should(Equal(http.StatusNoContent))
+
+			logsRelativeUrl := fmt.Sprintf("/containers/%s/logs", testContainerName)
+			opts := "?stdout=1" +
+				"&stderr=0" +
+				"&follow=0" +
+				"&tail=0"
+			res, err = uClient.Get(client.ConvertToFinchUrl(version, logsRelativeUrl+opts))
+			Expect(err).Should(BeNil())
+			body, err := io.ReadAll(res.Body)
+			Expect(err).Should(BeNil())
+
+			dateStr := string(body[8 : len(body)-1])
+			date, _ := time.Parse(time.UnixDate, dateStr)
+			Expect(before.Before(date)).Should(BeTrue())
+		})
+		It("should fail to restart container that does not exist", func() {
+			// restart a container that does not exist
+			relativeUrl := client.ConvertToFinchUrl(version, "/containers/container-does-not-exist/restart")
+			res, err := uClient.Post(relativeUrl, "application/json", nil)
+			Expect(err).Should(BeNil())
+			Expect(res.StatusCode).Should(Equal(http.StatusNotFound))
+			var errResponse response.Error
+			err = json.NewDecoder(res.Body).Decode(&errResponse)
+			Expect(err).Should(BeNil())
+			Expect(errResponse.Message).Should(Not(BeEmpty()))
+		})
+		It("should restart a stopped container", func() {
+			containerShouldBeRunning(opt, testContainerName)
+
+			stopRelativeUrl := fmt.Sprintf("/containers/%s/stop", testContainerName)
+			res, err := uClient.Post(client.ConvertToFinchUrl(version, stopRelativeUrl), "application/json", nil)
+			Expect(err).Should(BeNil())
+			Expect(res.StatusCode).Should(Equal(http.StatusNoContent))
+			containerShouldNotBeRunning(opt, testContainerName)
+
+			restartRelativeUrl := fmt.Sprintf("/containers/%s/restart", testContainerName)
+			res, err = uClient.Post(client.ConvertToFinchUrl(version, restartRelativeUrl), "application/json", nil)
+			Expect(err).Should(BeNil())
+			Expect(res.StatusCode).Should(Equal(http.StatusNoContent))
+			containerShouldBeRunning(opt, testContainerName)
+		})
+		It("should restart the container with timeout", func() {
+			containerShouldBeRunning(opt, testContainerName)
+
+			// stop the container with a timeout of 5 seconds
+			now := time.Now()
+			restartRelativeUrl := fmt.Sprintf("/containers/%s/restart?t=5", testContainerName)
+			res, err := uClient.Post(client.ConvertToFinchUrl(version, restartRelativeUrl), "application/json", nil)
+			later := time.Now()
+			Expect(err).Should(BeNil())
+			Expect(res.StatusCode).Should(Equal(http.StatusNoContent))
+			elapsed := later.Sub(now)
+			Expect(elapsed.Seconds()).Should(BeNumerically(">", 4.0))
+			Expect(elapsed.Seconds()).Should(BeNumerically("<", 10.0))
+			containerShouldBeRunning(opt, testContainerName)
+		})
+	})
+}

internal/service/container/restart.go

@@ -0,0 +1,32 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package container
+
+import (
+	"context"
+	"time"
+
+	"github.com/runfinch/finch-daemon/pkg/errdefs"
+)
+
+func (s *service) Restart(ctx context.Context, cid string, timeout time.Duration) error {
+	con, err := s.getContainer(ctx, cid)
+	if err != nil {
+		return err
+	}
+
+	// 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, &timeout); err != nil && !errdefs.IsNotModified(err) {
+		s.logger.Errorf("Failed to stop container: %s. Error: %v", cid, err)
+		return err
+	}
+	if err = s.nctlContainerSvc.StartContainer(ctx, con); err != nil {
+		s.logger.Errorf("Failed to start container: %s. Error: %v", cid, err)
+		return err
+	}
+	s.logger.Debugf("successfully restarted: %s", cid)
+	return nil
+}