Remove HAProxy installation from the router image

HAProxy now runs on a sidecar container, using a distinct image from the
router one. This update is removing the HAProxy installation from the
router image. The reload script is also being removed, along with any
configuration pointing to it, like envvar in the router image and dev
documentation.

The --reload command-line option is still supported, so this does not
change the expectation of the nginx plugin. Providing a script however
is optional if the HAProxy admin unix socket is provided.

Hacking documentation for running the router locally is also updated.

Jira: https://redhat.atlassian.net/browse/NE-2816

Author: jcmoraisjr

HACKING.md

@@ -82,7 +82,13 @@ sudo cp haproxy /usr/sbin/
 
 ### Run OpenShift router locally
 
-Prepare the local environment and start router with default options:
+Prepare the local environment:
+
+```bash
+make local-prepare
+```
+
+On another terminal, start router with default options:
 
 ```bash
 make local-run
@@ -94,7 +100,7 @@ Prepare means create and clean `/var/lib/haproxy` and sub-directories, which rou
 
 Here is the VSCode launch configuration that starts OpenShift router in debug mode.
 
-> Run `make local-prepare` before the very first run, and whenever a clean environment is needed.
+> Run `make local-prepare` before starting router.
 
 `.vscode/launch.json` content, merge its content with any previous one:
 
@@ -119,7 +125,7 @@ Here is the VSCode launch configuration that starts OpenShift router in debug mo
             },
             "args": [
                 "--template=images/router/haproxy/conf/haproxy-config.template",
-                "--reload=images/router/haproxy/reload-haproxy",
+                "--haproxy-admin-unix-socket=/var/lib/haproxy/run/admin.sock",
             ],
         }
     ]
@@ -129,4 +135,3 @@ Here is the VSCode launch configuration that starts OpenShift router in debug mo
 Caveats:
 
 1. On debug mode, the binary name is not `openshift-router`, making the bootstrap code to fail. Temporarily patch `cmd/openshift-router/main.go` and hardcode `openshift-router` in the `CommandFor()` call.
-1. A haproxy instance should be left behind depending on how the router is stopped. Run `killall haproxy` in case the router complains when trying to listen to sockets.

hack/Dockerfile.debug

@@ -1,6 +1,4 @@
 FROM registry.access.redhat.com/ubi9/ubi
-RUN yum install -y https://github.com/frobware/haproxy-builds/raw/master/rhaos-4.16-rhel-9/haproxy28-2.8.5-1.rhaos4.16.el9.x86_64.rpm
-RUN haproxy -vv
 RUN INSTALL_PKGS="rsyslog procps-ng util-linux socat" && \
     yum install -y --setopt=install_weak_deps=0 $INSTALL_PKGS && \
     rpm -V $INSTALL_PKGS && \
@@ -17,6 +15,5 @@ USER 1001
 EXPOSE 80 443 7000
 WORKDIR /var/lib/haproxy/conf
 ENV XDG_CONFIG_HOME=/tmp \
-    TEMPLATE_FILE=/var/lib/haproxy/conf/haproxy-config.template \
-    RELOAD_SCRIPT=/var/lib/haproxy/reload-haproxy
+    TEMPLATE_FILE=/var/lib/haproxy/conf/haproxy-config.template
 ENTRYPOINT ["/usr/bin/openshift-router", "--v=4"]

hack/local-router.sh

@@ -18,7 +18,7 @@ help() {
 }
 
 router_prepare() {
-    # dropping any leaked haproxy instance that router did not have a chance to kill
+    # dropping any previously running haproxy instance
     killall haproxy 2>/dev/null || :
 
     # copying mandatory files to HAProxy's configuration dir
@@ -28,19 +28,24 @@ router_prepare() {
         sudo mkdir -p "$haproxydir"
         sudo chown "${USER}:${USER}" "$haproxydir"
     fi
-    rm -rf "${haproxydir}/{conf,run}"
-    mkdir -p "${haproxydir}/{conf,run}"
+    rm -rf "${haproxydir}/conf" "${haproxydir}/run"
+    mkdir -p "${haproxydir}/conf" "${haproxydir}/run" "${haproxydir}/router/certs" "${haproxydir}/router/cacerts" "${haproxydir}/router/allowlists"
     cp images/router/haproxy/conf/* "${haproxydir}/conf/"
+    touch "${haproxydir}/conf/haproxy.config"
 
-    echo "$haproxydir prepared"
+    echo
+    echo "$haproxydir prepared, starting HAProxy in foreground -- ^C to stop"
+
+    # starting the "sidecar" haproxy
+    haproxy -W -db -S "${haproxydir}/run/admin.sock,mode,600" -f "${haproxydir}/conf/haproxy.config"
 }
 
 router_run() {
     # changing listening ports, we usually don't have permission to bind to the default ones 80/443
-    ROUTER_SERVICE_HTTP_PORT=9090 ROUTER_SERVICE_HTTPS_PORT=9443 STATS_USERNAME=admin STATS_PASSWORD=admin STATS_PORT=1936 \
+    ROUTER_SERVICE_HTTP_PORT=9090 ROUTER_SERVICE_HTTPS_PORT=9443 STATS_USERNAME=admin STATS_PASSWORD=admin STATS_PORT=1936 ROUTER_GRACEFUL_SHUTDOWN_DELAY=2s \
         go run -v ./cmd/openshift-router \
             --template images/router/haproxy/conf/haproxy-config.template \
-            --reload images/router/haproxy/reload-haproxy
+            --haproxy-admin-unix-socket /var/lib/haproxy/run/admin.sock
 }
 
 [ $# -ne 1 ] && help
@@ -50,7 +55,7 @@ cd "$(dirname $0)/.."
 
 case "$1" in
     prepare) router_prepare;;
-    run) router_prepare; router_run;;
+    run) router_run;;
     help) help;;
     *) die "invalid command: $1";;
 esac

images/router/haproxy/Dockerfile

@@ -1,5 +1,5 @@
 FROM registry.svc.ci.openshift.org/openshift/origin-v4.0:base-router
-RUN INSTALL_PKGS="socat haproxy28 rsyslog sysvinit-tools" && \
+RUN INSTALL_PKGS="socat rsyslog sysvinit-tools" && \
     yum install -y --setopt=install_weak_deps=0 $INSTALL_PKGS && \
     rpm -V $INSTALL_PKGS && \
     yum clean all && \
@@ -16,6 +16,5 @@ LABEL io.k8s.display-name="OpenShift HAProxy Router" \
 USER 1001
 EXPOSE 80 443
 WORKDIR /var/lib/haproxy/conf
-ENV TEMPLATE_FILE=/var/lib/haproxy/conf/haproxy-config.template \
-    RELOAD_SCRIPT=/var/lib/haproxy/reload-haproxy
+ENV TEMPLATE_FILE=/var/lib/haproxy/conf/haproxy-config.template
 ENTRYPOINT ["/usr/bin/openshift-router", "--v=2"]

images/router/haproxy/Dockerfile.ocp

@@ -1,5 +1,5 @@
 FROM registry.ci.openshift.org/ocp/5.0:haproxy-router-base
-RUN INSTALL_PKGS="socat haproxy32 rsyslog procps-ng util-linux" && \
+RUN INSTALL_PKGS="socat rsyslog procps-ng util-linux" && \
     yum install -y --setopt=install_weak_deps=0 $INSTALL_PKGS && \
     rpm -V $INSTALL_PKGS && \
     yum clean all && \
@@ -17,6 +17,5 @@ LABEL io.k8s.display-name="OpenShift HAProxy Router" \
 USER 1001
 EXPOSE 80 443
 WORKDIR /var/lib/haproxy/conf
-ENV TEMPLATE_FILE=/var/lib/haproxy/conf/haproxy-config.template \
-    RELOAD_SCRIPT=/var/lib/haproxy/reload-haproxy
+ENV TEMPLATE_FILE=/var/lib/haproxy/conf/haproxy-config.template
 ENTRYPOINT ["/usr/bin/openshift-router", "--v=2"]

images/router/haproxy/Dockerfile.rhel

@@ -1,5 +1,5 @@
 FROM registry.svc.ci.openshift.org/ocp/4.0:base-router
-RUN INSTALL_PKGS="socat haproxy28 rsyslog sysvinit-tools" && \
+RUN INSTALL_PKGS="socat rsyslog sysvinit-tools" && \
     yum install -y --setopt=install_weak_deps=0 $INSTALL_PKGS && \
     rpm -V $INSTALL_PKGS && \
     yum clean all && \
@@ -16,6 +16,5 @@ LABEL io.k8s.display-name="OpenShift HAProxy Router" \
 USER 1001
 EXPOSE 80 443
 WORKDIR /var/lib/haproxy/conf
-ENV TEMPLATE_FILE=/var/lib/haproxy/conf/haproxy-config.template \
-    RELOAD_SCRIPT=/var/lib/haproxy/reload-haproxy
+ENV TEMPLATE_FILE=/var/lib/haproxy/conf/haproxy-config.template
 ENTRYPOINT ["/usr/bin/openshift-router", "--v=2"]

images/router/haproxy/reload-haproxy

@@ -111,6 +111,7 @@ type TemplateRouter struct {
 	WorkingDir                          string
 	TemplateFile                        string
 	ReloadScript                        string
+	HAProxyAdminUnixSocket              string
 	ReloadInterval                      time.Duration
 	DefaultCertificate                  string
 	DefaultCertificatePath              string
@@ -172,6 +173,7 @@ func (o *TemplateRouter) Bind(flag *pflag.FlagSet) {
 	flag.StringVar(&o.DefaultDestinationCAPath, "default-destination-ca-path", env("DEFAULT_DESTINATION_CA_PATH", ""), "A path to a PEM file containing the default CA bundle to use with re-encrypt routes. This CA should sign for certificates in the Kubernetes DNS space (service.namespace.svc).")
 	flag.StringVar(&o.TemplateFile, "template", env("TEMPLATE_FILE", ""), "The path to the template file to use")
 	flag.StringVar(&o.ReloadScript, "reload", env("RELOAD_SCRIPT", ""), "The path to the reload script to use")
+	flag.StringVar(&o.HAProxyAdminUnixSocket, "haproxy-admin-unix-socket", env("ROUTER_HAPROXY_ADMIN_UNIX_SOCKET", ""), "The path to the HAProxy admin unix socket")
 	flag.DurationVar(&o.ReloadInterval, "interval", getIntervalFromEnv("RELOAD_INTERVAL", defaultReloadInterval), "Controls how often router reloads are invoked. Mutiple router reload requests are coalesced for the duration of this interval since the last reload time.")
 	flag.BoolVar(&o.BindPortsAfterSync, "bind-ports-after-sync", env("ROUTER_BIND_PORTS_AFTER_SYNC", "") == "true", "Bind ports only after route state has been synchronized")
 	flag.StringVar(&o.MaxConnections, "max-connections", env("ROUTER_MAX_CONNECTIONS", ""), "Specifies the maximum number of concurrent connections.")
@@ -552,8 +554,8 @@ func (o *TemplateRouterOptions) Validate() error {
 			return fmt.Errorf("unable to load default destination CA certificate: %v", err)
 		}
 	}
-	if len(o.ReloadScript) == 0 {
-		return errors.New("reload script must be specified")
+	if len(o.ReloadScript) == 0 && len(o.HAProxyAdminUnixSocket) == 0 {
+		return errors.New("either the reload script or the HAProxy admin unix socket must be specified")
 	}
 	return nil
 }
@@ -611,12 +613,11 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {
 			}
 		}
 
-		adminUnixSocket := os.Getenv("ROUTER_HAPROXY_ADMIN_UNIX_SOCKET")
-		hasHAProxySidecar := len(adminUnixSocket) > 0
+		hasHAProxySidecar := len(o.HAProxyAdminUnixSocket) > 0
 
 		pidFn := haproxyPidEmbedded
 		if hasHAProxySidecar {
-			pidFn = haproxyPidSidecar(ctx, adminUnixSocket)
+			pidFn = haproxyPidSidecar(ctx, o.HAProxyAdminUnixSocket)
 		}
 		collector, err := haproxy.NewPrometheusCollector(haproxy.PrometheusOptions{
 			// Only template router customizers who alter the image should need this
@@ -777,6 +778,7 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {
 		WorkingDir:                    o.WorkingDir,
 		TemplatePath:                  o.TemplateFile,
 		ReloadScriptPath:              o.ReloadScript,
+		HAProxyAdminUnixSocketPath:    o.HAProxyAdminUnixSocket,
 		ReloadInterval:                o.ReloadInterval,
 		ReloadCallbacks:               reloadCallbacks,
 		DefaultCertificate:            o.DefaultCertificate,

pkg/cmd/infra/router/template.go

@@ -47,6 +47,7 @@ type TemplatePluginConfig struct {
 	WorkingDir                    string
 	TemplatePath                  string
 	ReloadScriptPath              string
+	HAProxyAdminUnixSocketPath    string
 	ReloadFn                      func(shutdown bool) error
 	ReloadInterval                time.Duration
 	ReloadCallbacks               []func()
@@ -152,6 +153,7 @@ func NewTemplatePlugin(cfg TemplatePluginConfig, lookupSvc ServiceLookup) (*Temp
 		dir:                           cfg.WorkingDir,
 		templates:                     templates,
 		reloadScriptPath:              cfg.ReloadScriptPath,
+		haproxyAdminUnixSocketPath:    cfg.HAProxyAdminUnixSocketPath,
 		reloadFn:                      cfg.ReloadFn,
 		reloadInterval:                cfg.ReloadInterval,
 		reloadCallbacks:               cfg.ReloadCallbacks,

pkg/router/template/plugin.go

@@ -68,6 +68,11 @@ type templateRouter struct {
 	state            map[ServiceAliasConfigKey]ServiceAliasConfig
 	serviceUnits     map[ServiceUnitKey]ServiceUnit
 	certManager      certificateManager
+	// haproxyAdminUnixSocketPath is the path to the unix socket of the HAProxy master process.
+	// The HAProxy master process has the ability to reach any running worker, as well as
+	// launch a new one to apply changes in the configuration file. If provided, it has priority
+	// over the reload script when the router needs to reload the proxy.
+	haproxyAdminUnixSocketPath string
 	// defaultCertificate is a concatenated certificate(s), their keys, and their CAs that should be used by the underlying
 	// implementation as the default certificate if no certificate is resolved by the normal matching mechanisms.  This is
 	// usually a wildcard certificate for a cloud domain such as *.mypaas.com to allow applications to create app.mypaas.com
@@ -140,6 +145,7 @@ type templateRouterCfg struct {
 	dir                           string
 	templates                     map[string]*template.Template
 	reloadScriptPath              string
+	haproxyAdminUnixSocketPath    string
 	reloadFn                      func(shutdown bool) error
 	reloadInterval                time.Duration
 	reloadCallbacks               []func()
@@ -253,6 +259,7 @@ func newTemplateRouter(cfg templateRouterCfg) (*templateRouter, error) {
 		dir:                           dir,
 		templates:                     cfg.templates,
 		reloadScriptPath:              cfg.reloadScriptPath,
+		haproxyAdminUnixSocketPath:    cfg.haproxyAdminUnixSocketPath,
 		reloadInterval:                cfg.reloadInterval,
 		reloadCallbacks:               cfg.reloadCallbacks,
 		reloadFn:                      cfg.reloadFn,
@@ -675,14 +682,13 @@ func (r *templateRouter) writeCertificates(cfg *ServiceAliasConfig) error {
 
 // reloadRouter reloads haproxy.
 func (r *templateRouter) reloadRouter(shutdown bool) error {
-	adminSocket := os.Getenv("ROUTER_HAPROXY_ADMIN_UNIX_SOCKET")
-	if adminSocket != "" {
+	if r.haproxyAdminUnixSocketPath != "" {
 		if shutdown {
 			// We are in HAProxy's master/worker mode, currently implemented as a sidecar,
-			// so there is no local process to handle and the sidecar one already received SIGTERM.
+			// so there is no local process to handle and the sidecar one will receive its own SIGTERM from kubelet.
 			return nil
 		}
-		return reloadRouterSidecar(r.appCtx, "unix://"+adminSocket)
+		return reloadRouterSidecar(r.appCtx, "unix://"+r.haproxyAdminUnixSocketPath)
 	}
 	return r.reloadRouterEmbedded(shutdown)
 }