fix: should not start a paused container

[Ace-Tang]

pouch should not start a paused container, or when create container
fail, pouch will do a clean to remove all container data in meta data.

func (mgr *ContainerManager) start(ctx context.Context, c *Container, options *types.ContainerStartOptions) error {
    var err error
    c.DetachKeys = options.DetachKeys

    attachedVolumes := map[string]struct{}{}
    defer func() {
        if err == nil {
            return
        }

        // release the container resources(network and containerio)
        err = mgr.releaseContainerResources(c)
        if err != nil {
            logrus.Errorf("failed to release container(%s) resources: %v", c.ID, err)
        }

        // detach the volumes
        for name := range attachedVolumes {
            if _, err = mgr.VolumeMgr.Detach(ctx, name, map[string]string{volumetypes.OptionRef: c.ID}); err != nil {
                logrus.Errorf("failed to detach volume(%s) when start container(%s) rollback: %v", name, c.ID, err)
            }
        }
    }()

reproduce:

pouch pause $cid
pouch start $cid
then container was removed by pouchd

Signed-off-by: Ace-Tang [email protected]

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

[codecov]

Codecov Report

Merging #2757 into master will decrease coverage by 0.07%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2757      +/-   ##
==========================================
- Coverage   69.42%   69.35%   -0.08%     
==========================================
  Files         277      277              
  Lines       17412    17414       +2     
==========================================
- Hits        12089    12077      -12     
- Misses       4002     4015      +13     
- Partials     1321     1322       +1

Flag	Coverage Δ
#criv1alpha2_test	39.36% <0%> (-0.04%)	⬇️
#integration_test_0	36.56% <0%> (-0.01%)	⬇️
#integration_test_1	35.72% <100%> (-0.1%)	⬇️
#integration_test_2	36.52% <0%> (+0.01%)	⬆️
#integration_test_3	35.56% <0%> (+0.1%)	⬆️
#node_e2e_test	35.34% <0%> (+0.02%)	⬆️
#unittest	28.51% <0%> (-0.01%)	⬇️

Impacted Files	Coverage Δ
daemon/mgr/container.go	61.04% <100%> (+0.29%)	⬆️
cri/ocicni/cni_manager.go	58.82% <0%> (-11.77%)	⬇️
pkg/streams/utils.go	82.14% <0%> (-7.15%)	⬇️
ctrd/watch.go	77.46% <0%> (-5.64%)	⬇️
pkg/meta/store.go	67.44% <0%> (-1.56%)	⬇️
ctrd/container.go	55.64% <0%> (-0.58%)	⬇️
daemon/mgr/container_utils.go	82.38% <0%> (-0.57%)	⬇️
daemon/mgr/network.go	72.72% <0%> (-0.43%)	⬇️
cri/v1alpha2/cri.go	72.01% <0%> (-0.13%)	⬇️
daemon/containerio/io.go	74.75% <0%> (ø)	⬆️
... and 5 more

[allencloud]

CI fails:

----------------------------------------------------------------------
FAIL: api_container_start_test.go:56: APIContainerStartSuite.TestStartPausedContainer
api_container_start_test.go:67:
    CheckRespStatus(c, resp, 409)
util_api.go:27:
    c.Assert(resp.StatusCode, check.Equals, status, check.Commentf("Response Body: %v", string(body)))
... obtained int = 304
... expected int = 409
... Response Body: 

[Ace-Tang]

Forget to modify test case

[allencloud]

Forget to modify test case

also need to consider the API definition:

  /containers/{id}/start:
    post:
      summary: "Start a container"
      operationId: "ContainerStart"
      responses:
        204:
          description: "no error"
        304:
          description: "container already started"
          schema:
            $ref: "#/definitions/ErrorResponse"
        404:
          description: "no such container"
          schema:
            $ref: "#/definitions/ErrorResponse"
          examples:
            application/json:
              message: "No such container: c2ada9df5af8"
        500:
          description: "server error"
          schema:
            $ref: "#/definitions/ErrorResponse"

[Ace-Tang]

Can we decide whether start a paused container http status should be 500 followed moby or other ? @allencloud

[fuweid]

LGTM