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

Driver: none host path should be /var/tmp for PVCs #7511

Closed
jradtilbrook opened this issue Apr 8, 2020 · 18 comments
Closed

Driver: none host path should be /var/tmp for PVCs #7511

jradtilbrook opened this issue Apr 8, 2020 · 18 comments
Labels
addon/storage-provisioner Issues relating to storage provisioner addon co/none-driver good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@jradtilbrook
Copy link

Sorry in advance but the issue template really does not apply at all to my issue.

Abstract

When using the none driver in a linux environment I get issues with PVCs after restarting. This is due to the /tmp directory being a tmpfs filesystem.

Details

Many mainstream linux distributions adopted systemd as the init system quite a while ago, and under this system, the /tmp directory is managed as a mount service and is mounted as a tmpfs filesystem. This is ideal for performance since it uses memory for anything in that directory. My issue is that when I have a pod that uses a persistent volume claim and I restart my machine the directory backed by the host is cleared (since it was in memory). This causes the pod to not start back up when I start minikube. I can get around this by disabling the tmp.mount systemd service (and therefore let it default to the disk in my machine) and adding some rules to not clean up the hostpath-provisioner directory inside /tmp but this is highly undesirable since many other applications use the directory and benefit from tmpfs. I don't want to have to do that just so PVCs will work in minikube.

If you see this article describing the tmp system interaction with systemd they recommend using /var/tmp for temporary storage that is not in memory and hence can be configured to remain after reboots. I think the minikube binary should be modified to use this directory instead. This will solve my issue with PVCs and allow me to keep tmp as tmpfs.

I'm willing to provide a PR, granted the maintainers understand my problem and agree with my proposed solution. Otherwise, if there are other ideas on how to resolve this I'd love to know and contribute any way I can. Cheers!

@afbjorklund
Copy link
Collaborator

afbjorklund commented Apr 8, 2020

The paths under /tmp are just mount points anyway, the real data is kept on /mnt/sda1

https://github.com/kubernetes/minikube/blob/v1.9.2/deploy/iso/minikube-iso/package/automount/minikube-automount#L124_L130

@afbjorklund afbjorklund added addon/storage-provisioner Issues relating to storage provisioner addon co/none-driver labels Apr 8, 2020
@jradtilbrook
Copy link
Author

The paths under /tmp are just mount points anyway, the real data is kept on /mnt/sda1

https://github.com/kubernetes/minikube/blob/v1.9.2/deploy/iso/minikube-iso/package/automount/minikube-automount#L124_L130

Oh sorry for the lack of detail. My problem is present when using the none driver. So I believe the data is stored directly at the path without any binds

@afbjorklund
Copy link
Collaborator

My problem is present when using the none driver. So I believe the data is stored directly at the path without any binds

I know, but when you are using the none driver you will have to do these things (install docker, setup symlinks) yourself. Could have been documented better, though...

@afbjorklund afbjorklund added the kind/documentation Categorizes issue or PR as related to documentation. label Apr 9, 2020
@afbjorklund
Copy link
Collaborator

Moving the temporary mountpoint from /tmp/hostpath-provisioner to /var/tmp/hostpath-provisioner seems like a workaround. I think that it might be a parameter "soon" ? #3318

There was also the issue with the storage-provisioner pod mounting the /tmp directory. So this needs to be updated the same way, and I think is one reason why it is a bind mount (not symlink) now.

@jradtilbrook
Copy link
Author

My problem is present when using the none driver. So I believe the data is stored directly at the path without any binds

I know, but when you are using the none driver you will have to do these things (install docker, setup symlinks) yourself. Could have been documented better, though...

I don't see why you would need to do that. Installing docker makes sense but minikube takes care of creating the /tmp/hostpath-provisioner directory and mounting that into the storage-provisioner container. So why can't it instead use the better purposed /var/tmp/hostpath-provisioner?

Moving the temporary mountpoint from /tmp/hostpath-provisioner to /var/tmp/hostpath-provisioner seems like a workaround. I think that it might be a parameter "soon" ? #3318

There was also the issue with the storage-provisioner pod mounting the /tmp directory. So this needs to be updated the same way, and I think is one reason why it is a bind mount (not symlink) now.

I only partly agree that this is a workaround. Under the assumption that most users that are using the none driver are on modern systems backed by systemd its safe to assume that /tmp is a tmpfs and therefore should follow the documentation I linked in the issue description about using the more appropriate temporary directory located at /var/tmp.

However, if it is configurable then that kills 2 birds with one stone. I still believe that the default should be /var/tmp and that either #3318 can introduce that or I can provide a patch prior to that issue that can be incorporated as well. What do you think?

@afbjorklund
Copy link
Collaborator

So why can't it instead use the better purposed /var/tmp/hostpath-provisioner?

It could, but it would be more meaningful to store "persistent" data in a non-tmp directory ?

So the plan was to allow to select something similar to /mnt/sda1/hostpath-provisioner

But /var might also be too small / used for logs and stuff. So I'm not sure that is the best.

So the user can select something appropriate ? See also /data, used for manual volumes.

@afbjorklund
Copy link
Collaborator

It was supposed to have been revisited in #5144, but I'm not sure anything actually happened ?

And as noticed in #7218 it has been a couple of years since we had a new storage-provisioner

@gattytto
Copy link

gattytto commented Apr 10, 2020

@jradtilbrook it is not a bug, by default minikube provisions ephemereal storage, which resides in /tmp by convention.

the version of minikube you are using is not affected by the bug @afbjorklund mentions (#7218) so if you want data to persist upon reboots, you have to use storage classes and persistent volumes.

and if you update to the latest minikube version and start being affected by #7218 then also storage classes and persistent volumes to force the hostPath folder is to be enforced.

@afbjorklund
Copy link
Collaborator

afbjorklund commented Apr 10, 2020

by default minikube provisions ephemereal storage, which resides in /tmp by convention.

That is not just true, it is only an unfortunate oversight when using the none driver...

All persistent volumes are supposed to be persistent, by mounting a persistent location.

The workaround is to do a similar setup as is done in the minikube.iso, but on the host:

    mkdir -p /var/tmp/hostpath-provisioner
    mkdir /tmp/hostpath-provisioner
    sudo mount --bind /var/tmp/hostpath-provisioner /tmp/hostpath-provisioner

At the moment the /tmp is hardcoded in the storage-provisioner, so this is quickest

@gattytto
Copy link

gattytto commented Apr 11, 2020

by default minikube provisions ephemereal storage, which resides in /tmp by convention.

That is not just true, it is only an unfortunate oversight when using the none driver...

All persistent volumes are supposed to be persistent, by mounting a persistent location.

The workaround is to do a similar setup as is done in the minikube.iso, but on the host:

    mkdir -p /var/tmp/hostpath-provisioner
    mkdir /tmp/hostpath-provisioner
    sudo mount --bind /var/tmp/hostpath-provisioner /tmp/hostpath-provisioner

At the moment the /tmp is hardcoded in the storage-provisioner, so this is quickest

very short and quick workaround indeed, thank you.

@jradtilbrook
Copy link
Author

Okay, in the meantime I will also use that bind mount. And it seems like the answer to this issue is to parameterise the directory used for PVCs, which will be resolved in one of #3318 or #5144. Is that correct?

If so, I'll close this issue and can carry on any discussion on whichever of those you would prefer - I am still willing and able to provide a PR for this, but parameterisation logic will just take more time for me to implement.

@sharifelgamal sharifelgamal added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Apr 15, 2020
@morhook
Copy link

morhook commented Apr 16, 2020

For fixing this problem on my Ubuntu (using docker driver), I had to run this replacements command to the ones mentioned:

minikube ssh
sudo su
echo '/var/tmp/hostpath-provisioner /tmp/hostpath-provisioner none defaults,bind 0 0' > /etc/fstab

@tstromberg
Copy link
Contributor

This seems reasonable: the P in PVC is persistent after all. I think it would just take modifying this line:

var pvDir = "/tmp/hostpath-provisioner"

Anyone up for doing so and testing it?

@tstromberg tstromberg added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Apr 22, 2020
@jradtilbrook
Copy link
Author

jradtilbrook commented Apr 23, 2020

This seems reasonable: the P in PVC is persistent after all. I think it would just take modifying this line:

var pvDir = "/tmp/hostpath-provisioner"

Anyone up for doing so and testing it?

I actually think that isn't all that's needed. The storage provisioner addon mounts a hostpath of /tmp to the pods /tmp. If we just changed the hostpath to /var/tmp the pod would still see it as /tmp and all its internal paths can remain the same, but it will map to /var/tmp on the host.

So I think modifying this line:

Happy to provide a PR!

@afbjorklund
Copy link
Collaborator

But it is still just a workaround, /tmp wasn't intended for storage any more than /mnt is...
And there is just something icky about storing things "persistently" in a temporary directory ?

Note that /var/tmp wasn't being persisted either (on the VM), until most recently. #7740
I won't actively block this PR, but it would be better if we updated the storage-provisioner.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 22, 2020
@morhook
Copy link

morhook commented Apr 13, 2021

This has been fixed on my minikube installation (with docker driver). Thanks a lot!

@socza
Copy link

socza commented Jan 26, 2022

For drive=none, I added the file /etc/tmpfiles.d/tmp.conf and filled it in as follows:
-D /tmp 1777 root root -
+d /tmp 1777 root root 1s
+x /tmp/hostpath-provisioner/*

now the /tmp/hostpath-provisioner folder is persistent ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon/storage-provisioner Issues relating to storage provisioner addon co/none-driver good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants