fix: update runc to 1.3.4 in Finch CI for symlink bind-mount support#8655
fix: update runc to 1.3.4 in Finch CI for symlink bind-mount support#8655bnusunny merged 2 commits intoaws:developfrom
Conversation
samcli/local/docker/container.py
Outdated
| try: | ||
| symlink_path = file.path | ||
| symlink_target = os.readlink(symlink_path) | ||
| os.remove(symlink_path) |
There was a problem hiding this comment.
Why do we remove the symlink_path (file.path) and recreate it againin the next line?
There was a problem hiding this comment.
When SAM CLI mounts symlinks into containers (e.g. during sam local invoke with --build-in-source), Finch/containerd's runc fails with a "not a directory" error. Unlike Docker, which transparently handles symlinks at mount target paths, Finch expects an actual directory at the mount point inside the container's rootfs.
The fix does two things:
In _create_mapped_symlink_files, before mounting a symlink, it temporarily replaces the symlink on the host with an empty directory. This lets the container runtime create a valid bind mount. The original symlink target is recorded in self._replaced_symlinks.
A new _restore_mapped_symlinks method restores the original symlinks after the container is deleted (in the delete method), so the host filesystem is left in its original state for subsequent invocations.
There was a problem hiding this comment.
Shouldn't the symlink be restored after the container is created? why do we restore the symlink after the container is deleted?
There was a problem hiding this comment.
No, we can't restore the symlink right after the container is created. The directory has to be there when the container is running. So we can only restore the symlink after the container is deleted.
There was a problem hiding this comment.
Let's reach out to Finch team to see if they can support the same as what Docker does so we don't have to change how we implement this for different containers
|
If we are just fixing the test, let's add a skip for finch for now. And a TODO to add back when it's fixed |
e13de23 to
13ac210
Compare
|
Since Finch team confirmed this is a bug, we are skipping this test and will add it back when it is fixed. |
Finch/containerd does not support bind-mounting over symlinks (fails with 'not a directory'). The Finch team confirmed this is a known bug on their side. Instead of working around it in SAM CLI, skip the affected integration tests when CONTAINER_RUNTIME=finch and remove the verification workflow. Tests will be re-enabled once Finch ships the fix.
13ac210 to
5c3dbc9
Compare
Instead of skipping symlink integration tests on Finch, update runc to 1.3.4 in the CI pipeline which fixes the underlying bind-mount over symlink issue in containerd/runc. - Install runc 1.3.4 after Finch package install, before service start - Revert test skip decorator on TestInvokeBuildInSourceSymlinkedModules
|
The fix is to upgrade runc to 1.3.4 |
Finch/containerd fails with 'not a directory' when bind-mounting over a symlink due to a bug in the bundled runc. The Finch team confirmed the fix is to update runc to 1.3.4.
Instead of skipping the affected integration tests, this PR updates runc to 1.3.4 in the GitHub Actions CI pipeline after Finch is installed, which resolves the underlying issue.
Changes
integration-tests.yml, replacing the binaries at/usr/sbin/runcand/usr/bin/runc@skipIftest decorator onTestInvokeBuildInSourceSymlinkedModulesso symlink tests run on all runtimes