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.

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

Add filesharing support #3269

Merged
merged 7 commits into from
Aug 4, 2022
Merged

Add filesharing support #3269

merged 7 commits into from
Aug 4, 2022

Conversation

cfergeau
Copy link
Contributor

@cfergeau cfergeau commented Jul 21, 2022

This PR uses vfkit virtiofs support in order to implement file sharing for podman containers.
This allows to use 'podman-remote run --volume /hostpath:/containerpath'

This is done by mounting the full user home dir in the VM through virtiofs. Then when using podman, one can specify a directory located in the user homedir, and it will transparently shared with the container.
Directories located outside of the user's home can't be shared with a container.
This also mounts the virtiofs filesystem using '-o context="system_u:object_r:container_file_t:s0"' to avoid selinux issues when accessing the data.

Instructions for testing:

$ crc start
$ eval $(crc podman-env)
$ mkdir ~/crc-podman
$ echo "hello podman\!" >~/crc-podman/hello.txt
$ podman run  --rm -v ~/crc-podman:/hello registry.access.redhat.com/ubi8/ubi cat /hello/hello.txt

(the directory to be shared with the container has to be in your home directory on the host)
podman -v ~/crc-podman:/hello:Z may or may not work, I had issues with macOS setxattr implementation erroring out on RO files, which was not the behaviour expected by podman. Since using mount -o context=... does the job without using :Z I did not dig further.

Fixes: Issue #3204

Relates to: Issue #3205, issue #3180, crc-org/vfkit#4, crc-org/machine#64

@cfergeau
Copy link
Contributor Author

macOS CI is currently failing because it's trying to use the released vfkit binary which does not have virtiofs support.

@gbraad
Copy link
Contributor

gbraad commented Jul 25, 2022

macOS CI is currently failing because it's trying to use the released vfkit binary which does not have virtiofs support.

Can you publish a pre-release version for testing purposes?

pkg/crc/machine/start.go Outdated Show resolved Hide resolved
pkg/crc/machine/start.go Outdated Show resolved Hide resolved
@cfergeau
Copy link
Contributor Author

macOS CI is currently failing because it's trying to use the released vfkit binary which does not have virtiofs support.

Can you publish a pre-release version for testing purposes?

Now that the vfkit changes have been merged, I've made such a release on my personal vfkit repo.

@cfergeau cfergeau force-pushed the virtiofs branch 2 times, most recently from f08d6f1 to c9da2d2 Compare July 25, 2022 15:25
go.mod Outdated Show resolved Hide resolved
@anjannath
Copy link
Member

anjannath commented Jul 26, 2022

Trying to run on windows, it tries to mount the home dir, i think this should only try to mount when the driver is vfkit or libvirt maybe (not sure if the libvirt driver already has implementation for virtiofs)

PS C:\Users\anath> crc start --log-level debug
DEBU CRC version: 2.6.0+c9da2d25
DEBU OpenShift version: 4.10.22
DEBU Podman version: 4.1.0
DEBU Running 'crc start'
DEBU Total memory of system is 34248990720 bytes
DEBU No new version available. The latest version is 2.6.0

[...]

DEBU No free space after /dev/sda4, nothing to do
DEBU Configuring shared directories
DEBU Using root access: Making / mutable
DEBU Running SSH command: sudo chattr -i /
DEBU SSH command results: err: <nil>, output:
DEBU Using root access: Creating C:\Users\anath
DEBU Running SSH command: sudo mkdir -p C:\Users\anath
DEBU SSH command results: err: <nil>, output:
DEBU Using root access: Making / immutable again
DEBU Running SSH command: sudo chattr +i /
DEBU SSH command results: err: <nil>, output:
DEBU Mounting tag dir0 at C:\Users\anath
DEBU Using root access: Mounting C:\Users\anath
DEBU Running SSH command: sudo mount -o context="system_u:object_r:container_file_t:s0" -t virtiofs dir0 C:\Users\anath
DEBU SSH command results: err: Process exited with status 32, output:
ssh command error:
command : sudo mount -o context="system_u:object_r:container_file_t:s0" -t virtiofs dir0 C:\Users\anath
err     : Process exited with status 32

on macOS its working as expected

@cfergeau
Copy link
Contributor Author

Trying to run on windows, it tries to mount the home dir, i think this should only try to mount when the driver is vfkit or libvirt maybe (not sure if the libvirt driver already has implementation for virtiofs)

More work is needed on platforms not supporting shared directories, either because they are too old (rhel7, macos11), or because the driver does not implement file sharing/does not support it. At the moment rhel7/macos11 are detected at the machine driver level, but I'm not even sure how crc behaves on these platforms. I hadn't thought at all of Windows, so I'll also need to take a look.

@cfergeau
Copy link
Contributor Author

The latest push uses upstream github.com/code-ready/machine instead of my fork, but does not address yet the Windows issue which was mentioned.

@anjannath
Copy link
Member

found a workaround/solution to the windows issue, if we add a GetSharedDirs function for the hyperv driver similar to the none driver then it avoids calling the mount functions and start doesn't error out.

diff --git a/pkg/drivers/hyperv/hyperv_windows.go b/pkg/drivers/hyperv/hyperv_windows.go
index c8ed4350..7c1d5a71 100644
--- a/pkg/drivers/hyperv/hyperv_windows.go
+++ b/pkg/drivers/hyperv/hyperv_windows.go
@@ -373,3 +373,7 @@ func (d *Driver) GetIP() (string, error) {

        return resp[0], nil
 }
+
+func (d *Driver) GetSharedDirs() ([]drivers.SharedDir, error) {
+    return []drivers.SharedDir{}, nil
+}

this might also work if added to the libvirt driver, but haven't tested

@cfergeau
Copy link
Contributor Author

found a workaround/solution to the windows issue, if we add a GetSharedDirs function for the hyperv driver similar to the none driver then it avoids calling the mount functions and start doesn't error out.

diff --git a/pkg/drivers/hyperv/hyperv_windows.go b/pkg/drivers/hyperv/hyperv_windows.go
index c8ed4350..7c1d5a71 100644
--- a/pkg/drivers/hyperv/hyperv_windows.go
+++ b/pkg/drivers/hyperv/hyperv_windows.go
@@ -373,3 +373,7 @@ func (d *Driver) GetIP() (string, error) {

        return resp[0], nil
 }
+
+func (d *Driver) GetSharedDirs() ([]drivers.SharedDir, error) {
+    return []drivers.SharedDir{}, nil
+}

this might also work if added to the libvirt driver, but haven't tested

I'm taking a slightly different approach, make the default GetSharedDirs implementation error out cfergeau/machine@26ae9c5
and then the machine drivers implementing shared dir support can override it. This will also be useful to report whether the host supports virtiofs or not.

The macos installer is now created as
out/macos-universal/crc-macos-installer.pkg
The artifact upload action was not updated for the new name
Even if the runtime sanity check for the installer fails, we might want
to access the installer which was built for testing. This commit moves
the artifact generation before the `crc start` attempt in the macOS
installer generation action.
@cfergeau cfergeau force-pushed the virtiofs branch 4 times, most recently from 1e14eb3 to 6fca834 Compare July 27, 2022 16:27
if err != nil {
return false
}
return strings.HasPrefix(macosVersion, "12.")
Copy link
Member

Choose a reason for hiding this comment

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

maybe return true also for macOS 12+ versions? otherwise we'll have to remember to change it when macOS 13 is released

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah thanks, initially wanted to use semver for this, then went with a shortcut. But yeah, definitely not very future proof this way :)

@cfergeau cfergeau force-pushed the virtiofs branch 2 times, most recently from db3336e to 340464b Compare July 28, 2022 10:04
This updates machine and vfkit to their latest versions to get file sharing
support.
This adds file sharing support to the vfkit driver.
This requires a version of vfkit with file sharing support (0.0.2 or
newer).
virtiofs file sharing is only available in macOS 12 and newer. This
commit makes use of sw_vers to return a proper error when running on too
old macOS versions.
This also skips vfkit shared directory configuration so that we can
start the VM anyway.
For all of this to work, vfkit has to be built on a macOS 11 builder,
the file sharing APIs will still be available when this binary is run on
macOS 12.
Building vfkit on macOS 12 and trying to run the resulting binary on
macOS 11 will badly fail though:

```
DEBU Running '/Users/teuf/.crc/bin/vfkit -v'
DEBU Command failed: signal: abort trap
DEBU stdout:
DEBU stderr: dyld: Symbol not found:
_OBJC_CLASS_$_VZMultipleDirectoryShare
  Referenced from: /Users/teuf/.crc/bin/vfkit (which was built for Mac OS X 12.0)
  Expected in:
/System/Library/Frameworks/Virtualization.framework/Versions/A/Virtualization
 in /Users/teuf/.crc/bin/vfkit
DEBU failed to run executable /Users/teuf/.crc/bin/vfkit: signal: abort trap
```
This commit adds file sharing support to `crc start`.
This uses the GetSharedDirs API from libmachine, and only supports
virtiofs for now.
File sharing is meant to be used together for podman support to support
`podman-remote -v somedir:/some-dir-in-container registry.example.com/container-image`
This is currently implemented similarly to what podman does: $HOME is
shared with the VM and mounted at the $HOME mountpoint. This way one can
use podman-remote $HOME/hostdir on the host, and the podman instance
running in the VM can also access $HOME/hostdir which is mounted in the
VM, and share it with the container.

We currently need to hardcode a selinux context on $HOME mounted in the
VM, mainly to overcome limitations in the macOS virtiofs implementation
which vfkit uses.

This fixes crc-org#3204
@cfergeau
Copy link
Contributor Author

I've updated the PR to make use of the just released vfkit 0.0.2

@praveenkumar
Copy link
Member

/retest

@anjannath
Copy link
Member

the e2e and integration tests are failing because it needs a new libvirt driver with the changes from crc-org/machine-driver-libvirt#101

@praveenkumar
Copy link
Member

@anjannath CI still failing with following issue

         Error creating machine: Error in driver during machine creation: Panic in the driver: runtime error: invalid memory address or nil pointer dereference
        goroutine 28 [running]:
        runtime/debug.Stack()
        	/usr/lib/golang/src/runtime/debug/stack.go:24 +0x65
        github.com/code-ready/machine-driver-libvirt/vendor/github.com/code-ready/machine/libmachine/drivers/rpc.(*StandardStack).Stack(0x0)
        	/builddir/build/BUILD/machine-driver-libvirt-0.13.4/_build/src/github.com/code-ready/machine-driver-libvirt/vendor/github.com/code-ready/machine/libmachine/drivers/rpc/server_driver.go:20 +0x17
        github.com/code-ready/machine-driver-libvirt/vendor/github.com/code-ready/machine/libmachine/drivers/rpc.trapPanic(0xc000277740)
        	/builddir/build/BUILD/machine-driver-libvirt-0.13.4/_build/src/github.com/code-ready/machine-driver-libvirt/vendor/github.com/code-ready/machine/libmachine/drivers/rpc/server_driver.go:72 +0x63
        panic({0x55811af6c560, 0x55811b29dc80})
        	/usr/lib/golang/src/runtime/panic.go:1038 +0x215
        github.com/code-ready/machine-driver-libvirt/pkg/libvirt.virtiofsSupported(0xc000296000)
        	/builddir/build/BUILD/machine-driver-libvirt-0.13.4/_build/src/github.com/code-ready/machine-driver-libvirt/pkg/libvirt/domain.go:180 +0xeb
        github.com/code-ready/machine-driver-libvirt/pkg/libvirt.domainXML(0xc000296000, {0x55811ac683cb, 0x3})
        	/builddir/build/BUILD/machine-driver-libvirt-0.13.4/_build/src/github.com/code-ready/machine-driver-libvirt/pkg/libvirt/domain.go:123 +0x9ef
        github.com/code-ready/machine-driver-libvirt/pkg/libvirt.(*Driver).Create(0xc000296000)
        	/builddir/build/BUILD/machine-driver-libvirt-0.13.4/_build/src/github.com/code-ready/machine-driver-libvirt/pkg/libvirt/libvirt.go:316 +0x95
        github.com/code-ready/machine-driver-libvirt/vendor/github.com/code-ready/machine/libmachine/drivers/rpc.(*RPCServerDriver).Create(0x2, 0xc00021ca00, 0x1)
        	/builddir/build/BUILD/machine-driver-libvirt-0.13.4/_build/src/github.com/code-ready/machine-driver-libvirt/vendor/github.com/code-ready/machine/libmachine/drivers/rpc/server_driver.go:83 +0x67
        reflect.Value.call({0xc0002800c0, 0xc000286158, 0x13}, {0x55811ac68639, 0x4}, {0xc000061ef8, 0x3, 0x3})
        	/usr/lib/golang/src/reflect/value.go:556 +0x845
        reflect.Value.Call({0xc0002800c0, 0xc000286158, 0x3}, {0xc00004e6f8, 0x3, 0x3})
        	/usr/lib/golang/src/reflect/value.go:339 +0xc5
        net/rpc.(*service).call(0xc00028c0c0, 0xc0002360f0, 0xc00004e730, 0xc00021c0a0, 0xc000290a00, 0xc0002360f0, {0x55811af3dec0, 0x55811b3283c0, 0x55811abe28a6}, {0x55811af3dec0, ...}, ...)
        	/usr/lib/golang/src/net/rpc/server.go:377 +0x239
        created by net/rpc.(*Server).ServeCodec
        	/usr/lib/golang/src/net/rpc/server.go:474 +0x405
        

@anjannath
Copy link
Member

/test integration-crc

@anjannath
Copy link
Member

/test all

1 similar comment
@anjannath
Copy link
Member

/test all

@anjannath anjannath force-pushed the virtiofs branch 3 times, most recently from e5f158e to 6bf731f Compare August 3, 2022 04:59
this update adds support for virtiofs directory sharing
Copy link
Member

@anjannath anjannath left a comment

Choose a reason for hiding this comment

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

Tested on linux, macOS and windows and its working as expected

@openshift-ci
Copy link

openshift-ci bot commented Aug 4, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anjannath

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Aug 4, 2022
@anjannath anjannath merged commit ab6e30d into crc-org:main Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants