Skip to content

Commit

Permalink
fix: remove FINCH_DOCKER_COMPAT
Browse files Browse the repository at this point in the history 
Signed-off-by: Gavin Inglis <[email protected]>
  • Loading branch information
@ginglis13
ginglis13 committed Aug 8, 2023
1 parent 55b5235 commit bb53b54
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 195 deletions.
54 changes: 0 additions & 54 deletions .github/workflows/ci.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,60 +139,6 @@ jobs:
git clean -f -d
REGISTRY=${{ steps.vars.outputs.has_creds == true && env.REGISTRY || '' }} make test-e2e
shell: zsh {0}
e2e-tests-for-docker-compat:
strategy:
fail-fast: false
matrix:
os:
[
[self-hosted, macos, amd64, 13, test],
[self-hosted, macos, arm64, 13, test],
]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v3
with:
# We need to get all the git tags to make version injection work. See VERSION in Makefile for more detail.
fetch-depth: 0
persist-credentials: false
submodules: true
- name: Set output variables
id: vars
run: |
has_creds=${{ github.event_name == 'push' || github.repository == github.event.pull_request.head.repo.full_name }}
echo "has_creds=$has_creds" >> $GITHUB_OUTPUT
- name: configure aws credentials
uses: aws-actions/[email protected]
if: steps.vars.outputs.has_creds == true
with:
role-to-assume: ${{ secrets.ROLE }}
role-session-name: credhelper-test-docker-compat
aws-region: ${{ secrets.REGION }}
- name: Clean up previous files
run: |
sudo rm -rf /opt/finch
sudo rm -rf ~/.finch
sudo rm -rf ./_output
if pgrep '^qemu-system'; then
sudo pkill '^qemu-system'
fi
if pgrep '^socket_vmnet'; then
sudo pkill '^socket_vmnet'
fi
- name: Install Rosetta 2
run: echo "A" | softwareupdate --install-rosetta || true
- run: brew install go lz4 automake autoconf libtool
shell: zsh {0}
- name: Build project
run: |
export PATH="/opt/homebrew/opt/libtool/libexec/gnubin:$PATH"
make
shell: zsh {0}
- run: |
git status
git clean -f -d
FINCH_DOCKER_COMPAT=1 REGISTRY=${{ steps.vars.outputs.has_creds == true && env.REGISTRY || '' }} make test-e2e
shell: zsh {0}
mdlint:
runs-on: ubuntu-latest
steps:
Expand Down
87 changes: 0 additions & 87 deletions cmd/finch/nerdctl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ package main
import (
"bufio"
"fmt"
"os"
"path/filepath"
"sort"
"strings"

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

dockerCompat := isDockerCompatEnvSet(nc.systemDeps)
firstVolumeIndex := -1

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 @@ -127,34 +121,11 @@ 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"):
// TODO: remove the case branch when the Nerdctl fix is released to Finch.
// Nerdctl tracking issue: https://github.com/containerd/nerdctl/issues/2254.
if dockerCompat {
if firstVolumeIndex == -1 {
firstVolumeIndex = i
}
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 {
// Need to insert to the middle to prevent messing up the subcommands at the beginning and the image ref at the end.
// The first volume index is a good middle position.
volumes = sortVolumes(volumes)
for i, vol := range volumes {
position := firstVolumeIndex + i*2
nerdctlArgs = append(nerdctlArgs[:position], append([]string{"-v", vol}, nerdctlArgs[position:]...)...)
}
}

// 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 @@ -334,20 +305,6 @@ func handleEnvFile(fs afero.Fs, systemDeps NerdctlCommandSystemDeps, arg, arg2 s
return skip, envs, nil
}

// getVolume extracts the volume string from a command line argument.
// It handles both "--volume=" and "-v=" prefixes.
// The function returns the extracted volume string and a boolean value indicating whether the volume was found in the next argument.
// If the volume string was found in the provided 'arg' string (with "--volume=" or "-v=" prefix), it returns the volume and 'false'.
// If the volume string wasn't in the 'arg', it assumes it's in the 'nextArg' string and returns 'nextArg' and 'true'.
func getVolume(arg, nextArg string) (string, bool) {
for _, prefix := range []string{"--volume=", "-v="} {
if strings.HasPrefix(arg, prefix) {
return arg[len(prefix):], 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 @@ -361,50 +318,6 @@ func resolveIP(host string, logger flog.Logger) string {
return host
}

func sortVolumes(volumes []string) []string {
type volume struct {
original string
destination 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],
})
} else {
// Still need to put the volume string back if it is in other format.
volumeSlices = append(volumeSlices, volume{
original: vol,
destination: "",
})
}
}
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
Loading

0 comments on commit bb53b54

Please sign in to comment.