From cedb01d295701d49436865b115114aadd7f8c258 Mon Sep 17 00:00:00 2001 From: bin Date: Tue, 11 Jan 2022 19:45:41 +0800 Subject: [PATCH 1/7] runtime: close span before return from function in case of error Return before closing span will cause invalid spans, so span should be closed before function return. Fixes: #3424 Signed-off-by: bin --- src/runtime/virtcontainers/container.go | 57 +++++++++++++++---------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/runtime/virtcontainers/container.go b/src/runtime/virtcontainers/container.go index 60a409a527..b0bffa266d 100644 --- a/src/runtime/virtcontainers/container.go +++ b/src/runtime/virtcontainers/container.go @@ -604,33 +604,44 @@ func (c *Container) unmountHostMounts(ctx context.Context) error { span, ctx := katatrace.Trace(ctx, c.Logger(), "unmountHostMounts", containerTracingTags, map[string]string{"container_id": c.id}) defer span.End() + unmountFunc := func(m Mount) (err error) { + span, _ := katatrace.Trace(ctx, c.Logger(), "unmount", containerTracingTags, map[string]string{"container_id": c.id, "host-path": m.HostPath}) + defer func() { + if err != nil { + katatrace.AddTags(span, "error", err) + } + span.End() + }() + + if err = syscall.Unmount(m.HostPath, syscall.MNT_DETACH|UmountNoFollow); err != nil { + c.Logger().WithFields(logrus.Fields{ + "host-path": m.HostPath, + "error": err, + }).Warn("Could not umount") + return err + } + + if m.Type == "bind" { + s, err := os.Stat(m.HostPath) + if err != nil { + return errors.Wrapf(err, "Could not stat host-path %v", m.HostPath) + } + // Remove the empty file or directory + if s.Mode().IsRegular() && s.Size() == 0 { + os.Remove(m.HostPath) + } + if s.Mode().IsDir() { + syscall.Rmdir(m.HostPath) + } + } + return nil + } + for _, m := range c.mounts { if m.HostPath != "" { - span, _ := katatrace.Trace(ctx, c.Logger(), "unmount", containerTracingTags, map[string]string{"container_id": c.id, "host-path": m.HostPath}) - - if err := syscall.Unmount(m.HostPath, syscall.MNT_DETACH|UmountNoFollow); err != nil { - c.Logger().WithFields(logrus.Fields{ - "host-path": m.HostPath, - "error": err, - }).Warn("Could not umount") + if err := unmountFunc(m); err != nil { return err } - - if m.Type == "bind" { - s, err := os.Stat(m.HostPath) - if err != nil { - return errors.Wrapf(err, "Could not stat host-path %v", m.HostPath) - } - // Remove the empty file or directory - if s.Mode().IsRegular() && s.Size() == 0 { - os.Remove(m.HostPath) - } - if s.Mode().IsDir() { - syscall.Rmdir(m.HostPath) - } - } - - span.End() } } From 770d4acf8b8b44250ae2bb2a67a6178590d3d0b6 Mon Sep 17 00:00:00 2001 From: Sebastian Hasler Date: Wed, 12 Jan 2022 23:24:43 +0100 Subject: [PATCH 2/7] tools: Fix groupname if it differs from username The script `tools/packaging/static-build/qemu/build-base-qemu.sh` previously failed on systems where the user's groupname differs from the username Fixes: #3461 Signed-off-by: Sebastian Hasler --- tools/packaging/static-build/qemu/build-base-qemu.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/packaging/static-build/qemu/build-base-qemu.sh b/tools/packaging/static-build/qemu/build-base-qemu.sh index a45651f155..0be853e59c 100755 --- a/tools/packaging/static-build/qemu/build-base-qemu.sh +++ b/tools/packaging/static-build/qemu/build-base-qemu.sh @@ -54,4 +54,4 @@ sudo "${container_engine}" run \ -v "${PWD}":/share qemu-static \ mv "${qemu_destdir}/${qemu_tar}" /share/ -sudo chown ${USER}:${USER} "${PWD}/${qemu_tar}" +sudo chown ${USER}:$(id -gn ${USER}) "${PWD}/${qemu_tar}" From 620bb97e3ff1f1ef504228fbc2a2005eb0367b7a Mon Sep 17 00:00:00 2001 From: liangxianlong Date: Thu, 13 Jan 2022 14:48:10 +0800 Subject: [PATCH 3/7] runtime: Provide protection for shared data The k.reqHandlers should be protected by locks when used Fixes #3440 Signed-off-by: liangxianlong --- src/runtime/virtcontainers/kata_agent.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index cd0336ee8d..ebfbb6df99 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -2068,10 +2068,20 @@ func (k *kataAgent) sendReq(spanCtx context.Context, request interface{}) (inter } msgName := proto.MessageName(request.(proto.Message)) + + k.Lock() + + if k.reqHandlers == nil { + return nil, errors.New("Client has already disconnected") + } + handler := k.reqHandlers[msgName] if msgName == "" || handler == nil { return nil, errors.New("Invalid request type") } + + k.Unlock() + message := request.(proto.Message) ctx, cancel := k.getReqContext(spanCtx, msgName) if cancel != nil { From 8c8571f4bab99408ebf2a366df77ef3bcd5ba054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Mon, 17 Jan 2022 16:47:15 +0100 Subject: [PATCH 4/7] workflows: Use the correct branch ref on test kata-deploy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The action used for testing kata-deploy is entirely based on the action used to build the kata-deploy tarball, but while the latter is able to use the correct branch, the former always uses `main`. This happens as the `issue_comment`, from GitHub actions, passed the "default branch" as the GITHUB_REF. As we're not the first ones to face such a issue, I've decided to take one of the approaches suggested at one of the checkout's issues, https://github.com/actions/checkout/issues/331, and take advantage of a new action provided by the community, which will get the PR where the comment was made, give us that ref, and that then can be used with the checkout action, resulting on what we originally wanted. Fixes: #3443 Signed-off-by: Fabiano Fidêncio --- .github/workflows/kata-deploy-test.yaml | 31 +++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/.github/workflows/kata-deploy-test.yaml b/.github/workflows/kata-deploy-test.yaml index 4dce7b2f30..6b93c1ec9f 100644 --- a/.github/workflows/kata-deploy-test.yaml +++ b/.github/workflows/kata-deploy-test.yaml @@ -48,7 +48,18 @@ jobs: - rootfs-initrd - shim-v2 steps: + # As Github action event `issue_comment` does not provide the right ref + # (commit/branch) to be tested, let's use this third part action to work + # this limitation around. + - name: resolve pr refs + id: refs + uses: kata-containers/resolve-pr-refs@v0.0.3 + with: + token: ${{ secrets.GITHUB_TOKEN }} + - uses: actions/checkout@v2 + with: + ref: ${{ steps.refs.outputs.head_ref }} - name: Install docker run: | curl -fsSL https://test.docker.com -o test-docker.sh @@ -75,7 +86,17 @@ jobs: runs-on: ubuntu-latest needs: build-asset steps: + # As Github action event `issue_comment` does not provide the right ref + # (commit/branch) to be tested, let's use this third part action to work + # this limitation around. + - name: resolve pr refs + id: refs + uses: kata-containers/resolve-pr-refs@v0.0.3 + with: + token: ${{ secrets.GITHUB_TOKEN }} - uses: actions/checkout@v2 + with: + ref: ${{ steps.refs.outputs.head_ref }} - name: get-artifacts uses: actions/download-artifact@v2 with: @@ -94,7 +115,17 @@ jobs: needs: create-kata-tarball runs-on: ubuntu-latest steps: + # As Github action event `issue_comment` does not provide the right ref + # (commit/branch) to be tested, let's use this third part action to work + # this limitation around. + - name: resolve pr refs + id: refs + uses: kata-containers/resolve-pr-refs@v0.0.3 + with: + token: ${{ secrets.GITHUB_TOKEN }} - uses: actions/checkout@v2 + with: + ref: ${{ steps.refs.outputs.head_ref }} - name: get-kata-tarball uses: actions/download-artifact@v2 with: From b8463224c89d91ec2d4f8272b97879504fa4178a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Tue, 18 Jan 2022 12:38:18 +0100 Subject: [PATCH 5/7] workflows: Ensure force-skip-ci skips all actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change it was only applied to the static-checks, but if we're already taking the extreme path of skipping the CI, we better ensure we skip all the actions and not just a few of them. Fixes: #3471 Signed-off-by: Fabiano Fidêncio --- .github/workflows/PR-wip-checks.yaml | 1 + .github/workflows/commit-message-check.yaml | 12 +++++++----- .github/workflows/kata-deploy-push.yaml | 7 +++++++ .github/workflows/move-issues-to-in-progress.yaml | 4 ++++ .github/workflows/require-pr-porting-labels.yaml | 3 +++ .github/workflows/snap.yaml | 3 +++ 6 files changed, 25 insertions(+), 5 deletions(-) diff --git a/.github/workflows/PR-wip-checks.yaml b/.github/workflows/PR-wip-checks.yaml index 16f8167b2b..97c35145a7 100644 --- a/.github/workflows/PR-wip-checks.yaml +++ b/.github/workflows/PR-wip-checks.yaml @@ -15,6 +15,7 @@ jobs: name: WIP Check steps: - name: WIP Check + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} uses: tim-actions/wip-check@1c2a1ca6c110026b3e2297bb2ef39e1747b5a755 with: labels: '["do-not-merge", "wip", "rfc"]' diff --git a/.github/workflows/commit-message-check.yaml b/.github/workflows/commit-message-check.yaml index 55f03fecde..3c2b5e91af 100644 --- a/.github/workflows/commit-message-check.yaml +++ b/.github/workflows/commit-message-check.yaml @@ -18,24 +18,26 @@ jobs: name: Commit Message Check steps: - name: Get PR Commits + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} id: 'get-pr-commits' uses: tim-actions/get-pr-commits@v1.0.0 with: token: ${{ secrets.GITHUB_TOKEN }} - name: DCO Check + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} uses: tim-actions/dco@2fd0504dc0d27b33f542867c300c60840c6dcb20 with: commits: ${{ steps.get-pr-commits.outputs.commits }} - name: Commit Body Missing Check - if: ${{ success() || failure() }} + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') && ( success() || failure() ) }} uses: tim-actions/commit-body-check@v1.0.2 with: commits: ${{ steps.get-pr-commits.outputs.commits }} - name: Check Subject Line Length - if: ${{ success() || failure() }} + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') && ( success() || failure() ) }} uses: tim-actions/commit-message-checker-with-regex@v0.3.1 with: commits: ${{ steps.get-pr-commits.outputs.commits }} @@ -44,7 +46,7 @@ jobs: post_error: ${{ env.error_msg }} - name: Check Body Line Length - if: ${{ success() || failure() }} + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') && ( success() || failure() ) }} uses: tim-actions/commit-message-checker-with-regex@v0.3.1 with: commits: ${{ steps.get-pr-commits.outputs.commits }} @@ -71,7 +73,7 @@ jobs: post_error: ${{ env.error_msg }} - name: Check Fixes - if: ${{ success() || failure() }} + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') && ( success() || failure() ) }} uses: tim-actions/commit-message-checker-with-regex@v0.3.1 with: commits: ${{ steps.get-pr-commits.outputs.commits }} @@ -82,7 +84,7 @@ jobs: one_pass_all_pass: 'true' - name: Check Subsystem - if: ${{ success() || failure() }} + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') && ( success() || failure() ) }} uses: tim-actions/commit-message-checker-with-regex@v0.3.1 with: commits: ${{ steps.get-pr-commits.outputs.commits }} diff --git a/.github/workflows/kata-deploy-push.yaml b/.github/workflows/kata-deploy-push.yaml index effca2e20e..051808f896 100644 --- a/.github/workflows/kata-deploy-push.yaml +++ b/.github/workflows/kata-deploy-push.yaml @@ -19,11 +19,13 @@ jobs: steps: - uses: actions/checkout@v2 - name: Install docker + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} run: | curl -fsSL https://test.docker.com -o test-docker.sh sh test-docker.sh - name: Build ${{ matrix.asset }} + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} run: | make "${KATA_ASSET}-tarball" build_dir=$(readlink -f build) @@ -33,6 +35,7 @@ jobs: KATA_ASSET: ${{ matrix.asset }} - name: store-artifact ${{ matrix.asset }} + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} uses: actions/upload-artifact@v2 with: name: kata-artifacts @@ -45,14 +48,17 @@ jobs: steps: - uses: actions/checkout@v2 - name: get-artifacts + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} uses: actions/download-artifact@v2 with: name: kata-artifacts path: build - name: merge-artifacts + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} run: | make merge-builds - name: store-artifacts + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} uses: actions/upload-artifact@v2 with: name: kata-static-tarball @@ -63,6 +69,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: make kata-tarball + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} run: | make kata-tarball sudo make install-tarball diff --git a/.github/workflows/move-issues-to-in-progress.yaml b/.github/workflows/move-issues-to-in-progress.yaml index 6a55340af3..0e15abaea3 100644 --- a/.github/workflows/move-issues-to-in-progress.yaml +++ b/.github/workflows/move-issues-to-in-progress.yaml @@ -16,6 +16,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Install hub + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} run: | HUB_ARCH="amd64" HUB_VER=$(curl -sL "https://api.github.com/repos/github/hub/releases/latest" |\ @@ -26,6 +27,7 @@ jobs: sudo install hub /usr/local/bin - name: Install hub extension script + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} run: | # Clone into a temporary directory to avoid overwriting # any existing github directory. @@ -35,9 +37,11 @@ jobs: popd &>/dev/null - name: Checkout code to allow hub to communicate with the project + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} uses: actions/checkout@v2 - name: Move issue to "In progress" + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} env: GITHUB_TOKEN: ${{ secrets.KATA_GITHUB_ACTIONS_TOKEN }} run: | diff --git a/.github/workflows/require-pr-porting-labels.yaml b/.github/workflows/require-pr-porting-labels.yaml index 3f38071947..585e86bc42 100644 --- a/.github/workflows/require-pr-porting-labels.yaml +++ b/.github/workflows/require-pr-porting-labels.yaml @@ -20,6 +20,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Install hub + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} run: | HUB_ARCH="amd64" HUB_VER=$(curl -sL "https://api.github.com/repos/github/hub/releases/latest" |\ @@ -30,6 +31,7 @@ jobs: sudo install hub /usr/local/bin - name: Checkout code to allow hub to communicate with the project + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} uses: actions/checkout@v2 - name: Install porting checker script @@ -42,6 +44,7 @@ jobs: popd &>/dev/null - name: Stop PR being merged unless it has a correct set of porting labels + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} env: GITHUB_TOKEN: ${{ secrets.KATA_GITHUB_ACTIONS_TOKEN }} run: | diff --git a/.github/workflows/snap.yaml b/.github/workflows/snap.yaml index 165a3a3bef..f80df268dc 100644 --- a/.github/workflows/snap.yaml +++ b/.github/workflows/snap.yaml @@ -5,13 +5,16 @@ jobs: runs-on: ubuntu-20.04 steps: - name: Check out + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} uses: actions/checkout@v2 with: fetch-depth: 0 - name: Install Snapcraft + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} uses: samuelmeuli/action-snapcraft@v1 - name: Build snap + if: ${{ !contains(github.event.pull_request.labels.*.name, 'force-skip-ci') }} run: | snapcraft -d snap --destructive-mode From 13b7d93b4f497518c97f02109f5c24da3561d6bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Tue, 18 Jan 2022 14:39:01 +0100 Subject: [PATCH 6/7] workflows: Ensure a label change re-triggers the actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed in order to ensure that, for instance, if `force-skip-ci` label is either added or removed later, the jobs related to the actions will be restarted and accordingly checked. Signed-off-by: Fabiano Fidêncio --- .github/workflows/commit-message-check.yaml | 2 ++ .github/workflows/kata-deploy-push.yaml | 11 ++++++++++- .github/workflows/move-issues-to-in-progress.yaml | 2 ++ .github/workflows/snap.yaml | 11 ++++++++++- 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/.github/workflows/commit-message-check.yaml b/.github/workflows/commit-message-check.yaml index 3c2b5e91af..7abb5d8515 100644 --- a/.github/workflows/commit-message-check.yaml +++ b/.github/workflows/commit-message-check.yaml @@ -5,6 +5,8 @@ on: - opened - reopened - synchronize + - labeled + - unlabeled env: error_msg: |+ diff --git a/.github/workflows/kata-deploy-push.yaml b/.github/workflows/kata-deploy-push.yaml index 051808f896..39b10a550f 100644 --- a/.github/workflows/kata-deploy-push.yaml +++ b/.github/workflows/kata-deploy-push.yaml @@ -1,6 +1,15 @@ name: kata deploy build -on: [push, pull_request] +on: + pull_request: + types: + - opened + - edited + - reopened + - synchronize + - labeled + - unlabeled + push jobs: build-asset: diff --git a/.github/workflows/move-issues-to-in-progress.yaml b/.github/workflows/move-issues-to-in-progress.yaml index 0e15abaea3..ab97e2de9b 100644 --- a/.github/workflows/move-issues-to-in-progress.yaml +++ b/.github/workflows/move-issues-to-in-progress.yaml @@ -10,6 +10,8 @@ on: types: - opened - reopened + - labeled + - unlabeled jobs: move-linked-issues-to-in-progress: diff --git a/.github/workflows/snap.yaml b/.github/workflows/snap.yaml index f80df268dc..5ec399e83c 100644 --- a/.github/workflows/snap.yaml +++ b/.github/workflows/snap.yaml @@ -1,5 +1,14 @@ name: snap CI -on: ["pull_request"] +on: + pull_request: + types: + - opened + - synchronize + - reopened + - edited + - labeled + - unlabeled + jobs: test: runs-on: ubuntu-20.04 From 99ed596ae42d062d229cc650194bd3986c685b2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Wed, 19 Jan 2022 11:05:58 +0100 Subject: [PATCH 7/7] workflows: Fix typo in kata-deploy-push action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `:` was missed when d87ab14fa7c2244a631189083cceb6344ae9f306 was introduced. Fixes: #3485 Signed-off-by: Fabiano Fidêncio --- .github/workflows/kata-deploy-push.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/kata-deploy-push.yaml b/.github/workflows/kata-deploy-push.yaml index 39b10a550f..0de8749303 100644 --- a/.github/workflows/kata-deploy-push.yaml +++ b/.github/workflows/kata-deploy-push.yaml @@ -9,7 +9,7 @@ on: - synchronize - labeled - unlabeled - push + push: jobs: build-asset: