fix API: Create container with an invalid configuration

[zhangguanzhang]

Signed-off-by: zhangguanzhang [email protected]
fix #6799
if use API to create containers, this will be null

        "Path": "",
        "Args": null,

[openshift-ci-robot]

Hi @zhangguanzhang. Thanks for your PR.

I'm waiting for a containers member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[rhatdan]

Tests are not happy.
/ok-to-test

[zhangguanzhang]

@TomSweeneyRedHat PTAL

[vrothberg]

@zhangguanzhang could you rebase and repush? I am under the impression that the CI had a hiccup.

[vrothberg]

I'd love to see tests for this in test/apiv2

[vrothberg]

Please squash the two commits into one. The merge commit isn't necessary. I usually do git pull --rebase upstream/master for rebasing.

Would you also add a test or two to test/apiv2? As it's a fix, we missed to test it in the CI and only tests will make sure we that we don't regress on it in the future.

Again thanks a lot for fixing!

[zhangguanzhang]

I'll try to add unit tests, and I'll submit them when I'm done

[edsantiago]

The tests take care of converting to JSON. The correct syntax here is:

t POST ... Image=$IMAGE

Also, the expected return status is 201, not 200

Also, please don't just check return code: add a check for reasonable status. Something like

.Id~[0-9a-f]\\{64\\}

Also, the /start isn't working, I'll let you figure that out.

See test/apiv2/README.md for instructions on how to run tests in your local environment.

[zhangguanzhang]

@edsantiago all has done:

[root@sz-test apiv2]# cat 20-zgz.at
podman pull $IMAGE &>/dev/null

# Unimplemented
#t POST libpod/containers/create '' 201 'sdf'


# Ensure that API does not occur: Create Container creates an invalid and the container fails to start
# https://github.com/containers/libpod/issues/6799
CNAME=testArgs
t POST libpod/containers/create?name=${CNAME} Image=${IMAGE} 201 \
  .Id~[0-9a-f]\\{64\\}
t GET libpod/containers/json?limit=1 200 \
  length=1 \
  .[0].Id~[0-9a-f]\\{64\\}
cid=$(jq -r '.[0].Id' <<<"$output")

t POST libpod/containers/${cid}/start '' 204
t DELETE libpod/containers/$cid 204

run the test in my local environment:

[root@sz-test apiv2]# ./test-apiv2 zgz
ok 1 [20-zgz] POST libpod/containers/create?name=testArgs [Image=quay.io/libpod/alpine_labels:latest] : status=201
ok 2 [20-zgz] POST libpod/containers/create?name=testArgs [Image=quay.io/libpod/alpine_labels:latest] : .Id ('26d9beb2f48c187183a721fda9fa2af33b3a3f37b6e2b0646a3817b4da1dd83b') ~ [0-9a-f]\{64\}
ok 3 [20-zgz] GET libpod/containers/json?limit=1 : status=200
ok 4 [20-zgz] GET libpod/containers/json?limit=1 : length=1
ok 5 [20-zgz] GET libpod/containers/json?limit=1 : .[0].Id ('26d9beb2f48c187183a721fda9fa2af33b3a3f37b6e2b0646a3817b4da1dd83b') ~ [0-9a-f]\{64\}
ok 6 [20-zgz] POST libpod/containers/26d9beb2f48c187183a721fda9fa2af33b3a3f37b6e2b0646a3817b4da1dd83b/start [] : status=204
ok 7 [20-zgz] DELETE libpod/containers/26d9beb2f48c187183a721fda9fa2af33b3a3f37b6e2b0646a3817b4da1dd83b : status=204
1..7

[zhangguanzhang]

@vrothberg ci error,but has no reason

[vrothberg]

Sorry, we had some rough time with our CI, mainly with the infrastructure. Could you rebase and repush?

[zhangguanzhang]

I will try later

[zhangguanzhang]

@vrothberg It still error

[vrothberg]

We're seeing it too. The infrastructure is sick but we're working on it. Thanks for your patience!

[zhangguanzhang]

Ok, please tell me when it's ready

[zhangguanzhang]

@rhatdan @vrothberg PTAL

[vrothberg]

LGTM, thanks a lot!

I want @edsantiago's ack on the test before merging.

[skorhone]

@zhangguanzhang If you have time, can you take a look why publish all ports and port randomization aren't working properly for v2 api.

If you create a container using api with just exposed port and publish all port flag set, ports won't be published.

[zhangguanzhang]

@skorhone I will take a look when I have time

[edsantiago]

tests LGTM and address my earlier concerns. Thanks!

[rhatdan]

/approve
/lgtm
Thanks @zhangguanzhang

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, zhangguanzhang

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 [rhatdan]

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