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

Ensure all layers are here when tagging, committing, saving, or converting #3435

Merged
merged 6 commits into from
Oct 2, 2024
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
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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was not able / interested enough to finish setting up a cluster registry.
This here is a start, and can be completed later on, either by me or someone else.

// 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()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason that escapes me, kubectl was not cleaning up with the pod name

}

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()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This correctly reproduce issue #827 I believe.

})
}
Loading
Loading