Merge pull request #2211 from HusterWan/zr/fix-upgrade

refactor: re-design upgrade feature

apis/swagger.yml

@@ -2486,15 +2486,27 @@ definitions:
 
   ContainerUpgradeConfig:
     description: |
-      ContainerUpgradeConfig is used for API "POST /containers/upgrade".
-      It wraps all kinds of config used in container upgrade.
-      It can be used to encode client params in client and unmarshal request body in daemon side.
-    allOf:
-      - $ref: "#/definitions/ContainerConfig"
-      - type: "object"
-        properties:
-          HostConfig:
-            $ref: "#/definitions/HostConfig"
+      ContainerUpgradeConfig is used for API "POST /containers/{name:.*}/upgrade". when upgrade a container,
+      we must specify new image used to create a new container, and also can specify `Cmd` and `Entrypoint` for
+      new container. There is all parameters that upgrade a container, if want to change other parameters, i
+      think you should use `update` API interface.
+    required: [Image]
+    properties:
+      Image:
+        type: "string"
+        x-nullable: false
+      Cmd:
+        type: "array"
+        description: "Execution commands and args"
+        items:
+          type: "string"
+      Entrypoint:
+        description: |
+          The entrypoint for the container as a string or an array of strings.
+          If the array consists of exactly one empty string (`[""]`) then the entry point is reset to system default.
+        type: "array"
+        items:
+          type: "string"
 
   LogConfig:
     description: "The logging configuration for this container"

cli/upgrade.go

@@ -3,24 +3,31 @@ package main
 import (
 	"context"
 	"fmt"
+	"strings"
+
+	"github.com/alibaba/pouch/apis/types"
 
 	"github.com/spf13/cobra"
 )
 
 // upgradeDescription is used to describe upgrade command in detail and auto generate command doc.
-var upgradeDescription = "Upgrade a container with new image and args"
+var upgradeDescription = "upgrade is a feature to replace a container's image." +
+	"You can specify the new Entrypoint and Cmd for the new container. When you want to update" +
+	"a container's image, but inherit the network and volumes of the old container, then you should" +
+	"think about the upgrade feature."
 
 // UpgradeCommand use to implement 'upgrade' command, it is used to upgrade a container.
 type UpgradeCommand struct {
 	baseCommand
-	*container
+	entrypoint string
+	image      string
 }
 
 // Init initialize upgrade command.
 func (ug *UpgradeCommand) Init(c *Cli) {
 	ug.cli = c
 	ug.cmd = &cobra.Command{
-		Use:   "upgrade [OPTIONS] IMAGE [COMMAND] [ARG...]",
+		Use:   "upgrade [OPTIONS] CONTAINER [COMMAND] [ARG...]",
 		Short: "Upgrade a container with new image and args",
 		Long:  upgradeDescription,
 		Args:  cobra.MinimumNArgs(1),
@@ -36,47 +43,52 @@ func (ug *UpgradeCommand) Init(c *Cli) {
 func (ug *UpgradeCommand) addFlags() {
 	flagSet := ug.cmd.Flags()
 	flagSet.SetInterspersed(false)
-
-	c := addCommonFlags(flagSet)
-	ug.container = c
+	flagSet.StringVar(&ug.entrypoint, "entrypoint", "", "Overwrite the default ENTRYPOINT of the image")
+	flagSet.StringVar(&ug.image, "image", "", "Specify image of the new container")
 }
 
 // runUpgrade is the entry of UpgradeCommand command.
 func (ug *UpgradeCommand) runUpgrade(args []string) error {
-	config, err := ug.config()
-	if err != nil {
-		return fmt.Errorf("failed to upgrade container: %v", err)
-	}
+	var cmd []string
 
-	config.Image = args[0]
+	name := args[0]
+	if name == "" {
+		return fmt.Errorf("failed to upgrade container: must specify container name")
+	}
 	if len(args) > 1 {
-		config.Cmd = args[1:]
+		cmd = args[1:]
 	}
 
-	containerName := ug.name
-	if containerName == "" {
-		return fmt.Errorf("failed to upgrade container: must specify container name")
+	image := ug.image
+	if image == "" {
+		return fmt.Errorf("failed to upgrade container: must specify new image")
+	}
+
+	upgradeConfig := &types.ContainerUpgradeConfig{
+		Image:      image,
+		Cmd:        cmd,
+		Entrypoint: strings.Fields(ug.entrypoint),
 	}
 
 	ctx := context.Background()
 	apiClient := ug.cli.Client()
 
-	if err := pullMissingImage(ctx, apiClient, config.Image, false); err != nil {
+	if err := pullMissingImage(ctx, apiClient, image, false); err != nil {
 		return err
 	}
 
-	err = apiClient.ContainerUpgrade(ctx, containerName, config.ContainerConfig, config.HostConfig)
-	if err == nil {
-		fmt.Println(containerName)
+	if err := apiClient.ContainerUpgrade(ctx, name, upgradeConfig); err != nil {
+		return err
 	}
 
-	return err
+	fmt.Println(name)
+	return nil
 }
 
 //upgradeExample shows examples in exec command, and is used in auto-generated cli docs.
 func upgradeExample() string {
-	return ` $ pouch run -d -m 20m --name test1  registry.hub.docker.com/library/busybox:latest
+	return ` $ pouch run -d -m 20m --name test  registry.hub.docker.com/library/busybox:latest
 4c58d27f58d38776dda31c01c897bbf554c802a9b80ae4dc20be1337f8a969f2
-$ pouch upgrade --name test1 registry.hub.docker.com/library/hello-world:latest
+$ pouch upgrade --image registry.hub.docker.com/library/hello-world:latest test
 test1`
 }

client/container_upgrade.go

@@ -8,12 +8,8 @@ import (
 )
 
 // ContainerUpgrade upgrade a container with new image and args.
-func (client *APIClient) ContainerUpgrade(ctx context.Context, name string, config types.ContainerConfig, hostConfig *types.HostConfig) error {
-	upgradeConfig := types.ContainerUpgradeConfig{
-		ContainerConfig: config,
-		HostConfig:      hostConfig,
-	}
-	resp, err := client.post(ctx, "/containers/"+name+"/upgrade", url.Values{}, upgradeConfig, nil)
+func (client *APIClient) ContainerUpgrade(ctx context.Context, name string, config *types.ContainerUpgradeConfig) error {
+	resp, err := client.post(ctx, "/containers/"+name+"/upgrade", url.Values{}, config, nil)
 	ensureCloseReader(resp)
 
 	return err

client/interface.go

@@ -36,7 +36,7 @@ type ContainerAPIClient interface {
 	ContainerPause(ctx context.Context, name string) error
 	ContainerUnpause(ctx context.Context, name string) error
 	ContainerUpdate(ctx context.Context, name string, config *types.UpdateConfig) error
-	ContainerUpgrade(ctx context.Context, name string, config types.ContainerConfig, hostConfig *types.HostConfig) error
+	ContainerUpgrade(ctx context.Context, name string, config *types.ContainerUpgradeConfig) error
 	ContainerTop(ctx context.Context, name string, arguments []string) (types.ContainerProcessList, error)
 	ContainerLogs(ctx context.Context, name string, options types.ContainerLogsOptions) (io.ReadCloser, error)
 	ContainerResize(ctx context.Context, name, height, width string) error

cri/v1alpha1/cri_utils.go

@@ -897,7 +897,7 @@ func (c *CriManager) getContainerMetrics(ctx context.Context, meta *mgr.Containe
 	}
 
 	// snapshot key may not equals container ID later
-	sn, err := c.SnapshotStore.Get(meta.ID)
+	sn, err := c.SnapshotStore.Get(meta.SnapshotID)
 	if err == nil {
 		usedBytes = sn.Size
 		inodesUsed = sn.Inodes

cri/v1alpha2/cri_utils.go

@@ -953,7 +953,7 @@ func (c *CriManager) getContainerMetrics(ctx context.Context, meta *mgr.Containe
 		return nil, fmt.Errorf("failed to get metadata of container %q: %v", meta.ID, err)
 	}
 
-	sn, err := c.SnapshotStore.Get(meta.ID)
+	sn, err := c.SnapshotStore.Get(meta.SnapshotID)
 	if err == nil {
 		usedBytes = sn.Size
 		inodesUsed = sn.Inodes

ctrd/container.go

@@ -490,10 +490,10 @@ func (c *Client) createContainer(ctx context.Context, ref, id, checkpointDir str
 		rootFSPath = container.BaseFS
 	} else { // containers created by pouch must first create snapshot
 		// check snapshot exist or not.
-		if _, err := c.GetSnapshot(ctx, id); err != nil {
+		if _, err := c.GetSnapshot(ctx, container.SnapshotID); err != nil {
 			return errors.Wrapf(err, "failed to create container %s", id)
 		}
-		options = append(options, containerd.WithSnapshot(id))
+		options = append(options, containerd.WithSnapshot(container.SnapshotID))
 	}
 
 	// specify Spec for new container

ctrd/container_types.go

@@ -11,12 +11,13 @@ import (
 // then create container by specifying the snapshot;
 // The other is create container by specify container rootfs, we use `RootFSProvided` flag to mark it,
 type Container struct {
-	ID      string
-	Image   string
-	Runtime string
-	Labels  map[string]string
-	IO      *containerio.IO
-	Spec    *specs.Spec
+	ID         string
+	Image      string
+	Runtime    string
+	Labels     map[string]string
+	IO         *containerio.IO
+	Spec       *specs.Spec
+	SnapshotID string
 
 	// BaseFS is rootfs used by containerd container
 	BaseFS string