Configure NGINX: Makefile & Dockerfile - #102
Conversation
razvand
left a comment
There was a problem hiding this comment.
Do NOT update the nginx/ directory. Add a new directory named bincompat-nginx/ with all the contents in it.
Do not add the root filesystem in the commit. The root filesystem will be generated by the user. Ignore it in the commit.
Use your proper name in the commit messages Karina Antoniu. Sign-off your commits. See the contributor guide. Read them 3-4 times. Look at the other commits in the repository and write the current commits similarly.
razvand
left a comment
There was a problem hiding this comment.
Sorry for not giving you this feedback earlier. Your approach is slightly incorrect. You need to do something similar to the bincompat-rust-hello app.
Namely:
- There is no
Makefilefor the unikernel. - The instructions would point you to the kernel from
../elfloader-net/that you would build in that directory../elfloader-net/. - Add a
README.mdwith instructions. - A
setup.shscript is not really required. I think it's part ofbincompat-rust-hello/by mistake.
razvand
left a comment
There was a problem hiding this comment.
Squash your commits into a single one.
There are no instructions, no scripts and no README in the pull request. Follow the example of bincompat-c-hello: have a similar structure, check it works.
There was a problem hiding this comment.
Pull request overview
This PR adds automation to build an NGINX root filesystem for the bincompat-nginx unikernel flow by building NGINX in a container and extracting the resulting binary/config/assets (and intended shared-library deps) into a host-side extracted_rootfs directory.
Changes:
- Added a
rootfs/Dockerfilethat stages/usr/sbin/nginx,/etc/nginx, and default HTML into/rootfs, and attempts to copy dynamic dependencies. - Added a
rootfs/Makefilethat builds the Docker image and copies/rootfsout torootfs/extracted_rootfs. - Added
.gitignoreentries to keep generated artifacts out of git.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| bincompat-nginx/rootfs/Makefile | Builds an NGINX-rootfs Docker image and extracts /rootfs to extracted_rootfs. |
| bincompat-nginx/rootfs/Dockerfile | Constructs a minimal /rootfs tree containing nginx + configs/assets and attempts to copy shared libs. |
| bincompat-nginx/.gitignore | Ignores generated workdir and extracted rootfs output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bb6358e to
12f3dac
Compare
Signed-off-by: Karina Antoniu <karina.antoniu@gmail.com>
12f3dac to
456eee1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (6)
bincompat-nginx/rootfs/Dockerfile:12
|| truewill mask real failures when collecting/copying the dynamic linker and shared libraries, potentially producing an incomplete rootfs that fails at runtime. Usexargs -r(no-run-if-empty) instead and let genuine copy errors fail the build.
RUN ldd /usr/sbin/nginx | grep -o '/lib/[^ ]*' | xargs -I {} cp {} /rootfs/lib/
RUN ldd /usr/sbin/nginx | grep -o '/lib64/[^ ]*' | xargs -I {} cp {} /rootfs/lib64/ || true
bincompat-nginx/rootfs/Makefile:10
- Using a fixed container name (
temp-container) can fail if a previous run left the container behind, and rerunningallwithoutcleancan result in$(ROOTFS_DIR)containing a nestedrootfs/directory. Prefer using the container ID returned bydocker createand remove/recreate$(ROOTFS_DIR)each run.
all:
docker build -t $(DOCKER_IMAGE) .
docker create --name temp-container $(DOCKER_IMAGE)
docker cp temp-container:/rootfs $(ROOTFS_DIR)
docker rm temp-container
bincompat-nginx/README.md:70
- This QEMU command uses
-netdev bridgebut is not run withsudo. Other catalog examples that use bridged networking run QEMU with elevated privileges (e.g.,nginx/README.md), otherwise the bridge backend often fails to initialize.
qemu-system-x86_64 \
bincompat-nginx/rootfs/Dockerfile:7
- The extracted rootfs is missing
/run, but the default nginx.conf in the officialnginx:1.25image usespid /run/nginx.pid;. Also, nginx.conf defaults touser nginx;, so/etc/passwdand/etc/groupshould be present in the extracted rootfs to avoid startup failures when resolving that user.
This issue also appears on line 11 of the same file.
RUN mkdir -p /rootfs/usr/sbin /rootfs/etc/nginx /rootfs/usr/share/nginx/html /rootfs/var/log/nginx /rootfs/var/run /rootfs/tmp /rootfs/lib /rootfs/lib64
RUN cp /usr/sbin/nginx /rootfs/usr/sbin/
RUN cp -r /etc/nginx/* /rootfs/etc/nginx/
RUN cp -r /usr/share/nginx/html/* /rootfs/usr/share/nginx/html/
bincompat-nginx/README.md:35
- This link points to a Google search query rather than the local section anchor, so it won’t reliably take readers to the instructions in this README.
If you are experimenting and you encounter a bug which prevents the virtual machine from stopping, see the instructions in the ["Forcefully Terminating a Machine" section](https://www.google.com/search?q=%23forcefully-terminating-a-machine).
bincompat-nginx/README.md:22
- This QEMU command uses
-netdev bridgebut is not run withsudo. Other catalog examples that use bridged networking run QEMU with elevated privileges (e.g.,nginx/README.md), otherwise the bridge backend often fails to initialize.
This issue also appears on line 70 of the same file.
qemu-system-x86_64 \
Added rootfs/Dockerfile and rootfsMakefile to automate the process of obtaining the NGINX executable. The Dockefile installs NGINX in an isolated environment, while the Makefile handles copying the binary and its dependencies back to the host machine. Created the rootfs/extracted_rootfs directory that contains everything that will be mounted as the filesystem in the unikernel (the binary, the config files and shared libraries). I also modified setup.sh to automatically clone and create symlinks for all the required Unikraft libraries and the Makefile to include the path to the sources.