From 3d46750596823816c3389ac3c1ac561ef3167204 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Fri, 17 Jul 2020 15:33:41 -0700 Subject: [PATCH 1/9] device: Ease device access for rootfs device to allow node creation For docker in docker scenario, the nested container created has entry "b *:* m" in the list of devices it is allowed to access under /sys/fs/cgroup/devices/docker/{ctrid}/devices.list. This entry was causing issues while starting a nested container as we were denying "m" access to the rootfs block devices. With this change we add back "m" access, the container would be allowed to create a device node for the rootfs device but will not have read-write access to the created device node. This fixes the docker in docker use case while still making sure the container is not allowed read/write access to the rootfs. Note, this could also be fixed by simply skipping {"Type : "b"} while creating the device cgroup with libcontainer. But this seems to be undocumented behaviour at this point, hence refrained from taking this approach. Fixes #426 Signed-off-by: Archana Shinde --- src/agent/src/device.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/agent/src/device.rs b/src/agent/src/device.rs index 775ead87db..423fc9abbb 100644 --- a/src/agent/src/device.rs +++ b/src/agent/src/device.rs @@ -398,7 +398,7 @@ pub fn update_device_cgroup(spec: &mut Spec) -> Result<()> { major: Some(major), minor: Some(minor), r#type: String::from("b"), - access: String::from("rwm"), + access: String::from("rw"), }); Ok(()) From c5081624c5c17b61bccbfa614bdb0daa43a71c74 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Tue, 21 Jul 2020 11:50:52 -0700 Subject: [PATCH 2/9] actions: Add action to perform WIP check for pull requests Use github actions for performing WIP checks on PRs. The action checks for keywords in subject line as well labels. Fixes: #437 Signed-off-by: Archana Shinde (cherry picked from commit 0d96145c299dc34bfa5e8aa8d8eed8ea6c341a40) --- .github/workflows/PR-wip-checks.yaml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .github/workflows/PR-wip-checks.yaml diff --git a/.github/workflows/PR-wip-checks.yaml b/.github/workflows/PR-wip-checks.yaml new file mode 100644 index 0000000000..9517afa8c9 --- /dev/null +++ b/.github/workflows/PR-wip-checks.yaml @@ -0,0 +1,21 @@ +name: Pull request WIP checks +on: + pull_request: + types: + - opened + - synchronize + - reopened + - edited + - labeled + - unlabeled + +jobs: + pr_wip_check: + runs-on: ubuntu-latest + name: WIP Check + steps: + - name: WIP Check + uses: tim-actions/wip-check@master + with: + labels: '["do-not-merge", "wip", "rfc"]' + keywords: '["WIP", "wip", "RFC", "rfc", "dnm", "DNM", "do-not-merge"]' From 8564c99eae956f4801843b0e9a0f1145a1d44322 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Tue, 21 Jul 2020 15:13:17 -0700 Subject: [PATCH 3/9] actions: Add github actions to perform DCO check Action performs a check to verify PR raised has commits that are signed-off. Signed-off-by: Archana Shinde (cherry picked from commit 1b157e50150d49ca00a0ebd43ad305abb60165c4) --- .github/workflows/dco-check.yaml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/workflows/dco-check.yaml diff --git a/.github/workflows/dco-check.yaml b/.github/workflows/dco-check.yaml new file mode 100644 index 0000000000..3760a3e92c --- /dev/null +++ b/.github/workflows/dco-check.yaml @@ -0,0 +1,22 @@ +name: DCO check +on: + pull_request: + types: + - opened + - reopened + - synchronize + +jobs: + dco_check_job: + runs-on: ubuntu-latest + name: DCO Check + steps: + - name: Get PR Commits + id: 'get-pr-commits' + uses: tim-actions/get-pr-commits@master + with: + token: ${{ secrets.GITHUB_TOKEN }} + - name: DCO Check + uses: tim-actions/dco@master + with: + commits: ${{ steps.get-pr-commits.outputs.commits }} From 33b1865e6ee48f2eb2769b489bf3fafe1d4a4c2f Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Wed, 22 Jul 2020 18:07:11 -0700 Subject: [PATCH 4/9] actions: Pin to a particular sha for actions Since actions can access the github token, lets use a particular version of sha rather than using master. Fixes: #437 Signed-off-by: Archana Shinde (cherry picked from commit 57b64f35e0c9beadd26d45cb984630d702d4c9fc) --- .github/workflows/PR-wip-checks.yaml | 2 +- .github/workflows/dco-check.yaml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/PR-wip-checks.yaml b/.github/workflows/PR-wip-checks.yaml index 9517afa8c9..16f8167b2b 100644 --- a/.github/workflows/PR-wip-checks.yaml +++ b/.github/workflows/PR-wip-checks.yaml @@ -15,7 +15,7 @@ jobs: name: WIP Check steps: - name: WIP Check - uses: tim-actions/wip-check@master + uses: tim-actions/wip-check@1c2a1ca6c110026b3e2297bb2ef39e1747b5a755 with: labels: '["do-not-merge", "wip", "rfc"]' keywords: '["WIP", "wip", "RFC", "rfc", "dnm", "DNM", "do-not-merge"]' diff --git a/.github/workflows/dco-check.yaml b/.github/workflows/dco-check.yaml index 3760a3e92c..159dec2e1a 100644 --- a/.github/workflows/dco-check.yaml +++ b/.github/workflows/dco-check.yaml @@ -13,10 +13,10 @@ jobs: steps: - name: Get PR Commits id: 'get-pr-commits' - uses: tim-actions/get-pr-commits@master + uses: tim-actions/get-pr-commits@ed97a21c3f83c3417e67a4733ea76887293a2c8f with: token: ${{ secrets.GITHUB_TOKEN }} - name: DCO Check - uses: tim-actions/dco@master + uses: tim-actions/dco@2fd0504dc0d27b33f542867c300c60840c6dcb20 with: commits: ${{ steps.get-pr-commits.outputs.commits }} From d914f0182903856bc025f8ef9c3e61e8e7f4620b Mon Sep 17 00:00:00 2001 From: Chelsea Mafrica Date: Fri, 24 Jul 2020 01:15:15 +0000 Subject: [PATCH 5/9] virtcontainers: Move unit tests for types/sandbox.go Move unit tests that were in virtcontainers/sandbox_test.go relating to Socket, Volume, and SandboxState to types/sandbox_test.go. Change testSandboxStateTransition function to use SandboxState only instead of Sandbox from virtcontainers/sandbox.go. Fixes #435 Signed-off-by: Chelsea Mafrica --- .../virtcontainers/types/sandbox_test.go | 215 ++++++++++++++++++ 1 file changed, 215 insertions(+) create mode 100644 src/runtime/virtcontainers/types/sandbox_test.go diff --git a/src/runtime/virtcontainers/types/sandbox_test.go b/src/runtime/virtcontainers/types/sandbox_test.go new file mode 100644 index 0000000000..875eee1a13 --- /dev/null +++ b/src/runtime/virtcontainers/types/sandbox_test.go @@ -0,0 +1,215 @@ +// Copyright (c) 2020 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0 +// + +package types + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func testSandboxStateTransition(t *testing.T, state StateString, newState StateString) error { + s := SandboxState{ + State: state, + } + + return s.ValidTransition(state, newState) +} + +func TestSandboxStateReadyRunning(t *testing.T) { + err := testSandboxStateTransition(t, StateReady, StateRunning) + assert.NoError(t, err) +} + +func TestSandboxStateRunningPaused(t *testing.T) { + err := testSandboxStateTransition(t, StateRunning, StatePaused) + assert.NoError(t, err) +} + +func TestSandboxStatePausedRunning(t *testing.T) { + err := testSandboxStateTransition(t, StatePaused, StateRunning) + assert.NoError(t, err) +} + +func TestSandboxStatePausedStopped(t *testing.T) { + err := testSandboxStateTransition(t, StatePaused, StateStopped) + assert.NoError(t, err) +} + +func TestSandboxStateRunningStopped(t *testing.T) { + err := testSandboxStateTransition(t, StateRunning, StateStopped) + assert.NoError(t, err) +} + +func TestSandboxStateReadyPaused(t *testing.T) { + err := testSandboxStateTransition(t, StateReady, StateStopped) + assert.NoError(t, err) +} + +func TestSandboxStatePausedReady(t *testing.T) { + err := testSandboxStateTransition(t, StateStopped, StateReady) + assert.Error(t, err) +} + +func testStateValid(t *testing.T, stateStr StateString, expected bool) { + state := &SandboxState{ + State: stateStr, + } + + ok := state.Valid() + assert.Equal(t, ok, expected) +} + +func TestStateValidSuccessful(t *testing.T) { + testStateValid(t, StateReady, true) + testStateValid(t, StateRunning, true) + testStateValid(t, StatePaused, true) + testStateValid(t, StateStopped, true) +} + +func TestStateValidFailing(t *testing.T) { + testStateValid(t, "", false) +} + +func TestValidTransitionFailingOldStateMismatch(t *testing.T) { + state := &SandboxState{ + State: StateReady, + } + + err := state.ValidTransition(StateRunning, StateStopped) + assert.Error(t, err) +} + +func TestVolumesSetSuccessful(t *testing.T) { + volumes := &Volumes{} + + volStr := "mountTag1:hostPath1 mountTag2:hostPath2" + + expected := Volumes{ + { + MountTag: "mountTag1", + HostPath: "hostPath1", + }, + { + MountTag: "mountTag2", + HostPath: "hostPath2", + }, + } + + err := volumes.Set(volStr) + assert.NoError(t, err) + assert.Exactly(t, *volumes, expected) +} + +func TestVolumesSetFailingTooFewArguments(t *testing.T) { + volumes := &Volumes{} + + volStr := "mountTag1 mountTag2" + + err := volumes.Set(volStr) + assert.Error(t, err) +} + +func TestVolumesSetFailingTooManyArguments(t *testing.T) { + volumes := &Volumes{} + + volStr := "mountTag1:hostPath1:Foo1 mountTag2:hostPath2:Foo2" + + err := volumes.Set(volStr) + assert.Error(t, err) +} + +func TestVolumesSetFailingVoidArguments(t *testing.T) { + volumes := &Volumes{} + + volStr := ": : :" + + err := volumes.Set(volStr) + assert.Error(t, err) +} + +func TestVolumesStringSuccessful(t *testing.T) { + volumes := &Volumes{ + { + MountTag: "mountTag1", + HostPath: "hostPath1", + }, + { + MountTag: "mountTag2", + HostPath: "hostPath2", + }, + } + + expected := "mountTag1:hostPath1 mountTag2:hostPath2" + + result := volumes.String() + assert.Equal(t, result, expected) +} + +func TestSocketsSetSuccessful(t *testing.T) { + sockets := &Sockets{} + + sockStr := "devID1:id1:hostPath1:Name1 devID2:id2:hostPath2:Name2" + + expected := Sockets{ + { + DeviceID: "devID1", + ID: "id1", + HostPath: "hostPath1", + Name: "Name1", + }, + { + DeviceID: "devID2", + ID: "id2", + HostPath: "hostPath2", + Name: "Name2", + }, + } + + err := sockets.Set(sockStr) + assert.NoError(t, err) + assert.Exactly(t, *sockets, expected) +} + +func TestSocketsSetFailingWrongArgsAmount(t *testing.T) { + sockets := &Sockets{} + + sockStr := "devID1:id1:hostPath1" + + err := sockets.Set(sockStr) + assert.Error(t, err) +} + +func TestSocketsSetFailingVoidArguments(t *testing.T) { + sockets := &Sockets{} + + sockStr := ":::" + + err := sockets.Set(sockStr) + assert.Error(t, err) +} + +func TestSocketsStringSuccessful(t *testing.T) { + sockets := &Sockets{ + { + DeviceID: "devID1", + ID: "id1", + HostPath: "hostPath1", + Name: "Name1", + }, + { + DeviceID: "devID2", + ID: "id2", + HostPath: "hostPath2", + Name: "Name2", + }, + } + + expected := "devID1:id1:hostPath1:Name1 devID2:id2:hostPath2:Name2" + + result := sockets.String() + assert.Equal(t, result, expected) +} From 07a307b4b1568f0b995db348ade01d05567fa771 Mon Sep 17 00:00:00 2001 From: Chelsea Mafrica Date: Fri, 24 Jul 2020 01:17:34 +0000 Subject: [PATCH 6/9] virtcontainers: Remove duplicate unit tests Remove tests from virtcontainers/sandbox_test.go which were moved to virtcontainers/types/sandbox_test.go. Signed-off-by: Chelsea Mafrica --- src/runtime/virtcontainers/sandbox_test.go | 209 --------------------- 1 file changed, 209 deletions(-) diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index d2ec1920ad..4af47ffaec 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -169,215 +169,6 @@ func TestCreateSandboxEmptyID(t *testing.T) { defer cleanUp() } -func testSandboxStateTransition(t *testing.T, state types.StateString, newState types.StateString) error { - hConfig := newHypervisorConfig(nil, nil) - - p, err := testCreateSandbox(t, testSandboxID, MockHypervisor, hConfig, NetworkConfig{}, nil, nil) - assert.NoError(t, err) - defer cleanUp() - - p.state = types.SandboxState{ - State: state, - } - - return p.state.ValidTransition(state, newState) -} - -func TestSandboxStateReadyRunning(t *testing.T) { - err := testSandboxStateTransition(t, types.StateReady, types.StateRunning) - assert.NoError(t, err) -} - -func TestSandboxStateRunningPaused(t *testing.T) { - err := testSandboxStateTransition(t, types.StateRunning, types.StatePaused) - assert.NoError(t, err) -} - -func TestSandboxStatePausedRunning(t *testing.T) { - err := testSandboxStateTransition(t, types.StatePaused, types.StateRunning) - assert.NoError(t, err) -} - -func TestSandboxStatePausedStopped(t *testing.T) { - err := testSandboxStateTransition(t, types.StatePaused, types.StateStopped) - assert.NoError(t, err) -} - -func TestSandboxStateRunningStopped(t *testing.T) { - err := testSandboxStateTransition(t, types.StateRunning, types.StateStopped) - assert.NoError(t, err) -} - -func TestSandboxStateReadyPaused(t *testing.T) { - err := testSandboxStateTransition(t, types.StateReady, types.StateStopped) - assert.NoError(t, err) -} - -func TestSandboxStatePausedReady(t *testing.T) { - err := testSandboxStateTransition(t, types.StateStopped, types.StateReady) - assert.Error(t, err) -} - -func testStateValid(t *testing.T, stateStr types.StateString, expected bool) { - state := &types.SandboxState{ - State: stateStr, - } - - ok := state.Valid() - assert.Equal(t, ok, expected) -} - -func TestStateValidSuccessful(t *testing.T) { - testStateValid(t, types.StateReady, true) - testStateValid(t, types.StateRunning, true) - testStateValid(t, types.StatePaused, true) - testStateValid(t, types.StateStopped, true) -} - -func TestStateValidFailing(t *testing.T) { - testStateValid(t, "", false) -} - -func TestValidTransitionFailingOldStateMismatch(t *testing.T) { - state := &types.SandboxState{ - State: types.StateReady, - } - - err := state.ValidTransition(types.StateRunning, types.StateStopped) - assert.Error(t, err) -} - -func TestVolumesSetSuccessful(t *testing.T) { - volumes := &types.Volumes{} - - volStr := "mountTag1:hostPath1 mountTag2:hostPath2" - - expected := types.Volumes{ - { - MountTag: "mountTag1", - HostPath: "hostPath1", - }, - { - MountTag: "mountTag2", - HostPath: "hostPath2", - }, - } - - err := volumes.Set(volStr) - assert.NoError(t, err) - assert.Exactly(t, *volumes, expected) -} - -func TestVolumesSetFailingTooFewArguments(t *testing.T) { - volumes := &types.Volumes{} - - volStr := "mountTag1 mountTag2" - - err := volumes.Set(volStr) - assert.Error(t, err) -} - -func TestVolumesSetFailingTooManyArguments(t *testing.T) { - volumes := &types.Volumes{} - - volStr := "mountTag1:hostPath1:Foo1 mountTag2:hostPath2:Foo2" - - err := volumes.Set(volStr) - assert.Error(t, err) -} - -func TestVolumesSetFailingVoidArguments(t *testing.T) { - volumes := &types.Volumes{} - - volStr := ": : :" - - err := volumes.Set(volStr) - assert.Error(t, err) -} - -func TestVolumesStringSuccessful(t *testing.T) { - volumes := &types.Volumes{ - { - MountTag: "mountTag1", - HostPath: "hostPath1", - }, - { - MountTag: "mountTag2", - HostPath: "hostPath2", - }, - } - - expected := "mountTag1:hostPath1 mountTag2:hostPath2" - - result := volumes.String() - assert.Equal(t, result, expected) -} - -func TestSocketsSetSuccessful(t *testing.T) { - sockets := &types.Sockets{} - - sockStr := "devID1:id1:hostPath1:Name1 devID2:id2:hostPath2:Name2" - - expected := types.Sockets{ - { - DeviceID: "devID1", - ID: "id1", - HostPath: "hostPath1", - Name: "Name1", - }, - { - DeviceID: "devID2", - ID: "id2", - HostPath: "hostPath2", - Name: "Name2", - }, - } - - err := sockets.Set(sockStr) - assert.NoError(t, err) - assert.Exactly(t, *sockets, expected) -} - -func TestSocketsSetFailingWrongArgsAmount(t *testing.T) { - sockets := &types.Sockets{} - - sockStr := "devID1:id1:hostPath1" - - err := sockets.Set(sockStr) - assert.Error(t, err) -} - -func TestSocketsSetFailingVoidArguments(t *testing.T) { - sockets := &types.Sockets{} - - sockStr := ":::" - - err := sockets.Set(sockStr) - assert.Error(t, err) -} - -func TestSocketsStringSuccessful(t *testing.T) { - sockets := &types.Sockets{ - { - DeviceID: "devID1", - ID: "id1", - HostPath: "hostPath1", - Name: "Name1", - }, - { - DeviceID: "devID2", - ID: "id2", - HostPath: "hostPath2", - Name: "Name2", - }, - } - - expected := "devID1:id1:hostPath1:Name1 devID2:id2:hostPath2:Name2" - - result := sockets.String() - assert.Equal(t, result, expected) -} - func TestSandboxListSuccessful(t *testing.T) { sandbox := &Sandbox{} From 0b3cbee8158f7b69ee12f8d8a6fdcb747ba9ba36 Mon Sep 17 00:00:00 2001 From: Chelsea Mafrica Date: Fri, 24 Jul 2020 02:57:41 +0000 Subject: [PATCH 7/9] virtcontainers: Add additional unit tests for sandbox Add tests for state change, empty string failures for Volumes and Sockets. Change two function names to accurately reflect tests. Signed-off-by: Chelsea Mafrica --- .../virtcontainers/types/sandbox_test.go | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/runtime/virtcontainers/types/sandbox_test.go b/src/runtime/virtcontainers/types/sandbox_test.go index 875eee1a13..05075e4495 100644 --- a/src/runtime/virtcontainers/types/sandbox_test.go +++ b/src/runtime/virtcontainers/types/sandbox_test.go @@ -44,12 +44,17 @@ func TestSandboxStateRunningStopped(t *testing.T) { assert.NoError(t, err) } -func TestSandboxStateReadyPaused(t *testing.T) { +func TestSandboxStateReadyStopped(t *testing.T) { err := testSandboxStateTransition(t, StateReady, StateStopped) assert.NoError(t, err) } -func TestSandboxStatePausedReady(t *testing.T) { +func TestSandboxStateStoppedRunning(t *testing.T) { + err := testSandboxStateTransition(t, StateStopped, StateRunning) + assert.NoError(t, err) +} + +func TestSandboxStateStoppedReady(t *testing.T) { err := testSandboxStateTransition(t, StateStopped, StateReady) assert.Error(t, err) } @@ -104,6 +109,15 @@ func TestVolumesSetSuccessful(t *testing.T) { assert.Exactly(t, *volumes, expected) } +func TestVolumesSetFailingEmptyString(t *testing.T) { + volumes := &Volumes{} + + volStr := "" + + err := volumes.Set(volStr) + assert.Error(t, err) +} + func TestVolumesSetFailingTooFewArguments(t *testing.T) { volumes := &Volumes{} @@ -174,6 +188,15 @@ func TestSocketsSetSuccessful(t *testing.T) { assert.Exactly(t, *sockets, expected) } +func TestSocketsSetFailingEmptyString(t *testing.T) { + sockets := &Sockets{} + + sockStr := "" + + err := sockets.Set(sockStr) + assert.Error(t, err) +} + func TestSocketsSetFailingWrongArgsAmount(t *testing.T) { sockets := &Sockets{} From 1283febdd6aa5ac53db8d0e9ef13abce1fed1147 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Thu, 23 Jul 2020 11:38:58 +0800 Subject: [PATCH 8/9] ci: checkout TRAVIS_BRANCH So that we use 2.0-dev branch for tests. Fixes: kata-containers/tests#2732 Signed-off-by: Peng Tao --- ci/lib.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/lib.sh b/ci/lib.sh index 76734f1f55..e40e696501 100644 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -16,6 +16,10 @@ clone_tests_repo() fi go get -d -u "$tests_repo" || true + + if [ -n "${TRAVIS_BRANCH:-}" ]; then + ( cd "${tests_repo_dir}" && git checkout "${TRAVIS_BRANCH}" ) + fi } run_static_checks() From e5910c9b887a197d197a60c06b54013653c79927 Mon Sep 17 00:00:00 2001 From: Evan Foster Date: Mon, 20 Jul 2020 11:26:47 -0600 Subject: [PATCH 9/9] sandbox: Stop and clean up containers that fail to create A container that is created and added to a sandbox can still fail the final creation steps. In this case, the container must be stopped and have its resources cleaned up to prevent leaking sandbox mounts. Forward port of https://github.com/kata-containers/runtime/pull/2826 Fixes #2816 Signed-off-by: Evan Foster --- src/runtime/virtcontainers/sandbox.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 82e0a3ac12..c7cfedb574 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -1,4 +1,5 @@ // Copyright (c) 2016 Intel Corporation +// Copyright (c) 2020 Adobe Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -1201,6 +1202,14 @@ func (s *Sandbox) CreateContainer(contConfig ContainerConfig) (VCContainer, erro defer func() { // Rollback if error happens. if err != nil { + logger := s.Logger().WithFields(logrus.Fields{"container-id": c.id, "sandox-id": s.id, "rollback": true}) + logger.Warning("Cleaning up partially created container") + + if err2 := c.stop(true); err2 != nil { + logger.WithError(err2).Warning("Could not delete container") + } + + logger.Debug("Removing stopped container from sandbox store") s.removeContainer(c.id) } }()