Skip to content

replace nginz disco with native upstream resolver WPB-15302 #4663

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

Merged
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
659a494
WPB-15302
jschaul Jul 15, 2025
a62a0a6
remove nginz_disco code
jschaul Jul 15, 2025
b4b8851
replace outside upstreams.txt file by an inline block
jschaul Jul 15, 2025
0118508
clusterDomain
jschaul Jul 15, 2025
9e91ce4
remove sidecar container from deployment
jschaul Jul 15, 2025
68ce42e
readme
jschaul Jul 15, 2025
4003f27
changelog, fixup
jschaul Jul 15, 2025
f758520
Drive-by: be more graceful when terminating nginz
jschaul Jul 15, 2025
46a35a2
don't try to upload images
jschaul Jul 15, 2025
e38d9c6
linting
jschaul Jul 15, 2025
7592fd1
fixup
jschaul Jul 15, 2025
4191265
fixup2
jschaul Jul 15, 2025
31d60fa
actually upgrade nginx to 1.28 (was hardcoded to 1.26 before)
jschaul Jul 15, 2025
a16b794
remove upstreams file from 'startNginzK8s'. Unsure if this is keeping…
jschaul Jul 16, 2025
aeac7ac
delete more stuff about reloading
jschaul Jul 17, 2025
921b10f
try to inline upstreams in integration-spawned processes
jschaul Jul 21, 2025
ada4987
format
jschaul Jul 21, 2025
617fd72
regex...
jschaul Jul 21, 2025
b6737cd
format
jschaul Jul 21, 2025
7366073
regex fixup
jschaul Jul 22, 2025
03a3873
fixup
jschaul Jul 22, 2025
1aa6c4e
regexes don't seem to work. Use another way.
jschaul Jul 22, 2025
d6fb13a
remove nginx override as versions are the same as in default nix package
jschaul Jul 22, 2025
4352f49
Refactor the nginx upstream creation for Joe's PR (#4679)
supersven Jul 24, 2025
8a2eee8
Replace upstream parser with regex
supersven Jul 25, 2025
1f01413
Make regex non-greedy
supersven Jul 28, 2025
3b221e6
Update integration/test/Testlib/ModService.hs
smatting Jul 29, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5-internal/nginz-disco-replacement
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
charts/nginz: remove nginz_disco script and sidecar container, and replace outside upstreams.txt file by an inline block, making use of 'resolve' keyword to directly reference DNS names inside the kubernetes cluster.
6 changes: 1 addition & 5 deletions charts/nginz/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# nginz chart

Deploys [nginz - nginx with authentication module](https://github.com/wireapp/wire-server/services/nginz), along with a little sidecar container
Deploys [nginz - nginx with authentication module](https://github.com/wireapp/wire-server/services/nginz).

## Configuring zauth

Expand All @@ -18,7 +18,3 @@ This only needs to be done when you wish to bypass normal authentication for som
* `htpasswd -cb myfile.txt myuser mypassword && cat myfile.txt` (`htpasswd` is from `httpd-tools` or `apache-utils`) generates a hashed user:password line which you can pass to the nginz chart under `nginz.secrets.basicAuth` (see also wire-server-deploy/values/wire-server/secrets.yaml )
* generate the base64 value of the original user:password (*not* of the myfile contents): `echo '<user>:<password>' | base64`
* deploy and try a request by passing a header `Authorization: Basic <base64-encoded-value>`

## Sidecar container nginz-disco

Due to nginx not supporting DNS names for its list of upstream servers (unless you pay extra), the [nginz-disco](https://github.com/wireapp/wire-server/tree/develop/tools/nginz_disco) container is a simple bash script to do DNS lookups and write the resulting IPs to a file. Nginz reloads on changes to this file.
16 changes: 15 additions & 1 deletion charts/nginz/templates/conf/_nginx.conf.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,22 @@ http {
#
# Proxied Upstream Services
#
{{- $clusterDomain := .Values.nginx_conf.cluster_domain }}

include {{ .Values.nginx_conf.upstream_config }};
resolver kube-dns.kube-system.svc.{{ $clusterDomain }} valid=30s ipv6=off;
resolver_timeout 5s;

{{- $validUpstreams := include "valid_upstreams" . | fromJson }}
{{- range $name, $_ := $validUpstreams }}
upstream {{ $name }} {
zone {{ $name }} 64k; # needed for dynamic DNS updates
least_conn;
keepalive 32;
Comment on lines +200 to +202
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where do these settings come from?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are kept from before, see https://github.com/wireapp/wire-server/blob/develop/tools/nginz_disco/nginz_disco.sh#L49-L55 which results in current upstream files looking like

upstream cargohold { 
	 least_conn; 
	 keepalive 32; 
	 server 10.100.213.88:8080 max_fails=3 weight=100; 
}
upstream galeb.integrations { 
	 least_conn; 
	 keepalive 32; 
	 server 10.100.18.166:8080 max_fails=3 weight=100; 
}

The only difference here is the addition of a zone, which according to the docs in https://nginx.org/en/docs/http/ngx_http_upstream_module.html (see 'resolve' keyword) is needed, as

The server group must reside in the shared memory.


{{- $ns := index $.Values.nginx_conf.upstream_namespace $name | default $.Release.Namespace }}
server {{ $name }}.{{ $ns }}.svc.{{ $clusterDomain }}:8080 max_fails=3 resolve;
}
{{ end }}

#
# Mapping for websocket connections
Expand Down
4 changes: 0 additions & 4 deletions charts/nginz/templates/conf/_upstreams.txt.tpl

This file was deleted.

1 change: 0 additions & 1 deletion charts/nginz/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ metadata:
data:
nginx.conf: |2
{{- include "nginz_nginx.conf" . | indent 4 }}
upstreams.txt: "{{- include "nginz_upstreams.txt" . | trim }}"

{{ (.Files.Glob "static/conf/*").AsConfig | indent 2 }}
---
Expand Down
24 changes: 6 additions & 18 deletions charts/nginz/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,6 @@ spec:
matchLabels:
app: nginz
containers:
- name: nginz-disco
image: "{{ .Values.images.nginzDisco.repository }}:{{ .Values.images.nginzDisco.tag }}"
{{- if eq (include "includeSecurityContext" .) "true" }}
securityContext:
{{- toYaml .Values.podSecurityContext | nindent 10 }}
{{- end }}
volumeMounts:
- name: config
mountPath: /etc/wire/nginz/conf
readOnly: true
- name: upstreams
mountPath: /etc/wire/nginz/upstreams
readOnly: false
- name: nginz
image: "{{ .Values.images.nginz.repository }}:{{ .Values.images.nginz.tag }}"
{{- if eq (include "includeSecurityContext" .) "true" }}
Expand All @@ -63,9 +50,6 @@ spec:
- name: config
mountPath: /etc/wire/nginz/conf
readOnly: true
- name: upstreams
mountPath: /etc/wire/nginz/upstreams
readOnly: true
- name: deeplink
mountPath: /etc/wire/nginz/deeplink
readOnly: true
Expand All @@ -88,6 +72,12 @@ spec:
path: /status
port: {{ .Values.config.http.httpPort }}
scheme: HTTP
lifecycle:
preStop:
exec:
# kubernetes by default sends a SIGTERM to the container,
# we can be more graceful with in-flight requests using quit
command: ["sh", "-c", "nginx -c /etc/wire/nginz/conf/nginx.conf -s quit && sleep 25"]
resources:
{{ toYaml .Values.resources | indent 12 }}
dnsPolicy: ClusterFirst
Expand All @@ -99,8 +89,6 @@ spec:
- name: secrets
secret:
secretName: nginz
- name: upstreams
emptyDir: {}
- name: deeplink
configMap:
name: nginz-deeplink
10 changes: 3 additions & 7 deletions charts/nginz/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ metrics:
serviceMonitor:
enabled: false
images:
nginzDisco:
repository: quay.io/wire/nginz_disco
tag: do-not-use
nginz:
repository: quay.io/wire/nginz
tag: do-not-use
Expand All @@ -22,9 +19,9 @@ config:
ws:
wsPort: 8081
useProxyProtocol: true
terminationGracePeriodSeconds: 30
terminationGracePeriodSeconds: 90
nginx_conf:
upstream_config: /etc/wire/nginz/upstreams/upstreams.conf
cluster_domain: cluster.local
zauth_keystore: /etc/wire/nginz/secrets/zauth.conf
zauth_acl: /etc/wire/nginz/conf/zauth.acl
basic_auth_file: /etc/wire/nginz/secrets/basic_auth.txt
Expand Down Expand Up @@ -124,8 +121,7 @@ nginx_conf:
ignored_upstreams: []

# If an upstream runs in a different namespace than nginz, its namespace must
# be specified here otherwise nginz_disco will fail to find the upstream and
# nginx will think that the upstream is down.
# be specified here otherwise nginz will think that the upstream is down.
upstream_namespace: {
# galeb: integrations
}
Expand Down
6 changes: 2 additions & 4 deletions hack/bin/kind-upload-images.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,5 @@ nix -v --show-trace -L build -f "$ROOT_DIR/nix" wireServer.imagesList -o "$image

xargs -I {} -P 10 "$SCRIPT_DIR/kind-upload-image.sh" "wireServer.$IMAGES_ATTR.{}" < "$image_list_file"

for image_name in nginz nginz-disco; do
printf '*** Unploading image %s\n' "$image_name"
"$SCRIPT_DIR/kind-upload-image.sh" "$image_name"
done
printf '*** Unploading image %s\n' nginz
"$SCRIPT_DIR/kind-upload-image.sh" nginz
6 changes: 2 additions & 4 deletions hack/bin/upload-images.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,5 @@ nix -v --show-trace -L build -f "$ROOT_DIR/nix" "wireServer.$IMAGES_ATTR" --no-l

xargs -I {} -P 10 "$SCRIPT_DIR/upload-image.sh" "wireServer.$IMAGES_ATTR.{}" < "$image_list_file"

for image_name in nginz nginz-disco; do
printf '*** Uploading image %s\n' "$image_name"
"$SCRIPT_DIR/upload-image.sh" "$image_name"
done
printf '*** Uploading image %s\n' nginz
"$SCRIPT_DIR/upload-image.sh" nginz
2 changes: 2 additions & 0 deletions integration/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
, HsOpenSSL
, http-client
, http-types
, interpolate
, kan-extensions
, lens
, lens-aeson
Expand Down Expand Up @@ -149,6 +150,7 @@ mkDerivation {
HsOpenSSL
http-client
http-types
interpolate
kan-extensions
lens
lens-aeson
Expand Down
1 change: 1 addition & 0 deletions integration/integration.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ library
, HsOpenSSL
, http-client
, http-types
, interpolate
, kan-extensions
, lens
, lens-aeson
Expand Down
Loading