Skip to content

preStop hook can't reach the localhost-only admin server since #745; add first-class exec-based shutdown #797

Description

@0sujaljain0

Since #745, the injected preStop hook cannot reach the proxy's admin server, and there is no first-class way to use the exec-based shutdown command that the proxy gained in v2.20.0. This issue covers both, since the second is the fix for the first.

The preStop hook no longer reaches the admin server

addHealthCheck() unconditionally injects an HTTP preStop hook (podspec_updates.go:896-904):

c.Lifecycle = &corev1.Lifecycle{
    PreStop: &corev1.LifecycleHandler{
        HTTPGet: &corev1.HTTPGetAction{
            Port: intstr.IntOrString{IntVal: adminPort},
            Path: "/quitquitquit",
        },
    },
}

#745 removed Host: "localhost" from this handler to fix #739 (k8s 1.34 blocks host on lifecycle handlers under baseline/restricted PSA). That fix was necessary, but it leaves the hook pointing somewhere the admin server isn't listening:

  • An httpGet handler with no Host defaults to the pod IP, so kubelet dials podIP:9091.
  • The proxy's admin server binds localhost only — net.JoinHostPort("localhost", cmd.conf.AdminPort) (cmd/root.go:1238). There is no flag to change the bind address; --admin-port sets only the port, and --http-address applies to the health-check server (which the operator already sets to 0.0.0.0 at podspec_updates.go:873).

So the hook should be getting connection-refused and producing a FailedPreStopHook event on every pod termination. This affects v1.7.5 (the first release containing #745) through v1.8.2.

The impact is muted — CSQL_PROXY_EXIT_ZERO_ON_SIGTERM=true means the container still exits 0, so the "exited in an error state" symptom from #425 stays fixed — but the graceful drain that #425 built the hook for no longer happens, and there's a warning event on every stop.

Caveat: this is a code-level analysis of both repos, not something I've reproduced on a live cluster. Happy to be corrected if the hook is in fact reaching the admin server through some path I've missed. The existing tests assert only the generated PodSpec, so they wouldn't catch this either way.

Why exec is the fix

Both constraints are now fixed points: PSA forbids host on the handler, and the admin server won't bind anything but localhost. That leaves no working HTTP form. An exec handler runs inside the container's own network namespace, so 127.0.0.1 resolves correctly and no host field is involved.

The proxy added exactly this in v2.20.0 (GoogleCloudPlatform/cloud-sql-proxy#2514):

lifecycle:
  preStop:
    exec:
      command: ["/cloud-sql-proxy", "shutdown", "--admin-port", "<ADMIN_PORT>"]

The command is safe for the operator to generate: the published image is distroless with ENTRYPOINT ["/cloud-sql-proxy"], and the operator already resolves the admin port itself (adminPort at podspec_updates.go:891, from adminServer.port or DefaultAdminPort).

Today the only way to get this is authProxyContainer.container, which fully replaces the container (podspec_updates.go:591-597 returns before any reconcile logic runs). That means hand-maintaining image, env, args, probes and security context forever, and forfeiting automatic proxy-image upgrades — the same complaint as #507.

Proposal

Add an opt-in shutdown config under authProxyContainer. Two shapes seem reasonable and they compose, so I'd like input on which to build rather than guessing.

A. First-class Exec mode. The operator generates the command from the admin port it already resolves:

spec:
  authProxyContainer:
    shutdown:
      mode: Exec          # HTTP | Exec, default HTTP

Nothing to keep in sync — change adminServer.port and the hook follows. Fixes the broken hook for anyone who opts in, with a one-line spec change.

B. User-supplied command. A general escape hatch for the preStop command:

spec:
  authProxyContainer:
    shutdown:
      exec:
        command: ["/cloud-sql-proxy", "shutdown", "--admin-port", "9091"]

More flexible and useful for custom images or wrapper entrypoints, but the admin port has to be repeated by hand and can silently drift from adminServer.port.

These aren't exclusive — mode: Exec with an optional command override covers both, defaulting to the generated command when omitted.

Open questions worth settling in this thread:

  • Should Exec eventually become the default? The bundled default image is 2.25.2, well past 2.20.0, and the HTTP form appears broken for everyone — so defaulting to HTTP means shipping a known-broken hook to anyone who doesn't opt in. Against that: it's a behavior change on upgrade, and users pinning a proxy older than 2.20.0 would get a hook whose subcommand doesn't exist. Opt-in first and flip the default in a later minor seems like the conservative path.
  • Version detection. There's no version-awareness anywhere in the operator today, and authProxyContainer.image is a free-form string, so auto-selecting exec by parsing the tag would be new and fragile machinery (latest and digest-pinned images can't be resolved at all). An explicit field avoids that entirely — the user asserts their image supports it.
  • Where the field belongs. authProxyContainer.shutdown as sketched, or folded into adminServer alongside port/enableAPIs, since it's an admin-server-driven behavior?

Happy to send a PR for whichever shape you prefer, tests included. If the analysis of the broken hook holds up, that part may be worth splitting into its own fix — let me know how you'd like it structured.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions