Skip to content
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

Add NGINX Agent as an official library image #15079

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dhurley
Copy link

@dhurley dhurley commented Jul 24, 2023

Hi,

This PR is to start the process of adding NGINX Agent docker images under the umbrella of Docker Library.

NGINX Agent is a companion daemon for your NGINX Open Source instance. It provides gRPC and REST interfaces for configuration management and metrics collection from the NGINX process and operating system. We use the official NGINX image as our base image and install the NGINX Agent alongside it.

To help fill out a review checklist:

Checklist for Review

NOTE: This checklist is intended for the use of the Official Images maintainers both to track the status of your PR and to help inform you and others of where we're at. As such, please leave the "checking" of items to the repository maintainers. If there is a point below for which you would like to provide additional information or note completion, please do so by commenting on the PR. Thanks! (and thanks for staying patient with us ❤️)

  • associated with or contacted upstream?
  • available under an OSI-approved license?
  • does it fit into one of the common categories? ("service", "language stack", "base distribution")
  • is it reasonably popular, or does it solve a particular use case well?
  • does a documentation PR exist? (should be reviewed and merged at roughly the same time so that we don't have an empty image page on the Hub for very long)
  • official-images maintainer dockerization review for best practices and cache gotchas/improvements (ala the official review guidelines)?
  • 2+ official-images maintainer dockerization review?
  • existing official images have been considered as a base? (ie, if foobar needs Node.js, has FROM node:... instead of grabbing node via other means been considered?)
  • if FROM scratch, tarballs only exist in a single commit within the associated history?
  • passes current tests? any simple new tests that might be appropriate to add? (https://github.com/docker-library/official-images/tree/master/test)

@github-actions
Copy link

Diff for 17f1186:
diff --git a/_bashbrew-arches b/_bashbrew-arches
index 8b13789..e85a97f 100644
--- a/_bashbrew-arches
+++ b/_bashbrew-arches
@@ -1 +1,2 @@
-
+amd64
+arm64v8
diff --git a/_bashbrew-cat b/_bashbrew-cat
index bdfae4a..201a67b 100644
--- a/_bashbrew-cat
+++ b/_bashbrew-cat
@@ -1 +1,14 @@
-Maintainers: New Image! :D (@docker-library-bot)
+Maintainers: NGINX Docker Maintainers <[email protected]> (@nginxinc)
+GitRepo: https://github.com/nginx/agent.git
+
+Tags: 2.27.0-nginx-mainline-alpine, nginx-mainline-alpine, latest
+Architectures: amd64, arm64v8
+GitCommit: 2f22eb8f373701cffa8a4dde78877e2df1d69840
+Directory: scripts/docker/official/nginx-oss-with-nginx-agent/alpine
+File: Dockerfile.mainline
+
+Tags: 2.27.0-nginx-stable-alpine, nginx-stable-alpine
+Architectures: amd64, arm64v8
+GitCommit: 2f22eb8f373701cffa8a4dde78877e2df1d69840
+Directory: scripts/docker/official/nginx-oss-with-nginx-agent/alpine
+File: Dockerfile.stable
diff --git a/_bashbrew-list b/_bashbrew-list
index e69de29..1d45922 100644
--- a/_bashbrew-list
+++ b/_bashbrew-list
@@ -0,0 +1,5 @@
+nginx-agent:2.27.0-nginx-mainline-alpine
+nginx-agent:2.27.0-nginx-stable-alpine
+nginx-agent:latest
+nginx-agent:nginx-mainline-alpine
+nginx-agent:nginx-stable-alpine
diff --git a/_bashbrew-list-build-order b/_bashbrew-list-build-order
index e69de29..f7465fa 100644
--- a/_bashbrew-list-build-order
+++ b/_bashbrew-list-build-order
@@ -0,0 +1,2 @@
+nginx-agent:latest
+nginx-agent:nginx-stable-alpine
diff --git a/nginx-agent_latest/Dockerfile.mainline b/nginx-agent_latest/Dockerfile.mainline
new file mode 100644
index 0000000..a39d3c2
--- /dev/null
+++ b/nginx-agent_latest/Dockerfile.mainline
@@ -0,0 +1,28 @@
+ARG NGINX_VERSION=1.25.1-alpine
+
+FROM nginx:${NGINX_VERSION}
+LABEL maintainer="NGINX Docker Maintainers <[email protected]>"
+
+ARG NGINX_AGENT_VERSION=2.27.0
+
+# Unlink symbolic links for access and error logs as the NGINX Agent 
+# requires both files to be able to monitor NGINX and provide metrics.
+RUN unlink /var/log/nginx/access.log && unlink /var/log/nginx/error.log
+
+RUN set -x \
+    && apk add curl bash iproute2
+
+RUN printf "%s%s\n" \
+    "http://packages.nginx.org/nginx-agent/alpine/v$(egrep -o '^[0-9]+\.[0-9]+' /etc/alpine-release)" \
+    "/main" | tee -a /etc/apk/repositories
+
+RUN curl -o /etc/apk/keys/nginx_signing.rsa.pub https://nginx.org/keys/nginx_signing.rsa.pub
+
+RUN apk add nginx-agent=${NGINX_AGENT_VERSION}
+
+COPY ./entrypoint.sh /agent/entrypoint.sh
+RUN chmod +x /agent/entrypoint.sh
+
+STOPSIGNAL SIGTERM
+
+ENTRYPOINT ["/agent/entrypoint.sh"]
diff --git a/nginx-agent_latest/entrypoint.sh b/nginx-agent_latest/entrypoint.sh
new file mode 100644
index 0000000..034745a
--- /dev/null
+++ b/nginx-agent_latest/entrypoint.sh
@@ -0,0 +1,46 @@
+#!/bin/bash
+
+set -euxo pipefail
+
+handle_term()
+{
+    echo "received TERM signal"
+    echo "stopping nginx-agent ..."
+    kill -TERM "${agent_pid}" 2>/dev/null
+    echo "stopping nginx ..."
+    kill -TERM "${nginx_pid}" 2>/dev/null
+}
+
+trap 'handle_term' TERM
+
+# Launch nginx
+echo "starting nginx ..."
+/usr/sbin/nginx -g "daemon off;" &
+
+nginx_pid=$!
+
+cat /etc/nginx-agent/nginx-agent.conf;
+
+# start nginx-agent, pass args
+echo "starting nginx-agent ..."
+nginx-agent "$@" &
+
+agent_pid=$!
+
+if [ $? != 0 ]; then
+    echo "couldn't start the agent, please check the log file"
+    exit 1
+fi
+
+wait_term()
+{
+    wait ${agent_pid}
+    trap - TERM
+    kill -QUIT "${nginx_pid}" 2>/dev/null
+    echo "waiting for nginx to stop..."
+    wait ${nginx_pid}
+}
+
+wait_term
+
+echo "nginx-agent process has stopped, exiting."
diff --git a/nginx-agent_nginx-stable-alpine/Dockerfile.stable b/nginx-agent_nginx-stable-alpine/Dockerfile.stable
new file mode 100644
index 0000000..67ba942
--- /dev/null
+++ b/nginx-agent_nginx-stable-alpine/Dockerfile.stable
@@ -0,0 +1,28 @@
+ARG NGINX_VERSION=1.24.0-alpine
+
+FROM nginx:${NGINX_VERSION}
+LABEL maintainer="NGINX Docker Maintainers <[email protected]>"
+
+ARG NGINX_AGENT_VERSION=2.27.0
+
+# Unlink symbolic links for access and error logs as the NGINX Agent 
+# requires both files to be able to monitor NGINX and provide metrics.
+RUN unlink /var/log/nginx/access.log && unlink /var/log/nginx/error.log
+
+RUN set -x \
+    && apk add curl bash iproute2
+
+RUN printf "%s%s\n" \
+    "http://packages.nginx.org/nginx-agent/alpine/v$(egrep -o '^[0-9]+\.[0-9]+' /etc/alpine-release)" \
+    "/main" | tee -a /etc/apk/repositories
+
+RUN curl -o /etc/apk/keys/nginx_signing.rsa.pub https://nginx.org/keys/nginx_signing.rsa.pub
+
+RUN apk add nginx-agent=${NGINX_AGENT_VERSION}
+
+COPY ./entrypoint.sh /agent/entrypoint.sh
+RUN chmod +x /agent/entrypoint.sh
+
+STOPSIGNAL SIGTERM
+
+ENTRYPOINT ["/agent/entrypoint.sh"]
diff --git a/nginx-agent_nginx-stable-alpine/entrypoint.sh b/nginx-agent_nginx-stable-alpine/entrypoint.sh
new file mode 100644
index 0000000..034745a
--- /dev/null
+++ b/nginx-agent_nginx-stable-alpine/entrypoint.sh
@@ -0,0 +1,46 @@
+#!/bin/bash
+
+set -euxo pipefail
+
+handle_term()
+{
+    echo "received TERM signal"
+    echo "stopping nginx-agent ..."
+    kill -TERM "${agent_pid}" 2>/dev/null
+    echo "stopping nginx ..."
+    kill -TERM "${nginx_pid}" 2>/dev/null
+}
+
+trap 'handle_term' TERM
+
+# Launch nginx
+echo "starting nginx ..."
+/usr/sbin/nginx -g "daemon off;" &
+
+nginx_pid=$!
+
+cat /etc/nginx-agent/nginx-agent.conf;
+
+# start nginx-agent, pass args
+echo "starting nginx-agent ..."
+nginx-agent "$@" &
+
+agent_pid=$!
+
+if [ $? != 0 ]; then
+    echo "couldn't start the agent, please check the log file"
+    exit 1
+fi
+
+wait_term()
+{
+    wait ${agent_pid}
+    trap - TERM
+    kill -QUIT "${nginx_pid}" 2>/dev/null
+    echo "waiting for nginx to stop..."
+    wait ${nginx_pid}
+}
+
+wait_term
+
+echo "nginx-agent process has stopped, exiting."

@tianon
Copy link
Member

tianon commented Jul 24, 2023

Thanks for the contribution! I haven't had a chance to dig into the details yet (or do a proper review of the Dockerization/software), but a bit of early feedback would be that I'm not sure this makes sense as a standalone image given that it's only really useful for NGINX itself. As far as I can tell in my limited look at it, it seems to be primarily a kind of log-based polyfill for some amount of NGINX Plus functionality? (In other words, it does not appear to be "standalone" or "generally useful" software by itself, and only does something in the context of NGINX, unlike NGINX or even NGINX Unit.)

Additionally, having an image that runs some bit of software with another as a sidecar (and some fragile shell code to manage both as if it were an init system) is not going to meet our requirements (see https://github.com/docker-library/official-images#init for more).

I don't know if you've discussed with the maintainer of the NGINX image (cc @thresheek), but this might make more sense as an alternate tag of that repository (since they're strongly tied together), and would have a better chance of being accepted if it were running alongside a separate NGINX container instead. 🙇

@dhurley
Copy link
Author

dhurley commented Jul 27, 2023

Thanks for the contribution! I haven't had a chance to dig into the details yet (or do a proper review of the Dockerization/software), but a bit of early feedback would be that I'm not sure this makes sense as a standalone image given that it's only really useful for NGINX itself. As far as I can tell in my limited look at it, it seems to be primarily a kind of log-based polyfill for some amount of NGINX Plus functionality? (In other words, it does not appear to be "standalone" or "generally useful" software by itself, and only does something in the context of NGINX, unlike NGINX or even NGINX Unit.)

Additionally, having an image that runs some bit of software with another as a sidecar (and some fragile shell code to manage both as if it were an init system) is not going to meet our requirements (see https://github.com/docker-library/official-images#init for more).

I don't know if you've discussed with the maintainer of the NGINX image (cc @thresheek), but this might make more sense as an alternate tag of that repository (since they're strongly tied together), and would have a better chance of being accepted if it were running alongside a separate NGINX container instead. 🙇

Hi @tianon, thanks for the feedback. I just have a question in relation to your suggestion of possibly adding an alternative tag to the already existing NGINX image. If we were to add an additional tag to the NGINX image that would provide an image that would have NGINX and NGINX Agent running alongside one another in the one container, would that kind of image be accepted?

We did explore the idea of having NGINX Agent running in a separate container to NGINX but we ran into difficulty with that based on how the NGINX Agent interacts with NGINX. Thats why we decided to go with just the one container that had both NGINX and NGINX Agent.

@tianon
Copy link
Member

tianon commented Jul 27, 2023

Unfortunately not, as there really isn't a great way to reliably manage both processes (especially when it comes to the failure modes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants