Add a system test to modify and import an exported container #11426

ttsuuubasa · 2021-09-03T05:20:46Z

Podman system test has a TODO list written in test/system/TODO.md.
This requesting test completed "Implied pull, build, export, modify, import, tag, run, kill" in the list.

The workflows written in TODO.md have multiple commands and therefore
the test file has been created apart from others as 800-scenario.bats.

800-scenario.bats has the workflow like the following,
and is different from the existing test 125-import.bats from the point of view of a purpose.
800-scenario.bats is more systematic than 125-import.bats
because it is not composed of a single command but user workflow oriented.

Please add this test 800-scenario.bats to the system tests.
Additionally, I would like to confirm that TODO.md is still valid.

(*) This test has the following WORKFLOW

Build from Dockerfile FROM non-existing local image
Export built container as tarball
Modify tarball contents by adding a file into tarball and deleting a file from tarball
Import tarball
Tag imported image
Run imported image to stop with SIGINT like not SIGTERM or SIGKILL
Kill can send non-TERM/KILL signal to container to exit
Confirm exit

vrothberg

Thank you for contributing!

The file should probably be called 800-import.bats since scenario seems a bit generic.

I just have a couple of nits. @edsantiago is the mastermind behind the system tests, PTAL :)

vrothberg · 2021-09-03T09:37:53Z

test/system/800-scenario.bats

+
+load helpers
+
+alpine="quay.io/libpod/alpine"


There's already an internal variable $IMAGE which other tests are using. I suggest using that instead of $alpine to reduce the number of pulls and hence speed up the test a bit.

vrothberg · 2021-09-03T09:38:12Z

test/system/800-scenario.bats

+  dockerfile=$PODMAN_TMPDIR/Dockerfile
+
+  cat >$dockerfile <<EOF
+FROM $alpine


FROM $IMAGE

vrothberg · 2021-09-03T09:40:55Z

test/system/800-scenario.bats

+  run_podman ps -a --filter name=$a_cnt --format '{{.Status}}'
+  is "$output" "Exited (33)" "Exit by non-TERM/KILL"
+
+  run_podman rmi -fa


Removing all images may be a bit too much. It would be nice if we could keep $IMAGE in the local storage.

vrothberg · 2021-09-03T09:41:29Z

test/system/800-scenario.bats

+
+alpine="quay.io/libpod/alpine"
+
+@test "Implied pull, build, export, modify, import, tag, run, kill" {


Maybe "podman import / export" ?

vrothberg · 2021-09-03T09:42:47Z

test/system/800-scenario.bats

+
+  # Build from Dockerfile FROM non-existing local image
+  run_podman build -t $b_img $PODMAN_TMPDIR
+  run_podman run -d --name $b_cnt --stop-signal SIGKILL $b_img sleep 60


Using podman create instead of run would be cheaper.

vrothberg · 2021-09-03T09:43:18Z

test/system/800-scenario.bats

+
+  # Export built container as tarball
+  run_podman export -o $PODMAN_TMPDIR/$b_cnt.tar $b_cnt
+  run_podman rm -fa


Removing all containers seems a bit too much. Could we just remove $b_cnt?

vrothberg · 2021-09-03T09:45:25Z

test/system/800-scenario.bats

+  iid=$output
+
+  # Tag imported image
+  run_podman tag $iid $a_img


Note: podman import $PODMAN_TMPDIR/$b_cnt.tar ... $a_img would tag the imported image.

vrothberg · 2021-09-03T09:48:12Z

test/system/800-scenario.bats

+
+  # Kill can send non-TERM/KILL signal to container to exit
+  run_podman kill --signal 2 $a_cnt 
+  run_podman wait $a_cnt


Going via podman logs works but I think that using podman exec may be easier. We could start a container, then do podman exec $ctr cat ... and check the output.

I'd expect it to be less work and easier to brain parse.

ttsuuubasa · 2021-09-06T11:24:52Z

@vrothberg　Thank you for your comment.
The test for "import" command has already existed as 125-import.bats.
Is it better to include this test to 125-import.bats?
I has modified the code based on your comment.

vrothberg · 2021-09-06T11:28:19Z

Thank you, @fj-tsubasa!

Is it better to include this test to 125-import.bats?

I think so, yes. But I suggest to wait for @edsantiago's review first.

edsantiago

This doesn't actually work with TMPDIR=/something-other-than-slash-tmp. Suggestion:

index c738d4433..29fd1c03c 100644
--- a/test/system/800-scenario.bats
+++ b/test/system/800-scenario.bats
@@ -9,8 +9,9 @@ load helpers
 @test "podman import / export" {

   # Create a test file following test
+    mkdir $PODMAN_TMPDIR/tmp
   touch $PODMAN_TMPDIR/testfile1
-  echo "modified tar file" >> $PODMAN_TMPDIR/testfile2
+  echo "modified tar file" >> $PODMAN_TMPDIR/tmp/testfile2

   # Create Dockerfile for test
   dockerfile=$PODMAN_TMPDIR/Dockerfile
@@ -36,7 +37,7 @@ EOF

   # Modify tarball contents
   tar --delete -f $PODMAN_TMPDIR/$b_cnt.tar tmp/testfile1
-  tar -rf $PODMAN_TMPDIR/$b_cnt.tar $PODMAN_TMPDIR/testfile2
+  tar -C $PODMAN_TMPDIR -rf $PODMAN_TMPDIR/$b_cnt.tar tmp/testfile2

   # Import tarball and Tag imported image
   run_podman import -q $PODMAN_TMPDIR/$b_cnt.tar \
@@ -48,11 +49,11 @@ EOF
   run_podman run --name $a_cnt -d $a_img

   # Confirm testfile1 is deleted from tarball
-  run_podman 1 exec $a_cnt cat $PODMAN_TMPDIR/testfile1 2>&1
-  is "$output" ".*can't open '$PODMAN_TMPDIR/testfile1': No such file or directory"
+  run_podman 1 exec $a_cnt cat /tmp/testfile1
+  is "$output" ".*can't open '/tmp/testfile1': No such file or directory"

   # Confirm testfile2 is added to tarball
-  run_podman exec $a_cnt cat $PODMAN_TMPDIR/testfile2
+  run_podman exec $a_cnt cat /tmp/testfile2
   is "$output" "modified tar file" "modify tarball content"

   # Kill can send non-TERM/KILL signal to container to exit

I agree with your suggestion to include this in 125-import, or at least somewhere in the 12x space (with a better name than "scenario").

I am OOTO on Tuesday and cannot follow up until late at night or Wednesday morning - sorry.

This is very nice work, thank you!

edsantiago · 2021-09-07T01:32:38Z

test/system/800-scenario.bats

+  run_podman run --name $a_cnt -d $a_img
+
+  # Confirm testfile1 is deleted from tarball
+  run_podman 1 exec $a_cnt cat $PODMAN_TMPDIR/testfile1 2>&1


The 2>&1 here is unnecessary (and misleading)

edsantiago · 2021-09-07T01:40:32Z

test/system/800-scenario.bats

+
+@test "podman import / export" {
+
+  # Create a test file following test


For consistency with every other test script, would you mind using 4-space indentation?

edsantiago · 2021-09-07T01:54:49Z

test/system/800-scenario.bats

+
+load helpers
+
+@test "podman import / export" {


How about: @test "podman export, alter tarball, re-import" ?

ttsuuubasa · 2021-09-08T10:16:12Z

@edsantiago Thank you for your comment !!
I has included my test 800-scenario.bats into 125-import.bats, and
I has modified the code based on your comment.

edsantiago · 2021-09-08T11:36:44Z

LGTM but please squash your commits; that should fix the long-commit and trailing-whitespace errors.

This test has completed one of TODO items in test/system/TODO.md. The item is "Implied pull, build, export, modify, import, tag, run, kill" Signed-off-by: Tsubasa Watanabe <[email protected]>

openshift-ci · 2021-09-09T20:09:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, fj-tsubasa

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [edsantiago]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

edsantiago · 2021-09-09T20:10:15Z

/lgtm

Thank you, @fj-tsubasa !

ttsuuubasa · 2021-09-10T02:10:50Z

@edsantiago @vrothberg
Thank you for your acceptation and support for my first commit to OSS.
I think it is OK to delete the item of TODO.md "Implied pull, build, export, modify, import, tag, run, kill".

vrothberg · 2021-09-10T09:46:17Z

An absolute pleasure, @fj-tsubasa. Thank you for contributing!

vrothberg reviewed Sep 3, 2021

View reviewed changes

edsantiago suggested changes Sep 7, 2021

View reviewed changes

ttsuuubasa force-pushed the system-test-scenario branch from 576954e to 4ccb4f8 Compare September 9, 2021 08:48

Add a system test to modify and import an exported container.

4ccb4f8

This test has completed one of TODO items in test/system/TODO.md. The item is "Implied pull, build, export, modify, import, tag, run, kill" Signed-off-by: Tsubasa Watanabe <[email protected]>

edsantiago approved these changes Sep 9, 2021

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 9, 2021

openshift-ci bot assigned edsantiago Sep 9, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2021

openshift-merge-robot merged commit 63f6656 into containers:main Sep 9, 2021

ttsuuubasa deleted the system-test-scenario branch September 10, 2021 02:08

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a system test to modify and import an exported container #11426

Add a system test to modify and import an exported container #11426

ttsuuubasa commented Sep 3, 2021

vrothberg left a comment

vrothberg Sep 3, 2021

vrothberg Sep 3, 2021

vrothberg Sep 3, 2021

vrothberg Sep 3, 2021

vrothberg Sep 3, 2021

vrothberg Sep 3, 2021

vrothberg Sep 3, 2021

vrothberg Sep 3, 2021

ttsuuubasa commented Sep 6, 2021

vrothberg commented Sep 6, 2021

edsantiago left a comment

edsantiago Sep 7, 2021

edsantiago Sep 7, 2021

edsantiago Sep 7, 2021

ttsuuubasa commented Sep 8, 2021

edsantiago commented Sep 8, 2021

openshift-ci bot commented Sep 9, 2021

edsantiago commented Sep 9, 2021

ttsuuubasa commented Sep 10, 2021 •

edited

Loading

vrothberg commented Sep 10, 2021


		alpine="quay.io/libpod/alpine"

		@test "Implied pull, build, export, modify, import, tag, run, kill" {


		@test "podman import / export" {

		# Create a test file following test

Add a system test to modify and import an exported container #11426

Add a system test to modify and import an exported container #11426

Conversation

ttsuuubasa commented Sep 3, 2021

vrothberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttsuuubasa commented Sep 6, 2021

vrothberg commented Sep 6, 2021

edsantiago left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttsuuubasa commented Sep 8, 2021

edsantiago commented Sep 8, 2021

openshift-ci bot commented Sep 9, 2021

edsantiago commented Sep 9, 2021

ttsuuubasa commented Sep 10, 2021 • edited Loading

vrothberg commented Sep 10, 2021

ttsuuubasa commented Sep 10, 2021 •

edited

Loading