Skip to content

Commit

Permalink
Ensure all layers are here when committing
Browse files Browse the repository at this point in the history
For context:
#3425
#3439

Signed-off-by: apostasie <[email protected]>
  • Loading branch information
apostasie committed Sep 19, 2024
1 parent 9d2effa commit 297233b
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 19 deletions.
46 changes: 31 additions & 15 deletions cmd/nerdctl/container/container_commit_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import (
"strings"
"testing"

"gotest.tools/v3/icmd"

"github.com/containerd/nerdctl/v2/pkg/testutil"
)

Expand All @@ -35,12 +33,11 @@ It will regularly succeed or fail, making random PR fail the Kube check.
func TestKubeCommitPush(t *testing.T) {
t.Parallel()

t.Skip("Test that confirm that #827 is still broken is too flaky")

base := testutil.NewBaseForKubernetes(t)
tID := testutil.Identifier(t)

var containerID string
// var registryIP string

setup := func() {
testutil.KubectlHelper(base, "run", "--image", testutil.CommonImage, tID, "--", "sleep", "Inf").
Expand All @@ -55,26 +52,45 @@ func TestKubeCommitPush(t *testing.T) {
cmd := testutil.KubectlHelper(base, "get", "pods", tID, "-o", "jsonpath={ .status.containerStatuses[0].containerID }")
cmd.Run()
containerID = strings.TrimPrefix(cmd.Out(), "containerd://")

// This below is missing configuration to allow for plain http communication
// This is left here for future work to successfully start a registry usable in the cluster
/*
// Start a registry
testutil.KubectlHelper(base, "run", "--port", "5000", "--image", testutil.RegistryImageStable, "testregistry").
AssertOK()
testutil.KubectlHelper(base, "wait", "pod", "testregistry", "--for=condition=ready", "--timeout=1m").
AssertOK()
cmd = testutil.KubectlHelper(base, "get", "pods", tID, "-o", "jsonpath={ .status.hostIPs[0].ip }")
cmd.Run()
registryIP = cmd.Out()
cmd = testutil.KubectlHelper(base, "apply", "-f", "-", fmt.Sprintf(`apiVersion: v1
kind: ConfigMap
metadata:
name: local-registry
namespace: nerdctl-test
data:
localRegistryHosting.v1: |
host: "%s:5000"
help: "https://kind.sigs.k8s.io/docs/user/local-registry/"
`, registryIP))
*/

}

tearDown := func() {
testutil.KubectlHelper(base, "delete", "pod", "-f", tID).Run()
testutil.KubectlHelper(base, "delete", "pod", "--all").Run()
}

tearDown()
t.Cleanup(tearDown)
setup()

t.Run("test commit / push on Kube (https://github.com/containerd/nerdctl/issues/827)", func(t *testing.T) {
t.Log("This test is meant to verify that we can commit / push an image from a pod." +
"Currently, this is broken, hence the test assumes it will fail. Once the problem is fixed, we should just" +
"change the expectation to 'success'.")

base.Cmd("commit", containerID, "registry.example.com/my-app:v1").AssertOK()
// See note above.
base.Cmd("push", "registry.example.com/my-app:v1").Assert(icmd.Expected{
ExitCode: 1,
Err: "failed to create a tmp single-platform image",
})
base.Cmd("commit", containerID, "testcommitsave").AssertOK()
base.Cmd("save", "testcommitsave").AssertOK()
})
}
19 changes: 18 additions & 1 deletion hack/build-integration-kubernetes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ readonly root

GO_VERSION=1.23
KIND_VERSION=v0.24.0
CNI_PLUGINS_VERSION=v1.5.1

[ "$(uname -m)" == "aarch64" ] && GOARCH=arm64 || GOARCH=amd64

Expand Down Expand Up @@ -53,6 +54,19 @@ install::kubectl(){
host::install "$temp"/kubectl
}

install::cni(){
local version="$1"
local temp
temp="$(fs::mktemp "install")"

http::get "$temp"/cni.tgz "https://github.com/containernetworking/plugins/releases/download/$version/cni-plugins-${GOOS:-linux}-${GOARCH:-amd64}-$version.tgz"
sudo mkdir -p /opt/cni/bin
sudo tar xzf "$temp"/cni.tgz -C /opt/cni/bin
mkdir -p ~/opt/cni/bin
tar xzf "$temp"/cni.tgz -C ~/opt/cni/bin
rm "$temp"/cni.tgz
}

exec::kind(){
local args=()
[ ! "$_rootful" ] || args=(sudo env PATH="$PATH" KIND_EXPERIMENTAL_PROVIDER="$KIND_EXPERIMENTAL_PROVIDER")
Expand Down Expand Up @@ -85,6 +99,9 @@ main(){
PATH=$(pwd)/_output:"$PATH"
export PATH

# Add CNI plugins
install::cni "$CNI_PLUGINS_VERSION"

# Hack to get go into kind control plane
exec::nerdctl rm -f go-kind 2>/dev/null || true
exec::nerdctl run -d --name go-kind golang:"$GO_VERSION" sleep Inf
Expand All @@ -97,4 +114,4 @@ main(){
exec::kind create cluster --name nerdctl-test --config=./hack/kind.yaml
}

main "$@"
main "$@"
2 changes: 2 additions & 0 deletions hack/kind.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ nodes:
containerPath: /usr/local/go
- hostPath: .
containerPath: /nerdctl-source
- hostPath: /opt/cni
containerPath: /opt/cni
2 changes: 1 addition & 1 deletion pkg/cmd/container/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func Commit(ctx context.Context, client *containerd.Client, rawRef string, req s
if found.MatchCount > 1 {
return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
}
imageID, err := commit.Commit(ctx, client, found.Container, opts)
imageID, err := commit.Commit(ctx, client, found.Container, opts, options.GOptions)
if err != nil {
return err
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/cmd/image/tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

containerd "github.com/containerd/containerd/v2/client"
"github.com/containerd/errdefs"
"github.com/containerd/log"

"github.com/containerd/nerdctl/v2/pkg/api/types"
"github.com/containerd/nerdctl/v2/pkg/idutil/imagewalker"
Expand Down Expand Up @@ -68,7 +69,8 @@ func Tag(ctx context.Context, client *containerd.Client, options types.ImageTagO

err = imgutil.EnsureAllContent(ctx, client, srcName, img.Target, "", options.GOptions)
if err != nil {
return err
log.G(ctx).Warn("Unable to fetch missing layers before tagging. " +
"If you try to save or push this image, it might fail. See https://github.com/containerd/nerdctl/issues/3439.")
}

if _, err = imageService.Create(ctx, img); err != nil {
Expand Down
10 changes: 9 additions & 1 deletion pkg/imgutil/commit/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"github.com/containerd/log"
"github.com/containerd/platforms"

"github.com/containerd/nerdctl/v2/pkg/api/types"
imgutil "github.com/containerd/nerdctl/v2/pkg/imgutil"
"github.com/containerd/nerdctl/v2/pkg/labels"
)
Expand All @@ -65,7 +66,7 @@ var (
emptyDigest = digest.Digest("")
)

func Commit(ctx context.Context, client *containerd.Client, container containerd.Container, opts *Opts) (digest.Digest, error) {
func Commit(ctx context.Context, client *containerd.Client, container containerd.Container, opts *Opts, options types.GlobalCommandOptions) (digest.Digest, error) {
id := container.ID()
info, err := container.Info(ctx)
if err != nil {
Expand Down Expand Up @@ -96,6 +97,13 @@ func Commit(ctx context.Context, client *containerd.Client, container containerd
return emptyDigest, err
}

// Ensure all the layers are here: https://github.com/containerd/nerdctl/issues/3425
err = imgutil.EnsureAllContent(ctx, client, baseImg.Name(), baseImg.Target(), "", options)
if err != nil {
log.G(ctx).Warn("Unable to fetch missing layers before tagging. " +
"If you try to save or push this image, it might fail. See https://github.com/containerd/nerdctl/issues/3439.")
}

if opts.Pause {
task, err := container.Task(ctx, cio.Load)
if err != nil {
Expand Down

0 comments on commit 297233b

Please sign in to comment.