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

docker image always create a /data volume. #197

Closed
janvda opened this issue Sep 4, 2020 · 8 comments
Closed

docker image always create a /data volume. #197

janvda opened this issue Sep 4, 2020 · 8 comments

Comments

@janvda
Copy link

janvda commented Sep 4, 2020

I noticed that the latest docker images at docker hub are automatically creating a /data volume:
See line 29: https://hub.docker.com/layers/nodered/node-red/latest/images/sha256-2a3af8fff798419e72c6f9461b4157d153f80860f00b73d5bad2bc906a0bdf0d?context=explore

image

This means that documentation at https://nodered.org/docs/getting-started/docker#managing-user-data is not fully inline.
it says:
image
while in fact the /data folder is also persisted in case no bind mount or named data volume is specified due to the line 29 in dockerfile.

I am just wondering if the VOLUME command is a good idea.
Especially when your docker files contain also the flows you want to build/deploy as is specified in section "Dockerfile which copies in local resources"

In that case you need to delete the data volume that is automatically created when deploying the image for a new version which has updated flows - otherwise it will still use the old flows.

Would it not be better to delete the VOLUME command ?

@natcl
Copy link
Contributor

natcl commented Sep 5, 2020

I was wondering the same thing. When copying flows upon image build it will only work on first build which can be quite confusing...

@janvda
Copy link
Author

janvda commented Sep 7, 2020

@janvda
Copy link
Author

janvda commented Sep 7, 2020

In order to test @dceejay has put a dev build (without the VOLUME config) up on the dev fork https://hub.docker.com/r/nodered/node-red-dev so you can try pulling direct from there. Does so far seem to be working fine for him - but obviously need others to test.

Take care that this test build might be overwritten later !

I have tested it with nodered/node-red-dev:test-12 and this worked for me. I have tested the 2 cases:

  • without defining a volume for /data
  • with defining a named volume for /data in my docker compose file.

Both cases are working as expected.

@PaulWieland
Copy link

Just leaving my 2 cents - I vote for removing the volume from the docker image. The user can define it if they want when the spin the container up. With the current implementation, you get a data volume whether you want one or not.

In my case, I don't want one because it makes upgrading NodeRED and the subnodes more difficult. In our environment, the user code (the flows) should be managed by git (which I recently found out you can do with the projects feature of NodeRED).

So the idea I have is to make the container static with no volume. Dev can be done in one container, and when the flow is to be deployed to production it can be pushed to git, then pulled by a production container.

When it's time to upgrade NodeRED, deploy a new image and container, then pull the flows from git. The only issue I have to work out is how to automatically install the latest revisions of all of the NodeRED Nodes that the flow depends on when doing the upgrade.

@edorgeville
Copy link

edorgeville commented Sep 8, 2020

Adding my +1 to removing the VOLUME instruction from the official image. The only reason I see to keep this is to protect users new to docker, that would docker run without adding a volume, losing their work when they upgrade. This could be solved by adding a notice in the README, IMO.

Here is our workflow:
For most project, we COPY the flows.json in our Dockerfile. This doesn't work with the official node-red image because of the volume, so we use the official node image and install node-red during the build.
For development, we simply add a volumes: - './src:/data' in ourdocker-compose.override.yml so the changes can be committed on the host.

@RaymondMouthaan
Copy link
Contributor

RaymondMouthaan commented Sep 8, 2020

hi all, @dceejay contacted me to have a look at this discussion going on about VOLUME.

The only reason I see to keep this is to protect users new to docker, that would docker run without adding a volume, losing their work when they upgrade.

The reason above was why VOLUME was added to the Dockerfile initially. This makes sure (even without explicit defining the volume by user) that Docker persists the Node-RED data to the host. Removing it from the Dockerfile could mean that one loses their hard work and long hours when the volume was not explicit given by the user in either their docker run, docker stack or compose command. Not only in case of an upgrade but in any case to redeploy a new container.

However, considering all above arguments to remove VOLUME from the Dockerfile (apart from the solution to build you own Docker image, where you would have full control), I would agree to remove it indeed, to be more flexible. Current Node-RED Docker documentation should be sufficient for starters to instruct to define a volume to persist their data, but we might consider to check all examples and add the 'volume' binding in each. Also we could add a note / warning / bold text to use volume unless you know what your are doing (like you guys 👍). This should prevent users to loose their data.

For current users that forgot the volume binding in their commands while they deployed their container, will loose their data once VOLUME is removed. A solution to this is to add instructions to copy the data dir from the running container to a save place, before upgrading to the new image.

@dceejay & @knolleary
Long story short, I agree to remove VOLUME (to be more flexible) as long as beginners are well instructed by documentation.

Cheers,
Ray

@dceejay
Copy link
Member

dceejay commented Sep 15, 2020

@jvda et al, I have just pushed up a new dev version to dockerhub - this has no VOLUME as before- but does have a default dummy flow with a warning node in it to help remind folk who don’t set a volume… I’d appreciate some feedback a) if this is “a good thing” and b) that it doesn’t break things when you do add an external mount etc (My testing says it doesn’t but…)…

@hammoaj - point about OpenStack noted… not sure what to do about that at the moment - but does mean we wont move this dev to master until we have a plan. Has that failure mode been verified using the dev build without VOLUME ? (edited)

@dceejay
Copy link
Member

dceejay commented Oct 2, 2020

Now merged to master so closing here

@dceejay dceejay closed this as completed Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants