feat(compute,api,update-server): Move system configs out of Dockerfile #2073
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sfoster1
added
api
sfoster1
requested review from
mcous,
andySigler,
btmorr,
sanni-t and
Laura-Danielle
sfoster1
force-pushed
the
api_server-installs-config-files-merge
branch
from
06465c5 to
59e95e6
Compare
mcous
reviewed
Aug 16, 2018
Dockerfile
Outdated
| ENV AUDIO_FILES /etc/audio | ||
| ENV USER_DEFN_ROOT /data/user_storage/opentrons_data/labware | ||
|
|
||
| COPY compute/container_setup.sh /usr/local/bin/container_setup.sh |
There was a problem hiding this comment.
[Style nitpick] All the other paths in this file have a leading ./
Dockerfile
Outdated
| COPY ./compute/conf/inetd.conf /etc/ | ||
| COPY ./compute/conf/nginx.conf /etc/nginx/nginx.conf | ||
| COPY ./compute/static /usr/share/nginx/html | ||
| # COPY ./compute/opentrons.asc . |
There was a problem hiding this comment.
Is it ok to remove these lines entirely?
api/opentrons/__init__.py
Outdated
| @@ -1,6 +1,7 @@ | |||
| import os | |||
| import sys | |||
| import json | |||
| import shutil | |||
| if [[ -z "$RUNNING_ON_PI" ]] | ||
| then | ||
| echo '[ $0 ] Forcing virtual smoothie board' | ||
| export ENABLE_VIRTUAL_SMOOTHIE=1 |
There was a problem hiding this comment.
Are we sure we want to do this? Running on a machine that isn't a Pi that is still connected to a real smoothie board seems like a valid dev use-case
| if [ -z $OT_ENVIRON_SET_UP ]; then | ||
| echo "[ $0 ] Configuring environment" | ||
|
|
||
| arg_running_on_pi=$1 |
There was a problem hiding this comment.
This variable doesn't appear to be used
| # if they are in fact still used | ||
| export OT_SETTINGS_DIR="" | ||
| export OT_SERVER_PORT=31950 | ||
| export OT_SERVER_UNIX_SOCKET_PATH=/tmp/aiohttp.sock |
There was a problem hiding this comment.
This PR might not be the time, but should we consider either ditching the Unix socket or aligning everything to use Unix sockets? At the very least, this is a really unhelpful socket name that we've been using
|
|
||
| echo "[ $0 ] API server setup beginning" | ||
|
|
||
| if [ ! -z $RUNNING_ON_PI ] ; then |
There was a problem hiding this comment.
[Nitpick, no change requested] Any reason to use ! -z $RUNNING_ON_PI here over -n "$RUNNING_ON_PI"?
There was a problem hiding this comment.
Slowly re-remembering bash conditionals, mostly.
| echo "[ $0 ] API server starting" | ||
| # mdns announcement | ||
| if [ ! -z $RUNNING_ON_PI ]; then | ||
| echo "[ $0 ] MDNS beginning" |
There was a problem hiding this comment.
Out of curiosity, why disable mDNS advertisements on non-Pis?
There was a problem hiding this comment.
This is one of a category of things in the container that only work at the very least on linux hosts and quite possibly only on the pi. I haven't been consistent with the rest of them, I think what we should do is create an issue to actually get a container running well on (osx, windows) docker.
|
|
||
| export ENABLE_NETWORKING_ENDPOINTS=true | ||
| echo "[ $0 ] Starting Opentrons API server" | ||
| python -m opentrons.server.main -U /tmp/aiohttp.sock opentrons.server.main:init |
There was a problem hiding this comment.
We should probably use the OT_SERVER_UNIX_SOCKET_PATH environment variable here
| @@ -44,6 +44,10 @@ robot. The script must: | |||
| Scripts in that directory will be executed in alphabetical order. The most straight-forward way to guarantee exectution | |||
| order is to prefix file names with numbers (e.g.: `00-configure-wifi` and `05-do-something-else`). | |||
|
|
|||
| ### System Configuration and Initialization | |||
There was a problem hiding this comment.
[Nice to have] While we're at improving documentation, does the top-level file RESIN_README.md mean anything anymore? If not, might be nice to ditch it in this PR
sfoster1
force-pushed
the
api_server-installs-config-files-merge
branch
from
59e95e6 to
763f12a
Compare
Codecov Report
@@ Coverage Diff @@
## edge #2073 +/- ##
==========================================
+ Coverage 32.86% 35.84% +2.98%
==========================================
Files 453 453
Lines 7312 7802 +490
==========================================
+ Hits 2403 2797 +394
- Misses 4909 5005 +96
Continue to review full report at Codecov.
|
sfoster1
force-pushed
the
api_server-installs-config-files-merge
branch
2 times, most recently
from
16cc3a5 to
c0fa188
Compare
2 tasks
btmorr
changed the title
btmorr
changed the title
Overhaul the container configuration and initialization to remove as much as possible from the Dockerfile and the volatile storage, preferring instead to live in files that are included in the api server wheel and unpacked to /data during initialization. Most configuration and setup scripts are no longer COPYd into /usr/local/bin. Instead, they are packed into the api server’s wheel (opentrons) in a /resources subdirectory. This resources subdirectory is recursive-included into the manifest, so putting a file into the directory will include it in the wheel. Because wheels do not run arbitrary code when they are installed, there is some external tooling to get the files out of wheel/resources and into their eventual home in /data/system. * Provisioning The compute/find_module_path.py is an executable python script that uses importlib to find the location of the opentrons module as Python would do (so it always gets the right one) without importing opentrons, which has significant side effects. The opentrons/api/opentrons/resources/scripts/provision-api-resources script uses find_module_path to find the current opentrons module and copy everything in /resources in the package to /data/system (the OT_CONFIG_DIR). This includes all the system configurations and scripts that were previously in compute/. The provision script is designed to be invoked during updates. This happens during the first boot of the container (more on this later) and during a software update via the update server. In the case of the update-server driven update, because we assume a user is never updating to 3.3 from 3.0 on a 3.0 Dockerfile, we can assume the provision script is in /data/system/scripts and is therefore in the path. The update server therefore invokes the provision script after installing a new api server, the provision script finds the right (newly-installed) opentrons module, and updates /data/resources with the new scripts. In the case of the first container boot, we rely on the one piece of initialization left in compute/. compute/container_setup.sh does the first-boot check that setup.sh used to do, and in addition to removing old api server installations and updating the cached container ID it runs the provision script - this time from the module installed in /usr/local/lib. * Container Initialization The Dockerfile CMD has been slightly simplified by symlinking the environment file into /etc/profile.d. This requires the use of bash’s -l flag to get a login shell in the docker container. Because the symlinks to the environment must be present when the shell starts, the environment script (opentrons/api/opentrons/resources/ot-environ.sh) is linked into /etc/profile.d by the Dockerfile (the rest of the configuration file symlinks are in opentrons/api/opentrons/resources/scripts/setup.sh). It is also linked twice - once from /data/system, which will be empty when the container boots for the first time; and once from the system installation of the api server in /usr/local/lib, as a fallback. The one-time environment variables ensure it is only executed once. Then, the container setup script runs. In most cases it doesn’t do much, but see above for the behavior during the first container boot. Since /data/system/scripts is in the path, we still call setup.sh and start.sh. In addition to what it used to do, setup.sh now makes symlinks for most of the configuration files we had been COPYing into Docker. These are now in /data/system. start.sh is pretty similar to before. * Container Building The Dockerfile is now parameterized to make building a local container slightly easier. Invoking docker build with no arguments builds a container for the pi (it has to, since this is what Resin does). Invoking docker build with the arguments to change the base image and clear RUNNING_ON_PI will build a container for local machines. To make this easier, there is a new top-level Makefile target, api-local-container, that invokes docker build with the correct arguments. Note that the local container still doesn’t 100% work; it needs more love to get over things like not having a dbus socket on all hosts. * Misc - Sweet new motd - Removed some of the nmcli commands used for janitoring the static ipv6 routes - Removed the nginx root server block and deleted the static files it was - serving - hardcoded some ports to get away from infinite env vars we can’t trust - Fixed an issue in the update server where it was sending the repr() of tracebacks in 500 messages for /server/update. Now it sends the result of traceback.format_tb() Closes #1114
sfoster1
force-pushed
the
api_server-installs-config-files-merge
branch
from
c0fa188 to
ee3872c
Compare
b-cooper
pushed a commit
that referenced
this pull request
Aug 29, 2018
#2073) Overhaul the container configuration and initialization to remove as much as possible from the Dockerfile and the volatile storage, preferring instead to live in files that are included in the api server wheel and unpacked to /data during initialization. Most configuration and setup scripts are no longer COPYd into /usr/local/bin. Instead, they are packed into the api server’s wheel (opentrons) in a /resources subdirectory. This resources subdirectory is recursive-included into the manifest, so putting a file into the directory will include it in the wheel. Because wheels do not run arbitrary code when they are installed, there is some external tooling to get the files out of wheel/resources and into their eventual home in /data/system. * Provisioning The compute/find_module_path.py is an executable python script that uses importlib to find the location of the opentrons module as Python would do (so it always gets the right one) without importing opentrons, which has significant side effects. The opentrons/api/opentrons/resources/scripts/provision-api-resources script uses find_module_path to find the current opentrons module and copy everything in /resources in the package to /data/system (the OT_CONFIG_DIR). This includes all the system configurations and scripts that were previously in compute/. The provision script is designed to be invoked during updates. This happens during the first boot of the container (more on this later) and during a software update via the update server. In the case of the update-server driven update, because we assume a user is never updating to 3.3 from 3.0 on a 3.0 Dockerfile, we can assume the provision script is in /data/system/scripts and is therefore in the path. The update server therefore invokes the provision script after installing a new api server, the provision script finds the right (newly-installed) opentrons module, and updates /data/resources with the new scripts. In the case of the first container boot, we rely on the one piece of initialization left in compute/. compute/container_setup.sh does the first-boot check that setup.sh used to do, and in addition to removing old api server installations and updating the cached container ID it runs the provision script - this time from the module installed in /usr/local/lib. * Container Initialization The Dockerfile CMD has been slightly simplified by symlinking the environment file into /etc/profile.d. This requires the use of bash’s -l flag to get a login shell in the docker container. Because the symlinks to the environment must be present when the shell starts, the environment script (opentrons/api/opentrons/resources/ot-environ.sh) is linked into /etc/profile.d by the Dockerfile (the rest of the configuration file symlinks are in opentrons/api/opentrons/resources/scripts/setup.sh). It is also linked twice - once from /data/system, which will be empty when the container boots for the first time; and once from the system installation of the api server in /usr/local/lib, as a fallback. The one-time environment variables ensure it is only executed once. Then, the container setup script runs. In most cases it doesn’t do much, but see above for the behavior during the first container boot. Since /data/system/scripts is in the path, we still call setup.sh and start.sh. In addition to what it used to do, setup.sh now makes symlinks for most of the configuration files we had been COPYing into Docker. These are now in /data/system. start.sh is pretty similar to before. * Container Building The Dockerfile is now parameterized to make building a local container slightly easier. Invoking docker build with no arguments builds a container for the pi (it has to, since this is what Resin does). Invoking docker build with the arguments to change the base image and clear RUNNING_ON_PI will build a container for local machines. To make this easier, there is a new top-level Makefile target, api-local-container, that invokes docker build with the correct arguments. Note that the local container still doesn’t 100% work; it needs more love to get over things like not having a dbus socket on all hosts. * Misc - Sweet new motd - Removed some of the nmcli commands used for janitoring the static ipv6 routes - Removed the nginx root server block and deleted the static files it was - serving - hardcoded some ports to get away from infinite env vars we can’t trust - Fixed an issue in the update server where it was sending the repr() of tracebacks in 500 messages for /server/update. Now it sends the result of traceback.format_tb() Closes #1114
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
overview
Overhaul of the container configuration and initialization to remove as much as
possible from the Dockerfile and the volatile storage, preferring instead to
live in files that are included in the api server wheel and unpacked to /data
during initialization.
Most configuration and setup scripts are no longer COPYd into /usr/local/bin.
Instead, they are packed into the api server’s wheel (opentrons) in a
/resources subdirectory. This resources subdirectory is recursive-included
into the manifest, so putting a file into the directory will include it in the
wheel.
Because wheels do not run arbitrary code when they are installed, there is some
external tooling to get the files out of wheel/resources and into their eventual
home in /data/system.
The compute/find_module_path.py is an executable python script that uses
importlib to find the location of the opentrons module as Python would do (so it
always gets the right one) without importing opentrons, which has significant
side effects.
The opentrons/api/opentrons/resources/scripts/provision-api-resources script
uses find_module_path to find the current opentrons module and copy everything
in /resources in the package to /data/system (the OT_CONFIG_DIR). This includes
all the system configurations and scripts that were previously in compute/.
The provision script is designed to be invoked during updates. This happens
during the first boot of the container (more on this later) and during a
software update via the update server.
In the case of the update-server driven update, because we assume a user is
never updating to 3.3 from 3.0 on a 3.0 Dockerfile, we can assume the provision
script is in /data/system/scripts and is therefore in the path. The update
server therefore invokes the provision script after installing a new api server,
the provision script finds the right (newly-installed) opentrons module, and
updates /data/resources with the new scripts.
In the case of the first container boot, we rely on the one piece of
initialization left in compute/. compute/container_setup.sh does the first-boot
check that setup.sh used to do, and in addition to removing old api server
installations and updating the cached container ID it runs the provision script
The Dockerfile CMD has been slightly simplified by symlinking the environment
file into /etc/profile.d. This requires the use of bash’s -l flag to get a login
shell in the docker container.
Because the symlinks to the environment must be present when the shell starts,
the environment script (opentrons/api/opentrons/resources/ot-environ.sh) is
linked into /etc/profile.d by the Dockerfile (the rest of the configuration file
symlinks are in opentrons/api/opentrons/resources/scripts/setup.sh). It is also
linked twice - once from /data/system, which will be empty when the container
boots for the first time; and once from the system installation of the api
server in /usr/local/lib, as a fallback. The one-time environment variables
ensure it is only executed once.
Then, the container setup script runs. In most cases it doesn’t do much, but see
above for the behavior during the first container boot.
Since /data/system/scripts is in the path, we still call setup.sh and start.sh.
In addition to what it used to do, setup.sh now makes symlinks for most of the
configuration files we had been COPYing into Docker. These are now in
/data/system. start.sh is pretty similar to before.
The Dockerfile is now parameterized to make building a local container slightly
easier. Invoking docker build with no arguments builds a container for the
pi (it has to, since this is what Resin does). Invoking docker build with the
arguments to change the base image and clear RUNNING_ON_PI will build a
container for local machines. To make this easier, there is a new top-level
Makefile target, api-local-container, that invokes docker build with the correct
arguments. Note that the local container still doesn’t 100% work; it needs more
love to get over things like not having a dbus socket on all hosts.
changelog
tracebacks in 500 messages for /server/update. Now it sends the result of traceback.format_tb()
Note that Dockerfiles without these changes will not get the new configuration and startup, even if an api server is properly installed and provisioned, since the environment script won't be sourced.
review requests
Test:
@andySigler Would you mind double-checking ot-environ.sh and the contents of api/opentrons/resources and making sure that the stuff you need for the factory is OK?
Everybody else Let me know if there's more testing I should be doing.