-
Notifications
You must be signed in to change notification settings - Fork 74
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
Refactor run-nodered script #816
Conversation
Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
488dcfa
to
d3a67c0
Compare
Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
# allow starting the nodered container from inside a devcontainer | ||
|
||
# Create docker volume to contain nodered config | ||
docker volume create nodered-config-volume |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nodered-config-volume
is quite generic, not sure if it might collide with other node red instances. Would it be possible to use a local path inside build
for example which gets mounted into the container instead of an volume. This would also ease the access to the config file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more or less the way how the script worked before, I changed it to be able to start this container from inside a devcontainer, see below.
I changed the name of the volume and temporarily container to have a prefix everest-
# In the following a volume is created to contain the nodered config, this is done to | ||
# allow starting the nodered container from inside a devcontainer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does using a volume allow to start the node-red container from inside a dev container?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workspace inside the devcontainer is mounted to an other path. So you would need to know about the host path of the flow file.
docker run
is executed on host with host paths
docker cp
is executed with container's paths
This way it is possible to copy a file with only knowing the devcontainer paths into a docker volume which then can be mounted by name into the nodered container
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, well this is really ugly. These docker inside docker scenarios are quite special, not sure whether we should support these - they might break quite easily. This script here basically only works, if the devcontainer has been launched correctly - which is an assumption I wouldn't necessarily make.
Does this script work, when executed from the host (i.e. outside of the container?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not real docker inside docker, because it is more like remote controlling the hosts docker socket.
This script is designed to work inside and outside the devcontainer. Of course it needs to be rebuild when changed because of paths
Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
I'm getting the following error when running a nodered runscript (in this case
|
Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
I added comamnd to set the UID and GID of the docker volume, could you try again pelase? |
This seems to have fixed it 👍 |
Describe your changes
Issue ticket number and link
Checklist before requesting a review
Requires