mirror of https://github.com/containers/podman.git
Move Commit() to new parsing for --change
It turns out we had two independent parsing impkementations for Dockerfile instructions out of --change. My previous commit fixed the one used in --change, but as I discovered to my dismay, commit used a different implementation. Remove that and use the new parsing implementation instead. While we're at it, fix some bugs in the current commit code. The addition of anonymous named volumes to Libpod recently means we can now include those in the image config when committing. Some changes (VOLUME, ENV, EXPOSE, LABEL) previously cleared the config of the former image when used; Docker does not do this, so I removed that behavior. Still needs fixing: the new implementation does not support ONBUILD, while the old one did. Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This commit is contained in:
Matthew Heon
parent
001d06d7f6
commit
c4fbd2fc94
|
|
@ -3,7 +3,6 @@ package libpod
|
|||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"os"
|
||||
"strings"
|
||||
|
||||
"github.com/containers/buildah"
|
||||
|
|
@ -12,6 +11,7 @@ import (
|
|||
"github.com/containers/libpod/libpod/define"
|
||||
"github.com/containers/libpod/libpod/events"
|
||||
"github.com/containers/libpod/libpod/image"
|
||||
libpodutil "github.com/containers/libpod/pkg/util"
|
||||
"github.com/pkg/errors"
|
||||
"github.com/sirupsen/logrus"
|
||||
)
|
||||
|
|
@ -32,10 +32,6 @@ type ContainerCommitOptions struct {
|
|||
// Commit commits the changes between a container and its image, creating a new
|
||||
// image
|
||||
func (c *Container) Commit(ctx context.Context, destImage string, options ContainerCommitOptions) (*image.Image, error) {
|
||||
var (
|
||||
isEnvCleared, isLabelCleared, isExposeCleared, isVolumeCleared bool
|
||||
)
|
||||
|
||||
if c.config.Rootfs != "" {
|
||||
return nil, errors.Errorf("cannot commit a container that uses an exploded rootfs")
|
||||
}
|
||||
|
|
@ -51,7 +47,7 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
|
|||
|
||||
if c.state.State == define.ContainerStateRunning && options.Pause {
|
||||
if err := c.pause(); err != nil {
|
||||
return nil, errors.Wrapf(err, "error pausing container %q", c.ID())
|
||||
return nil, errors.Wrapf(err, "error pausing container %q to commit", c.ID())
|
||||
}
|
||||
defer func() {
|
||||
if err := c.unpause(); err != nil {
|
||||
|
|
@ -103,7 +99,7 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
|
|||
}
|
||||
// Expose ports
|
||||
for _, p := range c.config.PortMappings {
|
||||
importBuilder.SetPort(fmt.Sprintf("%d", p.ContainerPort))
|
||||
importBuilder.SetPort(fmt.Sprintf("%d/%s", p.ContainerPort, p.Protocol))
|
||||
}
|
||||
// Labels
|
||||
for k, v := range c.Labels() {
|
||||
|
|
@ -111,7 +107,9 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
|
|||
}
|
||||
// No stop signal
|
||||
// User
|
||||
importBuilder.SetUser(c.User())
|
||||
if c.config.User != "" {
|
||||
importBuilder.SetUser(c.config.User)
|
||||
}
|
||||
// Volumes
|
||||
if options.IncludeVolumes {
|
||||
for _, v := range c.config.UserVolumes {
|
||||
|
|
@ -119,107 +117,73 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai
|
|||
importBuilder.AddVolume(v)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Only include anonymous named volumes added by the user by
|
||||
// default.
|
||||
for _, v := range c.config.NamedVolumes {
|
||||
include := false
|
||||
for _, userVol := range c.config.UserVolumes {
|
||||
if userVol == v.Dest {
|
||||
include = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if include {
|
||||
vol, err := c.runtime.GetVolume(v.Name)
|
||||
if err != nil {
|
||||
return nil, errors.Wrapf(err, "volume %s used in container %s has been removed", v.Name, c.ID())
|
||||
}
|
||||
if vol.IsCtrSpecific() {
|
||||
importBuilder.AddVolume(v.Dest)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// Workdir
|
||||
importBuilder.SetWorkDir(c.Spec().Process.Cwd)
|
||||
importBuilder.SetWorkDir(c.config.Spec.Process.Cwd)
|
||||
|
||||
genCmd := func(cmd string) []string {
|
||||
trim := func(cmd []string) []string {
|
||||
if len(cmd) == 0 {
|
||||
return cmd
|
||||
}
|
||||
|
||||
retCmd := []string{}
|
||||
for _, c := range cmd {
|
||||
if len(c) >= 2 {
|
||||
if c[0] == '"' && c[len(c)-1] == '"' {
|
||||
retCmd = append(retCmd, c[1:len(c)-1])
|
||||
continue
|
||||
}
|
||||
}
|
||||
retCmd = append(retCmd, c)
|
||||
}
|
||||
return retCmd
|
||||
}
|
||||
if strings.HasPrefix(cmd, "[") {
|
||||
cmd = strings.TrimPrefix(cmd, "[")
|
||||
cmd = strings.TrimSuffix(cmd, "]")
|
||||
return trim(strings.Split(cmd, ","))
|
||||
}
|
||||
return []string{"/bin/sh", "-c", cmd}
|
||||
}
|
||||
// Process user changes
|
||||
for _, change := range options.Changes {
|
||||
splitChange := strings.SplitN(change, "=", 2)
|
||||
if len(splitChange) != 2 {
|
||||
splitChange = strings.SplitN(change, " ", 2)
|
||||
if len(splitChange) < 2 {
|
||||
return nil, errors.Errorf("invalid change %s format", change)
|
||||
}
|
||||
}
|
||||
|
||||
switch strings.ToUpper(splitChange[0]) {
|
||||
case "CMD":
|
||||
importBuilder.SetCmd(genCmd(splitChange[1]))
|
||||
case "ENTRYPOINT":
|
||||
importBuilder.SetEntrypoint(genCmd(splitChange[1]))
|
||||
case "ENV":
|
||||
change := strings.Split(splitChange[1], " ")
|
||||
name := change[0]
|
||||
val := ""
|
||||
if len(change) < 2 {
|
||||
change = strings.Split(change[0], "=")
|
||||
}
|
||||
if len(change) < 2 {
|
||||
var ok bool
|
||||
val, ok = os.LookupEnv(name)
|
||||
if !ok {
|
||||
return nil, errors.Errorf("invalid env variable %q: not defined in your environment", name)
|
||||
}
|
||||
} else {
|
||||
name = change[0]
|
||||
val = strings.Join(change[1:], " ")
|
||||
}
|
||||
if !isEnvCleared { // Multiple values are valid, only clear once.
|
||||
importBuilder.ClearEnv()
|
||||
isEnvCleared = true
|
||||
}
|
||||
importBuilder.SetEnv(name, val)
|
||||
case "EXPOSE":
|
||||
if !isExposeCleared { // Multiple values are valid, only clear once
|
||||
importBuilder.ClearPorts()
|
||||
isExposeCleared = true
|
||||
}
|
||||
importBuilder.SetPort(splitChange[1])
|
||||
case "LABEL":
|
||||
change := strings.Split(splitChange[1], " ")
|
||||
if len(change) < 2 {
|
||||
change = strings.Split(change[0], "=")
|
||||
}
|
||||
if len(change) < 2 {
|
||||
return nil, errors.Errorf("invalid label %s format, requires to NAME=VAL", splitChange[1])
|
||||
}
|
||||
if !isLabelCleared { // multiple values are valid, only clear once
|
||||
importBuilder.ClearLabels()
|
||||
isLabelCleared = true
|
||||
}
|
||||
importBuilder.SetLabel(change[0], strings.Join(change[1:], " "))
|
||||
case "ONBUILD":
|
||||
importBuilder.SetOnBuild(splitChange[1])
|
||||
case "STOPSIGNAL":
|
||||
// No Set StopSignal
|
||||
case "USER":
|
||||
importBuilder.SetUser(splitChange[1])
|
||||
case "VOLUME":
|
||||
if !isVolumeCleared { // multiple values are valid, only clear once
|
||||
importBuilder.ClearVolumes()
|
||||
isVolumeCleared = true
|
||||
}
|
||||
importBuilder.AddVolume(splitChange[1])
|
||||
case "WORKDIR":
|
||||
importBuilder.SetWorkDir(splitChange[1])
|
||||
}
|
||||
newImageConfig, err := libpodutil.GetImageConfig(options.Changes)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if newImageConfig.User != "" {
|
||||
importBuilder.SetUser(newImageConfig.User)
|
||||
}
|
||||
// EXPOSE only appends
|
||||
for port := range newImageConfig.ExposedPorts {
|
||||
importBuilder.SetPort(port)
|
||||
}
|
||||
// ENV only appends
|
||||
for _, env := range newImageConfig.Env {
|
||||
splitEnv := strings.SplitN(env, "=", 2)
|
||||
key := splitEnv[0]
|
||||
value := ""
|
||||
if len(splitEnv) == 2 {
|
||||
value = splitEnv[1]
|
||||
}
|
||||
importBuilder.SetEnv(key, value)
|
||||
}
|
||||
if newImageConfig.Entrypoint != nil {
|
||||
importBuilder.SetEntrypoint(newImageConfig.Entrypoint)
|
||||
}
|
||||
if newImageConfig.Cmd != nil {
|
||||
importBuilder.SetCmd(newImageConfig.Cmd)
|
||||
}
|
||||
// VOLUME only appends
|
||||
for vol := range newImageConfig.Volumes {
|
||||
importBuilder.AddVolume(vol)
|
||||
}
|
||||
if newImageConfig.WorkingDir != "" {
|
||||
importBuilder.SetWorkDir(newImageConfig.WorkingDir)
|
||||
}
|
||||
for k, v := range newImageConfig.Labels {
|
||||
importBuilder.SetLabel(k, v)
|
||||
}
|
||||
if newImageConfig.StopSignal != "" {
|
||||
importBuilder.SetStopSignal(newImageConfig.StopSignal)
|
||||
}
|
||||
|
||||
candidates, _, _, err := util.ResolveName(destImage, "", sc, c.runtime.store)
|
||||
if err != nil {
|
||||
return nil, errors.Wrapf(err, "error resolving name %q", destImage)
|
||||
|
|
|
|||
Loading…
Reference in New Issue