Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: re-design upgrade feature #2211

Merged
merged 2 commits into from
Oct 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions apis/swagger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2479,15 +2479,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"
Expand Down
92 changes: 50 additions & 42 deletions apis/types/container_upgrade_config.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

56 changes: 34 additions & 22 deletions cli/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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`
}
8 changes: 2 additions & 6 deletions client/container_upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion client/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cri/v1alpha1/cri_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cri/v1alpha2/cri_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions ctrd/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use combine the else and if the reduce the ident, right?

    } else if  _, err := c.GetSnapshot(ctx, container.SnapshotID); err != nil {
        return ......
    } else {
        options = append(options, containerd.WithSnapshot(container.SnapshotID))
    }

WDYT? @HusterWan

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid we cannot do that, we must first ensure the snapshot existence than we add a option

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, my mistake.

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
Expand Down
13 changes: 7 additions & 6 deletions ctrd/container_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading