Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove WORKAROUND_ISSUE_622 #3974

Closed
wants to merge 2 commits into from
Closed

Remove WORKAROUND_ISSUE_622 #3974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8a9c5d
Implement updated test matrix
apostasie Mar 4, 2025
a135e8e
Remove WORKAROUND_ISSUE_622
apostasie Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 16 additions & 18 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ jobs:
- runner: ubuntu-24.04
containerd: v1.6.36
arch: amd64
- runner: ubuntu-24.04
containerd: v1.7.25
arch: amd64
- runner: ubuntu-24.04
containerd: v2.0.3
arch: amd64
Expand Down Expand Up @@ -106,7 +103,7 @@ jobs:
matrix:
include:
- ubuntu: 22.04
containerd: v1.7.25
containerd: v1.6.36
runner: "ubuntu-22.04"
arch: amd64
- ubuntu: 24.04
Expand All @@ -115,7 +112,7 @@ jobs:
arch: amd64
- ubuntu: 24.04
containerd: v2.0.3
runner: ubuntu-24.04-arm
runner: "ubuntu-24.04-arm"
arch: arm64
env:
CONTAINERD_VERSION: "${{ matrix.containerd }}"
Expand Down Expand Up @@ -219,26 +216,35 @@ jobs:
needs: build-dependencies
timeout-minutes: 40
name: "${{ matrix.target }} | ${{ matrix.containerd }} | ${{ matrix.rootlesskit }} | ${{ matrix.ubuntu }}"
runs-on: "ubuntu-${{ matrix.ubuntu }}"
runs-on: "${{ matrix.runner }}"
strategy:
fail-fast: false
matrix:
include:
- ubuntu: 22.04
containerd: v1.7.25
containerd: v1.6.36
rootlesskit: v1.1.1 # Deprecated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still have RootlessKit v1 here, so the workaround still has to be enabled for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have rootlesskit v1, but we do not test slirp with it.

The only target we are testing slirp for is https://github.com/containerd/nerdctl/pull/3974/files#diff-faff1af3d8ff408964a57b2e475f69a6b7c7b71c9978cccc8f471798caac2c88R242-R246

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two kinds of the slirp4netns drivers:

  • network driver: outgoing connections
  • port driver: incoming connections

The code here still uses slirp4netns as the network driver

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad.
Thanks for clarifying!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of curious to see it fail though. Can we leave this PR open long enough for the full CI run?
Then I'll close it.

target: rootless
runner: "ubuntu-22.04"
arch: amd64
- ubuntu: 24.04
containerd: v2.0.3
rootlesskit: v2.3.2
target: rootless
arch: amd64
runner: "ubuntu-24.04"
- ubuntu: 24.04
containerd: v1.7.25
containerd: v2.0.3
rootlesskit: v2.3.2
target: rootless
arch: arm64
runner: "ubuntu-24.04-arm"
- ubuntu: 24.04
containerd: v2.0.3
rootlesskit: v2.3.2
target: rootless-port-slirp4netns
arch: amd64
runner: "ubuntu-24.04"
env:
CONTAINERD_VERSION: "${{ matrix.containerd }}"
ARCH: "${{ matrix.arch }}"
Expand Down Expand Up @@ -282,18 +288,10 @@ jobs:
--output=type=docker \
--cache-from type=gha,scope=${ARCH}-${CONTAINERD_VERSION} \
-t ${TEST_TARGET} --target ${TEST_TARGET} --build-arg UBUNTU_VERSION=${UBUNTU_VERSION} --build-arg CONTAINERD_VERSION=${CONTAINERD_VERSION} --build-arg ROOTLESSKIT_VERSION=${ROOTLESSKIT_VERSION} .
- name: "Disable BuildKit for RootlessKit v1 (workaround for issue #622)"
run: |
# https://github.com/containerd/nerdctl/issues/622
WORKAROUND_ISSUE_622=
if echo "${ROOTLESSKIT_VERSION}" | grep -q v1; then
WORKAROUND_ISSUE_622=1
fi
echo "WORKAROUND_ISSUE_622=${WORKAROUND_ISSUE_622}" >> "$GITHUB_ENV"
- name: "Test (network driver=slirp4netns, port driver=builtin)"
run: docker run -t --rm --privileged -e WORKAROUND_ISSUE_622=${WORKAROUND_ISSUE_622} ${TEST_TARGET} /test-integration-rootless.sh ./hack/test-integration.sh -test.only-flaky=false
run: docker run -t --rm --privileged ${TEST_TARGET} /test-integration-rootless.sh ./hack/test-integration.sh -test.only-flaky=false
- name: "Test (network driver=slirp4netns, port driver=builtin) (flaky)"
run: docker run -t --rm --privileged -e WORKAROUND_ISSUE_622=${WORKAROUND_ISSUE_622} ${TEST_TARGET} /test-integration-rootless.sh ./hack/test-integration.sh -test.only-flaky=true
run: docker run -t --rm --privileged ${TEST_TARGET} /test-integration-rootless.sh ./hack/test-integration.sh -test.only-flaky=true

build:
timeout-minutes: 5
Expand Down
11 changes: 1 addition & 10 deletions Dockerfile.d/test-integration-rootless.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ if [[ "$(id -u)" = "0" ]]; then
nerdctl apparmor load
fi

: "${WORKAROUND_ISSUE_622:=}"
if [[ "$WORKAROUND_ISSUE_622" = "1" ]]; then
touch /workaround-issue-622
fi

# Switch to the rootless user via SSH
systemctl start ssh
exec ssh -o StrictHostKeyChecking=no rootless@localhost "$0" "$@"
Expand All @@ -39,11 +34,7 @@ else
containerd-rootless-setuptool.sh nsenter -- sh -euc 'echo "options use-vc" >>/etc/resolv.conf'
fi

if [[ -e /workaround-issue-622 ]]; then
echo "WORKAROUND_ISSUE_622: Not enabling BuildKit (https://github.com/containerd/nerdctl/issues/622)" >&2
else
CONTAINERD_NAMESPACE="nerdctl-test" containerd-rootless-setuptool.sh install-buildkit-containerd
fi
CONTAINERD_NAMESPACE="nerdctl-test" containerd-rootless-setuptool.sh install-buildkit-containerd
containerd-rootless-setuptool.sh install-stargz
if [ ! -f "/home/rootless/.config/containerd/config.toml" ] ; then
echo "version = 2" > /home/rootless/.config/containerd/config.toml
Expand Down
Loading