Rework docker development environment #2859
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2859 +/- ##
=======================================
Coverage 56.69% 56.69%
=======================================
Files 602 602
Lines 43971 43971
=======================================
Hits 24931 24931
Misses 19040 19040 ☔ View full report in Codecov by Sentry. |
lunkwill42
force-pushed
the
feature/docker-python-improvements
branch
from
2d14d19
to
ff3e26b
Compare
lunkwill42
force-pushed
the
feature/docker-python-improvements
branch
from
ff3e26b
to
c4a0510
Compare
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Not reproducible for me. |
tools/docker/web.sh
Outdated
|
|
||
| # Ensure latest NAV code is built | ||
| mydir=$(dirname $0) | ||
| "$mydir/build.sh" | ||
| cd /source | ||
|
|
||
|
|
||
| django-admin check && exec django-admin runserver --insecure 0.0.0.0:80 | ||
| django-admin check && exec django-admin runserver --insecure 0.0.0.0:8080 |
There was a problem hiding this comment.
Missing newline after? The side-by-side diff in github ends with a red stop-sign for the new code.
There was a problem hiding this comment.
bah humbug #blames-editor
There was a problem hiding this comment.
Shouldn't pre-commit handle this for us though... lesigh
There was a problem hiding this comment.
probably... should be cleaned up in the latest push, though.
There was a problem hiding this comment.
Pre-commit doesn't handle it for us since we don't have the hook end-of-file-fixer added (like we have in Argus). Should I make a PR to add that?
Describe the bug With changes in this PR, accessing the Netmap tool in the web leads to an alert pop-up and an error message in the browser console. No graph is loaded. To Reproduce Steps to reproduce the behavior:
Expected behavior Netmap tool in web works without prompting alerts or error messages, and the graph is loaded properly with all the nodes. Screenshots 
Tracebacks Logs from the Environment (please complete the following information):
Additional context MacBook with Apple chip M2. |
Still not reproducible for me But, you could be re-using an outdated cache mount, @podliashanyk ? Have you completely destroyed your docker compose environment and removed the associated volumes? Specifically, docker volume rm nav_nav_cache |
|
Specifically, these volumes are defined for re-use in Lines 107 to 111 in c4a0510 |
Sounds like a good candidate for a "make nuke"-rule. |
|
I also cannot reproduce the netmap bug and the |
Just in case I have also docker pruned everything related to NAV containers, and rebuilt everything from scratch. The bug still appears. I noticed that the same happens with Watch Dog tool, except that /watchdog/ page doesn't load at all. |
Sure, if you can find a volume nuke command that works in all cases. |
|
My |
The build step used to be there for "caching" purposes, and for the build files to be stored with owner privileges for the `nav` user, rather than dropping files owned by root into the mounted `/source` directory. Using the `build` module does not give this effect, since it does everything inside a separate virtual environment, meaning the subsequent `pip install` command receives no caching benefit. In fact, we just end up with two redundant (and potentially slow) builds of NAV and its dependencies.
This enables us to better detect which build steps are taking too much time.
This adds Docker image build time cache mounts for the pip cache directory, in an effort to reduce build times for the docker-based dev environment.
When docker image rebuilds are needed, using cache mounts can improve the rebuild speed, by ensuring APT's download cache is persistent between rebuilds.
Prep for installing everything NAV-related as an unprivileged user inside the development container. This also ensures installating Python stuff will keep working once the image transitions to Debian Bookworm, which will not allow pip to install packages at the system level.
These now all need to be able to run as an unprivileged user (but they can gain root using sudo, if need be)
Since the web process now runs as an unprivileged user inside the container, it cannot listen to port 80.
Simplifies passing of the current uid/gid combo to `docker compose build`
lunkwill42
force-pushed
the
feature/docker-python-improvements
branch
from
c4a0510
to
4f87336
Compare
Yeah, I don't think we should provide that kind of shoot-yourself-in-the-foot service to just anyone. If you know how to use it, you can do it yourself 😆 |
|
I just pushed updated docs and a slightly altered method for getting the UID/GID into the images - which hopefully reduces the hassle. Also, I realized that the nuke command you may be looking for is |
For the new Docker image to work properly, the PATH variable needs to propagate through user changes performed by `nav start` commands. There were, however, two barriers: First, su is used to drop privileges before starting nav daemons, but this resets the PATH variable to the ones defined in `/etc/login.defs`. Secondly, `nav start` uses a `-` to tell `su` to use a login shell, in which case the PATH is still reset. The login shell option was introduced by a fix for Uninett#2218, but it's not clear that the use or non-use of a login shell was the actual problem solved, it was just that the order of the arguments needed to be portable. Tagged Uninett#2218 just in case this turns into another BSD regression.
|
|
||
| Rebuilding the NAV code from scratch | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| A complete rebuild of the NAV code can be initiated by:: | ||
|
|
||
| docker-compose restart nav | ||
| docker compose restart nav |
There was a problem hiding this comment.
Probably necessary to remind ppl about upgrading docker somewhere..
Fresh install of NAV, nothing added to seeddb. If I visit If I visit |
Resolved in ffa18e2 (co-authored with @lunkwill42) |
After the Docker development container was re-engineered (in #2859) to run as a non-root user, this helper script stopped working, as it now needs sudo to run the nav start/stop command.
This PR aims to do several things to the docker-compose based development environment:
pipfrom installing system-level packages, so a virtualenv will be necessary anyway.navuser to a UID/GID pair at build time, so we don't need to have strange entrypoint magic to dynamically switch thenavusers uid/gid every time the container starts.Dockerfilein an attempt to speed up image rebuild times.Docs are updated. Essential difference for developers is that a UID/GID value now needs to be passed when the container images are built, but this can be automated and reduced to a call to
make docker.