Open jwbensley opened 7 months ago
Not sure if you want me to keep adding words to docs/spelling_wordlist.txt until it passes or not?
This is a good start! Some thoughts:
.git
should be excluded.:z
? They aren't shared right?I'm also wondering if original client IPs are still visible to IRRD with this network setup?
If all this creates complexity beyond your available time/interest/skill, I can probably also pick this up myself :)
I have tried to address you concerns:
- I don't think we should distribute RIPE db by default. The default image should probably not have any sources configured. There should definitely not be db dumps in the docker base image.
Yeah makes sense, my original PR was coming from a dev perspective (more on this later). I have removed the IRR data and put it in a script so that devs can import data only if they want to, for testing. I've also added it to the documentation.
- I'd prefer another workaround for the git status rather than hardcoding a path in the compose file. Why would we even care in the image? Don't see a reason to run git there. Or perhaps .git should be excluded.
The git thing was coming from running Sphinx tests inside docker. Because the .git/ sub-folder is mounted by docker-compose Sphinx checks if the owner of the folder is correctly set (it's not, because docker runs as root). It's not possible to not mount the .git/ sub-folder. I also tried to make the .git/ sub-folder empty but Sphinx then complains there is a .git/ folder which is not a real repo. To resolve this I updated the documentation, you still need to run the command, but only if you want to run the Sphinx tests, and at run time, not build time, so it's not baked into the container.
- Any reason to label the volumes with :z? They aren't shared right?
Just internal practice I copy-pastered. Fixed now.
- This makes me want a two layer irrd.yaml config file: in a docker instance, operators should never meddle with the listening host. And if they use the compose file, probably not database_url. But surely they would want to set the source settings. Or maybe something templated, so that the image can be used in different compose configs rather than the included one. And how to handle reloads well? Kind of a useful feature that we have now. I'm not sure about the best approach.
Hold that thought...
I'm also wondering if original client IPs are still visible to IRRD with this network setup?
Yes original client IPs are visible, but this comment, and the one above, and also you first comment I quoted above, all make me think we have different goals with "dockerising" IRRd.
My aim with this PR is to get IRRd and all it's Python dependencies, and application dependencies (Postgres and Redis), all running in containers for easy development work (and that also requires some real data, hence the import data script).
Right now the barrier to entry to doing dev work on IRRd is quite high because I need to basically deploy a new virtual machine and do a full OS install, then install Postgres and Redis, then I can build IRRd, then config IRR DB sources and wait for the data import.
What I have tried to achieve with this PR is to dockerise IRRd for dev work. You comments make me think you want to dockerise it for production operation. That's fine but IMO it's a different process. I see getting IRRd into docker for dev work as step 1, and getting it into docker for deployment as step 2.
Just my two pence; internally we create Dockerfile's and docker-compose files for all apps:
Update: I see there is another Docker PR open. That PR pulls the latest "release" of IRRd and dockerises that. This PR I have raised uses the local codebase. These are different approaches, my is aimed at dev work, that other PR is aimed at running in production. This is further to my point, I would split the two things up because they are not the same.
Update 2: I forgot to mention what's changed in the latest version of the PR, apart from addressing your concerns:
main
docutils
wasn't pinned and the latest release is b0rked, so I pinned itPytest is fully passing inside docker:
================================================================================== 768 passed, 1761 warnings in 85.12s (0:01:25) ==================================================================================
Sphinx test is fully passing inside docker (there are a bunch of words no in the dictionary which I didn't add, which it complains about though):
Warning, treated as error:
Found 63 misspelled words
As discussed, this PR adds an initial docker implementation and documentation.
I don't really know IRRd at all so you might need to rework this somehow but this is working as I had hoped.
Build the container:
Start the containers:
(Wait for initial data import to complete by checking status HTTP endpoint....)
Check data is available via WHOIS and GraphQL:
Run pytest inside docker (everything passes):