diff --git a/deploy/helm/container-cache/README.md b/deploy/helm/container-cache/README.md index c3ca5460e..e98d49024 100644 --- a/deploy/helm/container-cache/README.md +++ b/deploy/helm/container-cache/README.md @@ -295,9 +295,16 @@ Key behaviors: | Parameter | Default | Description | |-----------|---------|-------------| -| `service.type` | `NodePort` | `NodePort`, `ClusterIP`, or `LoadBalancer` | | `service.port` | `30345` | Primary service port | +The service is always `NodePort` and the type is not configurable. The host +container runtime reaches the cache at `${NODE_IP}:${port}` from the host +network namespace, where cluster service DNS and ClusterIP addresses are not +dependable, so the port has to be published on every node. Setting +`service.type` to anything other than `NodePort` fails the render: a ClusterIP +service publishes no node port, so the mirror endpoint would point at a closed +port and every pull would fall back to the upstream registry with no error. + ### CRI-O | Parameter | Default | Description | diff --git a/deploy/helm/container-cache/deploy/templates/_helpers.tpl b/deploy/helm/container-cache/deploy/templates/_helpers.tpl index 3f9db27a2..6301d0620 100644 --- a/deploy/helm/container-cache/deploy/templates/_helpers.tpl +++ b/deploy/helm/container-cache/deploy/templates/_helpers.tpl @@ -77,6 +77,23 @@ Create the name of the service account to use {{- end }} {{- end }} +{{/* +Reject a service.type override. + +The registry mirror endpoint written into hosts.toml and the CRI-O drop-in is +${NODE_IP}:${port}, which only resolves when the service publishes that port on +every node. Any other service type renders no nodePort, leaving the mirror +pointing at a closed port: pulls then fall back to the upstream registry with no +error, which is indistinguishable from a working cache until you look at cache +metrics. Fail the render instead of installing something that silently no-ops. +*/}} +{{- define "nvcf-container-cache.validateServiceType" -}} +{{- $type := (.Values.service | default dict).type -}} +{{- if and $type (ne $type "NodePort") -}} +{{- fail (printf "service.type=%s is not supported: container-cache must publish a NodePort so the host container runtime can reach it at ${NODE_IP}:${port}. Remove service.type from your values (the chart always renders NodePort) and set service.port instead." $type) -}} +{{- end -}} +{{- end }} + {{/* Compute CRI-O registry port mappings. If .Values.crio.registryPorts is set, use it. diff --git a/deploy/helm/container-cache/deploy/templates/daemonset.yaml b/deploy/helm/container-cache/deploy/templates/daemonset.yaml index e15a865b8..b8e722d07 100644 --- a/deploy/helm/container-cache/deploy/templates/daemonset.yaml +++ b/deploy/helm/container-cache/deploy/templates/daemonset.yaml @@ -59,6 +59,17 @@ spec: value: {{ $.Values.vault.certLocation | quote }} securityContext: privileged: true + # Not-ready means this node's registry config is written but not yet in + # effect (containerd needs a restart or a node cycle to pick up + # config_path). The pod keeps running either way -- this reports the + # node, it does not try to fix it. A DaemonSet whose ready count is + # below its desired count is the signal that some nodes are still + # pulling straight from the upstream registry. + readinessProbe: + exec: + command: ["/bin/bash", "-c", "test -f /tmp/nvcf-cc-ready"] + periodSeconds: 30 + failureThreshold: 1 {{- with $.Values.configure.resources }} resources: {{- toYaml . | nindent 10 }} @@ -86,6 +97,13 @@ spec: set -euo pipefail hosts=(${TARGET_HOST//,/ }) declare -A CRIO_PORTS + + # Cleared here and only written once the node's registry config is + # actually live; the readinessProbe reads it, so `kubectl get ds` + # counts exactly the nodes whose cache routing is in effect. + READY_MARKER=/tmp/nvcf-cc-ready + rm -f "${READY_MARKER}" + containerd_restart_pending=false {{- $crioPorts := include "nvcf-container-cache.crioRegistryPorts" . | fromYaml -}} {{- if $crioPorts }} {{- range $name, $val := $crioPorts }} @@ -128,7 +146,31 @@ spec: capabilities = ["pull", "resolve"] EOF done + + # containerd re-reads certs.d/*/hosts.toml on every pull, but + # registry.config_path lives in config.toml and is only read at + # startup -- containerd has no config reload, and SIGHUP kills the + # daemon rather than reloading it. So a config.toml edit is inert + # until containerd next starts. + # + # This DaemonSet deliberately does not restart containerd: that is + # disruptive on nodes running function workloads, and node lifecycle + # is managed out of band. Instead, hash around the updater to detect + # that we changed the active config, and report the node as pending + # so an operator can see which nodes still need a containerd restart + # or a node cycle. Until then this node keeps pulling from the + # upstream registry -- correctly, just without the cache. + before="$(sha256sum /host/etc/containerd/config.toml | cut -d' ' -f1)" python3 update_config.py /host/etc/containerd/config.toml + after="$(sha256sum /host/etc/containerd/config.toml | cut -d' ' -f1)" + + if [ "${before}" != "${after}" ]; then + containerd_restart_pending=true + echo "NOTICE: corrected containerd registry config on this node." >&2 + echo "NOTICE: containerd only reads registry.config_path at startup, so the" >&2 + echo "NOTICE: correction is inactive and image pulls still bypass the cache." >&2 + echo "NOTICE: Restart containerd or cycle this node to activate it." >&2 + fi fi # CRI-O: write a single drop-in. Do NOT touch /etc/containers/registries.conf; @@ -190,6 +232,32 @@ spec: mkdir -p "${user_drop_in_dir}" cp "${crio_conf}" "${user_drop_in_dir}/nvcf-container-cache.conf" fi + + # auto_reload_registries makes CRI-O watch registries.conf.d, but + # that setting only takes effect once CRI-O has read the drop-in we + # just wrote -- on a first install it has not. Unlike containerd, + # CRI-O reloads registry config on SIGHUP without dropping + # workloads, so nudge it rather than leaving the mirror inactive + # until something else restarts the daemon. Best-effort: a node + # where CRI-O is absent or unsignalable is not a failure. + crio_pid="$(pgrep -x crio | head -1 || true)" + if [ -n "${crio_pid}" ]; then + if kill -HUP "${crio_pid}" 2>/dev/null; then + echo "Reloaded CRI-O registry config (SIGHUP)." + else + echo "Could not signal CRI-O; the drop-in applies on its next reload." >&2 + fi + fi + fi + + # containerd is the only runtime that can be left with config on disk + # that is not yet in effect, because config_path is read at startup + # and it has no reload. CRI-O has no equivalent gap: the mirror lives + # directly in the drop-in, which it re-reads. + if [ "${containerd_restart_pending}" = "true" ]; then + echo "NOT READY: this node needs a containerd restart or a node cycle." >&2 + else + touch "${READY_MARKER}" fi # CRI-O picks up registries.conf.d changes via auto_reload_registries=true, diff --git a/deploy/helm/container-cache/deploy/templates/service.yaml b/deploy/helm/container-cache/deploy/templates/service.yaml index 605bea378..65adacef8 100644 --- a/deploy/helm/container-cache/deploy/templates/service.yaml +++ b/deploy/helm/container-cache/deploy/templates/service.yaml @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +{{- include "nvcf-container-cache.validateServiceType" . -}} apiVersion: v1 kind: Service metadata: @@ -21,21 +22,23 @@ metadata: app: {{ $.Release.Name }} {{- include "nvcf-container-cache.labels" . | nindent 4 }} spec: - type: {{ .Values.service.type | default "ClusterIP" }} + # Always NodePort, and not configurable. The host container runtime + # (containerd/CRI-O) reaches this service as ${NODE_IP}:${port} from the host + # network namespace, where cluster service DNS and ClusterIP addresses are not + # dependable. A ClusterIP service renders no nodePort at all, so the mirror + # endpoint the DaemonSet writes points at a port nothing listens on and every + # pull silently falls back to the upstream registry. + type: NodePort ports: - port: {{ .Values.service.port | default 30345 }} - {{ if eq .Values.service.type "NodePort" }} nodePort: {{ .Values.service.port | default 30345 }} - {{ end }} targetPort: https name: https {{- $crioPorts := include "nvcf-container-cache.crioRegistryPorts" . | fromYaml -}} {{- if $crioPorts }} {{- range $name, $val := $crioPorts }} - port: {{ $val.port }} - {{ if eq $.Values.service.type "NodePort" }} nodePort: {{ $val.port }} - {{ end }} targetPort: crio-{{ $name }} name: crio-{{ $name }} {{- end }} diff --git a/deploy/helm/container-cache/deploy/values.yaml b/deploy/helm/container-cache/deploy/values.yaml index b89312cb1..ecec2d8fe 100644 --- a/deploy/helm/container-cache/deploy/values.yaml +++ b/deploy/helm/container-cache/deploy/values.yaml @@ -142,8 +142,13 @@ persistentVolumeClaim: freeProxyPct: 15 # Service configuration. +# +# The service type is not configurable: it is always NodePort. The host +# container runtime reaches the cache at ${NODE_IP}:${port} from the host +# network namespace, so the port has to be published on every node. Setting +# service.type to anything else fails the render rather than installing a cache +# that pulls silently bypass. service: - type: NodePort port: 30345 # Proxy service (nvcf-proxy-cache ClusterIP) configuration. diff --git a/deploy/helm/container-cache/tests/chart-render/verify-mirrors.sh b/deploy/helm/container-cache/tests/chart-render/verify-mirrors.sh index 2ac555dfc..2884b0d33 100644 --- a/deploy/helm/container-cache/tests/chart-render/verify-mirrors.sh +++ b/deploy/helm/container-cache/tests/chart-render/verify-mirrors.sh @@ -38,6 +38,48 @@ assert_has 'name: nvcf-container-cache' # nodes without a local cache pod (kube-proxy drops NodePort traffic). assert_not_has 'externalTrafficPolicy: Local' assert_not_has 'internalTrafficPolicy: Local' +# The registry mirror endpoint is ${NODE_IP}:${port}, so the port must be +# published on every node. A ClusterIP service renders no nodePort and the +# mirror then points at a closed port, which fails as a silent fallback to the +# upstream registry rather than a visible error. +assert_has 'type: NodePort' + +echo "Checking service.type cannot be overridden..." +# Capture rather than pipe: `set -o pipefail` would otherwise report helm's +# intentional non-zero exit as the pipeline's failure. +override_out="$(helm template container-cache ./deploy --set service.type=ClusterIP 2>&1 || true)" +if ! printf '%s' "${override_out}" | grep -F -q -- 'is not supported'; then + echo "FAILED: service.type=ClusterIP should fail the render with an explanation" >&2 + echo "${override_out}" >&2 + exit 1 +fi +# NodePort is still accepted, so existing values files keep working. +helm template container-cache ./deploy --set service.type=NodePort >/dev/null + +echo "Checking containerd pending-restart reporting..." +# containerd reads registry.config_path only at daemon start and has no config +# reload, so correcting config.toml does not take effect on its own. Detect that +# we changed it and report the node; never restart containerd, which would be +# disruptive on nodes running function workloads. +assert_has 'before="$(sha256sum /host/etc/containerd/config.toml | cut -d'"'"' '"'"' -f1)"' +assert_has 'if [ "${before}" != "${after}" ]; then' +assert_has 'containerd_restart_pending=true' +assert_not_has 'systemctl restart containerd' +assert_not_has 'nsenter' + +echo "Checking readiness reflects whether cache routing is live..." +# A DaemonSet ready count below its desired count is the signal that some nodes +# still pull straight from the upstream registry. +assert_has 'test -f /tmp/nvcf-cc-ready' +assert_has 'readinessProbe:' +assert_has 'touch "${READY_MARKER}"' + +echo "Checking CRI-O reload..." +# SIGHUP is a documented CRI-O reload, not a kill, so it is safe to signal. +# Needed because auto_reload_registries only applies once CRI-O has read the +# crio.conf.d drop-in this DaemonSet writes. +assert_has 'kill -HUP "${crio_pid}"' +assert_has 'pgrep -x crio' echo "Checking multi-domain NodePort listeners..." assert_has 'nodePort: 30346' diff --git a/docs/user/cluster-management/container-cache.md b/docs/user/cluster-management/container-cache.md index 99d548b80..35cb739ed 100644 --- a/docs/user/cluster-management/container-cache.md +++ b/docs/user/cluster-management/container-cache.md @@ -290,19 +290,26 @@ persistentVolumeClaim: ### Service Configuration -The service type and port can be configured based on your access requirements: +The service port is configurable. The service type is not: Container Cache is +always exposed as a `NodePort`. ```yaml # values.yaml service: - # Service type: ClusterIP, NodePort, or LoadBalancer - type: ClusterIP - # Port for the Container Cache service port: 30345 ``` +The container runtime on each node reaches the cache at `${NODE_IP}:${port}` +from the host network namespace, where cluster service DNS and ClusterIP +addresses are not dependable, so the port must be published on every node. + +Setting `service.type` fails the install with an explicit error. A ClusterIP +service publishes no node port, so the registry mirror written to each node +would point at a port nothing listens on, and image pulls would fall back to +the upstream registry with no error and no cache involvement. + ### Metrics Configuration Container Cache includes Prometheus metrics for monitoring cache performance: