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

Jump to bottom

Add support ZFS filesystem #1020

Merged
merged 1 commit into from
Jan 13, 2016
Merged

Add support ZFS filesystem #1020

merged 1 commit into from
Jan 13, 2016
+1,970 −3

Conversation

miguel250
Copy link
Contributor

To access ZFS stats from a container, it needs to have the privileged flag and zfsutilsneeds to be installed in the container. @vishh

sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 1196BA81F6B0FC61
sudo apt-get install python-software-properties
sudo apt-add-repository ppa:zfs-native/stable
apt-get update
apt-get install zfsutils

@cadvisorJenkinsBot
Copy link
Collaborator

Can one of the admins verify this patch?

@k8s-bot
Copy link
Collaborator

k8s-bot commented Dec 15, 2015

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

vishh
vishh reviewed Dec 15, 2015
View reviewed changes
fs/fs.go
@@ -128,7 +135,7 @@ func getDockerImagePaths(context Context) []string {
// TODO(rjnagal): Detect docker root and graphdriver directories from docker info.
dockerRoot := context.DockerRoot
dockerImagePaths := []string{}
for _, dir := range []string{"devicemapper", "btrfs", "aufs", "overlay"} {
for _, dir := range []string{"devicemapper", "btrfs", "aufs", "overlay", "zfs"} {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is ZFS a storage driver in docker?
I thought we can use ZFS instead of ext4 as the base fs.

Copy link
Contributor

Choose a reason for hiding this comment

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

nvm. Just noticed this page: https://docs.docker.com/engine/userguide/storagedriver/zfs-driver/

@vishh
Copy link
Contributor

vishh commented Dec 15, 2015

What about handling capacity and usage for the base filesystem under zfs? Is it already working ?

@miguel250
Add support ZFS filesystem
e0fef76 
- fix container usage stat not loading on first load
@miguel250
Copy link
Contributor Author

@vishh Yeah this set as well.

@vishh
Copy link
Contributor

vishh commented Dec 15, 2015

We should add an e2e test for this. Can you file an issue?

On Tue, Dec 15, 2015 at 12:45 PM, Miguel Perez notifications@github.com
wrote:

@vishh https://github.com/vishh Yeah this set as well.

—
Reply to this email directly or view it on GitHub
#1020 (comment).

@vishh
Copy link
Contributor

vishh commented Dec 15, 2015

ok to test

On Tue, Dec 15, 2015 at 12:46 PM, Vishnu Kannan vishnuk@google.com wrote:

We should add an e2e test for this. Can you file an issue?

On Tue, Dec 15, 2015 at 12:45 PM, Miguel Perez notifications@github.com
wrote:

@vishh https://github.com/vishh Yeah this set as well.

—
Reply to this email directly or view it on GitHub
#1020 (comment).

@miguel250
Copy link
Contributor Author

Just created #1021

@vishh vishh closed this Dec 17, 2015
@vishh vishh reopened this Dec 17, 2015
@cadvisorJenkinsBot
Copy link
Collaborator

Can one of the admins verify this patch?

@vishh
Copy link
Contributor

vishh commented Dec 17, 2015

ok to test

@vishh
Copy link
Contributor

vishh commented Dec 17, 2015

@jimmidyson: I wonder why the network tests are flaky :(

The e2e failures are unrelated to this PR. So this PR LGTM

@miguel250
Copy link
Contributor Author

Hi @vishh,
is there anything else that needs to be done for this pull request?

@vishh
Copy link
Contributor

vishh commented Jan 8, 2016

Oops. This fell out of my radar. Sorry!

@vishh vishh closed this Jan 8, 2016
@vishh vishh reopened this Jan 8, 2016
@vishh
Copy link
Contributor

vishh commented Jan 8, 2016

@cadvisorJenkinsBot: test this please

@vishh vishh closed this Jan 13, 2016
@vishh vishh reopened this Jan 13, 2016
@vishh
Copy link
Contributor

vishh commented Jan 13, 2016

ok to test

@k8s-bot
Copy link
Collaborator

k8s-bot commented Jan 13, 2016

Jenkins GCE e2e

Build/test passed for commit e0fef76.

vishh added a commit that referenced this pull request Jan 13, 2016
@vishh
Merge pull request #1020 from miguel250/zfs-support
01434a1 
Add support ZFS filesystem
@vishh vishh merged commit 01434a1 into google:master Jan 13, 2016
@DanielDent
Copy link

I'm unclear if zfs support is supposed to be working right now. For me, it does not appear to be.. On systems with ZFS, I get complaints about /dev/zfs being missing. If I provide the /dev/zfs device by bind mounting the host's device, then I get new complaints about zfs libraries being missing.

@dashpole dashpole mentioned this pull request Mar 16, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@miguel250 @cadvisorJenkinsBot @k8s-bot @vishh @DanielDent