Skip to content

Commit

Permalink
fix: sort volume args in DOCKER_COMPAT mode
Browse files Browse the repository at this point in the history 
Signed-off-by: Ziwen Ning <ningziwe@amazon.com>
  • Loading branch information
@ningziwen
ningziwen committed May 26, 2023
1 parent bb1cab2 commit 66f0e6d
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 0 deletions.
71 changes: 71 additions & 0 deletions cmd/finch/nerdctl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ package main
import (
"bufio"
"fmt"
"os"
"path/filepath"
"sort"
"strings"

dockerops "github.com/docker/docker/opts"
Expand Down Expand Up @@ -85,8 +87,11 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
var (
nerdctlArgs, envs, fileEnvs []string
skip bool
volumes []string
)

dockerCompat := isDockerCompatEnvSet(nc.systemDeps)

for i, arg := range args {
// parsing environment values from the command line may pre-fetch and
// consume the next argument; this loop variable will skip these pre-consumed
Expand Down Expand Up @@ -121,10 +126,27 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
arg = fmt.Sprintf("%s%s", arg[0:11], resolvedIP)
}
nerdctlArgs = append(nerdctlArgs, arg)
case strings.HasPrefix(arg, "-v") || strings.HasPrefix(arg, "--volume"):
if dockerCompat {
volume, shouldSkip := getVolume(arg, args[i+1])
volumes = append(volumes, volume)
skip = shouldSkip
} else {
nerdctlArgs = append(nerdctlArgs, arg)
}
default:
nerdctlArgs = append(nerdctlArgs, arg)
}
}

if dockerCompat {
// Reverse sort to append to beginning.
volumes = reverseSortVolumes(volumes)
for _, vol := range volumes {
nerdctlArgs = append([]string{"-v", vol}, nerdctlArgs...)
}
}

// to handle environment variables properly, we add all entries found via
// env-file includes to the map first and then all command line environment
// flags, making sure that command line overrides environment file options,
Expand Down Expand Up @@ -300,6 +322,17 @@ func handleEnvFile(fs afero.Fs, systemDeps NerdctlCommandSystemDeps, arg, arg2 s
return skip, envs, nil
}

func getVolume(arg, nextArg string) (string, bool) {
if strings.Contains(arg, "=") {
if strings.HasPrefix(arg, "--volume=") {
return arg[9:], false
} else if strings.HasPrefix(arg, "-v=") {
return arg[3:], false
}
}
return nextArg, true
}

func resolveIP(host string, logger flog.Logger) string {
parts := strings.SplitN(host, ":", 2)
// If the IP Address is a string called "host-gateway", replace this value with the IP address that can be used to
Expand All @@ -313,6 +346,44 @@ func resolveIP(host string, logger flog.Logger) string {
return host
}

type volume struct {
original string
destination string
}

func reverseSortVolumes(volumes []string) []string {
volumeSlices := make([]volume, 0)
for _, vol := range volumes {
splitVol := strings.Split(vol, ":")
if len(splitVol) >= 2 {
volumeSlices = append(volumeSlices, volume{
original: vol,
destination: splitVol[1],
})
}
}
sort.Slice(volumeSlices, func(i, j int) bool {
// Consistent with the less function in Docker.
// /~https://github.com/moby/moby/blob/0db417451313474133c5ed62bbf95e2d3c92444d/daemon/volumes.go#L45
c1 := strings.Count(filepath.Clean(volumeSlices[i].destination), string(os.PathSeparator))
c2 := strings.Count(filepath.Clean(volumeSlices[j].destination), string(os.PathSeparator))
return c1 > c2
})
sortedVolumes := make([]string, 0)
for _, vol := range volumeSlices {
sortedVolumes = append(sortedVolumes, vol.original)
}
return sortedVolumes
}

// isDockerCompatEnvSet checks if the FINCH_DOCKER_COMPAT environment variable is set, so that callers can
// modify behavior to match docker instead of nerdctl.
// Intended to be used for temporary workarounds until changes are merged upstream.
func isDockerCompatEnvSet(systemDeps NerdctlCommandSystemDeps) bool {
_, s := systemDeps.LookupEnv("FINCH_DOCKER_COMPAT")
return s
}

var nerdctlCmds = map[string]string{
"build": "Build an image from Dockerfile",
"builder": "Manage builds",
Expand Down
47 changes: 47 additions & 0 deletions cmd/finch/nerdctl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func TestNerdctlCommand_runAdaptor(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "info").Return(c)
Expand Down Expand Up @@ -104,6 +105,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "build", "-t", "demo", ".").Return(c)
Expand Down Expand Up @@ -205,6 +207,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
logger.EXPECT().SetLevel(flog.Debug)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
Expand All @@ -229,6 +232,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
c := mocks.NewCommand(ctrl)
ncsd.EXPECT().LookupEnv("ARG2")
ncsd.EXPECT().LookupEnv("ARG3")
Expand All @@ -255,6 +259,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
c := mocks.NewCommand(ctrl)
ncsd.EXPECT().LookupEnv("ARG2")
ncsd.EXPECT().LookupEnv("ARG3").Return("val3", true)
Expand Down Expand Up @@ -284,6 +289,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
c := mocks.NewCommand(ctrl)
ncsd.EXPECT().LookupEnv("ARG2")
ncsd.EXPECT().LookupEnv("NOTSETARG")
Expand Down Expand Up @@ -314,6 +320,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
c := mocks.NewCommand(ctrl)
ncsd.EXPECT().LookupEnv("ARG2").Return("val2", true)
ncsd.EXPECT().LookupEnv("NOTSETARG")
Expand Down Expand Up @@ -341,6 +348,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
},
},
{
Expand All @@ -360,6 +368,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
logger.EXPECT().Debugf(`Resolving special IP "host-gateway" to %q for host %q`, "192.168.5.2", "name")
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
Expand All @@ -385,6 +394,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
Expand All @@ -409,6 +419,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
Expand All @@ -433,6 +444,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
logger.EXPECT().Debugf(`Resolving special IP "host-gateway" to %q for host %q`, "192.168.5.2", "name")
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
Expand All @@ -458,13 +470,43 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
"--rm", "--add-host=name:0.0.0.0", "alpine:latest").Return(c)
c.EXPECT().Run()
},
},
{
name: "with multiple nested volumes",
cmdName: "run",
args: []string{
"--rm", "-v", "/tmp:/tmp1/tmp2:rro", "--volume", "/tmp:/tmp1:rprivate,rro", "-v=/tmp:/:rro",
"--volume=/tmp:/tmp1/tmp3/tmp4:rshared", "alpine:latest",
},
wantErr: nil,
mockSvc: func(
t *testing.T,
lcc *mocks.LimaCmdCreator,
ncsd *mocks.NerdctlCommandSystemDeps,
logger *mocks.Logger,
ctrl *gomock.Controller,
fs afero.Fs,
) {
getVMStatusC := mocks.NewCommand(ctrl)
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("1", true)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run",
"-v", "/tmp:/:rro", "-v", "/tmp:/tmp1:rprivate,rro", "-v", "/tmp:/tmp1/tmp2:rro",
"-v", "/tmp:/tmp1/tmp3/tmp4:rshared", "--rm", "alpine:latest").Return(c)
c.EXPECT().Run()
},
},
{
name: "with --help flag",
cmdName: "pull",
Expand All @@ -482,6 +524,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
lcc.EXPECT().RunWithReplacingStdout(
testStdoutRs, "shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag", "--help").Return(nil)
Expand All @@ -504,6 +547,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false)
lcc.EXPECT().RunWithReplacingStdout(
testStdoutRs, "shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag", "--help").
Expand All @@ -527,6 +571,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("test", true)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", "COSIGN_PASSWORD=test", nerdctlCmdName,
Expand All @@ -551,6 +596,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("test", true)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", "COSIGN_PASSWORD=test", nerdctlCmdName,
Expand All @@ -575,6 +621,7 @@ func TestNerdctlCommand_run(t *testing.T) {
lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC)
getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil)
logger.EXPECT().Debugf("Status of virtual machine: %s", "Running")
ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false)
ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("test", true)
c := mocks.NewCommand(ctrl)
lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", "COSIGN_PASSWORD=test",
Expand Down

0 comments on commit 66f0e6d

Please sign in to comment.