From 0c8e29b05a023ac328198bec56cb9c93cb4845b0 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Tue, 22 Sep 2026 22:22:37 +0800 Subject: [PATCH] fix(platform): give every control-plane Deployment real probes (#8 follow-up) The api, operator and registry Deployments shipped with no liveness/readiness probes at all: a wedged process stayed 'Running' forever, and the operator had no health listener to probe in the first place. Kaniko build evidence on a fresh install showed the only cluster-wide red after a disk-pressure pass was Deployment status that never reflected health. - felis-api: readiness /readyz (DB + K8s API round-trip) and liveness /healthz on the internal face (:8081), the only listener that serves both endpoints; liveness deliberately avoids /readyz so a DB blip cannot restart the api. - felis-operator: new --health-probe-bind-address (:8081) with controller- runtime's /healthz + /readyz (registered ping checks; an unregistered handler map would 404), plus the matching container port and probes. - registry: /v2/ probes on the pinned port, so a broken storage backend stops reading as 'Running'. Tests pin paths, ports, and that each probe targets a declared container port. --- cmd/felis/operator.go | 25 ++++++++- internal/platform/workloads.go | 78 ++++++++++++++++++++++++++++- internal/platform/workloads_test.go | 63 +++++++++++++++++++++++ 3 files changed, 162 insertions(+), 4 deletions(-) diff --git a/cmd/felis/operator.go b/cmd/felis/operator.go index 779213e..7bff9f3 100644 --- a/cmd/felis/operator.go +++ b/cmd/felis/operator.go @@ -16,6 +16,7 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/healthz" ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" ) @@ -27,6 +28,11 @@ func cmdOperator(args []string, _, stderr io.Writer) int { fs := flag.NewFlagSet("operator", flag.ContinueOnError) fs.SetOutput(stderr) metricsAddr := fs.String("metrics-bind-address", ":8080", "address the metric endpoint binds to") + // healthAddr serves the manager's health endpoints (/healthz, /readyz) that the + // Deployment's probes dial. Without it the operator pod would carry no probe at + // all, and a wedged manager would keep its endpoint forever. It must differ from + // metricsAddr: the metrics server owns :8080. + healthAddr := fs.String("health-probe-bind-address", ":8081", "address the health probe endpoint binds to") // namespace MUST equal the [k8s] namespace felis-api is configured with, and // the deployment manifests (felis manifests) render both from one value. It // scopes the manager's cache (informers) to a single namespace so the operator @@ -51,8 +57,9 @@ func cmdOperator(args []string, _, stderr io.Writer) int { ctrl.SetLogger(logr.FromSlogHandler(slog.Default().Handler())) mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ - Scheme: scheme, - Metrics: metricsserver.Options{BindAddress: *metricsAddr}, + Scheme: scheme, + Metrics: metricsserver.Options{BindAddress: *metricsAddr}, + HealthProbeBindAddress: *healthAddr, // Scope every informer to the single watched namespace. Without this the // cached client (mgr.GetClient) would LIST/WATCH cluster-wide, which a // namespaced Role cannot grant — the operator would fail closed at runtime @@ -68,6 +75,20 @@ func cmdOperator(args []string, _, stderr io.Writer) int { } fmt.Fprintf(stderr, "felis operator: watching namespace %q\n", *namespace) + // Register the two probe endpoints. controller-runtime only mounts /healthz and + // /readyz once at least one check is registered, so a bare listener would 404. + // The checks are the canonical always-pass ping: the probes' contract is "the + // manager process is up and serving", and a dependency hiccup (e.g. an API blip) + // must not restart the operator. + if err := mgr.AddHealthzCheck("ping", healthz.Ping); err != nil { + fmt.Fprintf(stderr, "felis operator: register healthz check: %v\n", err) + return 1 + } + if err := mgr.AddReadyzCheck("ping", healthz.Ping); err != nil { + fmt.Fprintf(stderr, "felis operator: register readyz check: %v\n", err) + return 1 + } + // Publish the named felis_* metrics (spec §23) on the endpoint the manager // already serves (metricsAddr). controller-runtime's metrics server exposes // its global Registry, so registering into it is all that is needed for diff --git a/internal/platform/workloads.go b/internal/platform/workloads.go index 4d15799..06fd03b 100644 --- a/internal/platform/workloads.go +++ b/internal/platform/workloads.go @@ -66,8 +66,13 @@ const ( apiInternalPort int32 = 8081 apiHTTPSPort int32 = 8443 operatorMetricsPort int32 = 8080 - apiTLSSecretName = "felis-api-tls" - apiTLSMountPath = "/etc/felis/tls" + // operatorHealthPort must match cmd/felis/operator.go's + // --health-probe-bind-address default (and the arg rendered below): it is the + // only listener the operator Deployment's probes can dial — :8080 is the + // metrics server, which serves no health endpoints. + operatorHealthPort int32 = 8081 + apiTLSSecretName = "felis-api-tls" + apiTLSMountPath = "/etc/felis/tls" registryName = "registry" registryDataPath = "/var/lib/registry" @@ -319,6 +324,30 @@ func APIDeployment(p Params) *appsv1.Deployment { // LocalContextStore can persist a submitted context. {Name: uploadsVolume, MountPath: UploadsLocalPath}, }, + // Probes dial the INTERNAL face (8081), the only listener carrying both + // /healthz and /readyz (the external face deliberately serves liveness + // only), and the face kubelet can reach without any Zero Trust hop. + // Readiness = /readyz (DB + K8s API round-trip): a not-ready answer only + // pulls the pod out of Service endpoints. Liveness = the cheap /healthz — + // pointing it at /readyz would restart the api on every DB blip. + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{HTTPGet: &corev1.HTTPGetAction{ + Path: "/readyz", Port: intstr.FromInt32(apiInternalPort), + }}, + InitialDelaySeconds: 5, + PeriodSeconds: 10, + TimeoutSeconds: 3, + FailureThreshold: 3, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", Port: intstr.FromInt32(apiInternalPort), + }}, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + TimeoutSeconds: 3, + FailureThreshold: 3, + }, Resources: controlPlaneResources(), SecurityContext: hardenedContainerSecurityContext(), } @@ -415,6 +444,7 @@ func OperatorDeployment(p Params) *appsv1.Deployment { Args: []string{ "--namespace", p.MinecraftNamespace, "--metrics-bind-address", fmt.Sprintf(":%d", operatorMetricsPort), + "--health-probe-bind-address", fmt.Sprintf(":%d", operatorHealthPort), }, // FELIS_IMAGE names this same image so the operator can run it as the // forwarding-config initContainer it injects into user servers (it must @@ -424,10 +454,32 @@ func OperatorDeployment(p Params) *appsv1.Deployment { }, Ports: []corev1.ContainerPort{ {Name: "metrics", ContainerPort: operatorMetricsPort, Protocol: corev1.ProtocolTCP}, + {Name: "health", ContainerPort: operatorHealthPort, Protocol: corev1.ProtocolTCP}, }, VolumeMounts: []corev1.VolumeMount{ {Name: tmpVolume, MountPath: "/tmp"}, }, + // controller-runtime serves /healthz and /readyz on the health listener + // (both registered as always-pass pings in cmd/felis/operator.go): the + // probe's contract is "the manager process is up", not a dependency check. + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{HTTPGet: &corev1.HTTPGetAction{ + Path: "/readyz", Port: intstr.FromInt32(operatorHealthPort), + }}, + InitialDelaySeconds: 5, + PeriodSeconds: 10, + TimeoutSeconds: 3, + FailureThreshold: 3, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", Port: intstr.FromInt32(operatorHealthPort), + }}, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + TimeoutSeconds: 3, + FailureThreshold: 3, + }, Resources: controlPlaneResources(), SecurityContext: hardenedContainerSecurityContext(), } @@ -606,6 +658,28 @@ func registryDeployment(p Params) *appsv1.Deployment { {Name: registryVolume, MountPath: registryDataPath}, {Name: tmpVolume, MountPath: "/tmp"}, }, + // Distribution serves GET /v2/ (200 = app + storage healthy) for any + // client, so both probes reuse it: without them a registry whose storage + // backend broke would stay "Running" and every build push would fail with + // nothing red in the Deployment status. + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{HTTPGet: &corev1.HTTPGetAction{ + Path: "/v2/", Port: intstr.FromString(registryName), + }}, + InitialDelaySeconds: 5, + PeriodSeconds: 10, + TimeoutSeconds: 3, + FailureThreshold: 3, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{HTTPGet: &corev1.HTTPGetAction{ + Path: "/v2/", Port: intstr.FromString(registryName), + }}, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + TimeoutSeconds: 3, + FailureThreshold: 3, + }, Resources: controlPlaneResources(), SecurityContext: hardenedContainerSecurityContext(), } diff --git a/internal/platform/workloads_test.go b/internal/platform/workloads_test.go index f498109..b2fcef8 100644 --- a/internal/platform/workloads_test.go +++ b/internal/platform/workloads_test.go @@ -1,6 +1,7 @@ package platform import ( + "fmt" "testing" appsv1 "k8s.io/api/apps/v1" @@ -8,6 +9,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/intstr" ) // podSpec returns the single container and the pod template of a Deployment, @@ -373,6 +375,9 @@ func TestOperatorDeployment_Wiring(t *testing.T) { if !contains(c.Args, "--namespace") || !contains(c.Args, p.MinecraftNamespace) { t.Errorf("operator must watch --namespace %s, got %v", p.MinecraftNamespace, c.Args) } + if !contains(c.Args, "--health-probe-bind-address") || !contains(c.Args, fmt.Sprintf(":%d", operatorHealthPort)) { + t.Errorf("operator must bind the health probe listener on :%d, got %v", operatorHealthPort, c.Args) + } if c.Image != p.FelisImage { t.Errorf("operator image = %q, want FelisImage %q", c.Image, p.FelisImage) } @@ -453,6 +458,64 @@ func TestRegistry_DeploymentServicePVC(t *testing.T) { } } +// TestWorkloads_DeploymentsCarryProbes pins probes on ALL three rendered +// Deployments (api, operator, registry): an unprobed control plane cannot be +// told apart from a wedged one, and every probe must target a port the container +// actually declares — a probe pointed at a dead port would leave the pod +// NotReady forever and surface only as a mysteriously empty Service. +func TestWorkloads_DeploymentsCarryProbes(t *testing.T) { + p := testParams().withDefaults() + cases := []struct { + dep *appsv1.Deployment + readyPath, livePath string + port int32 + }{ + {APIDeployment(p), "/readyz", "/healthz", apiInternalPort}, + {OperatorDeployment(p), "/readyz", "/healthz", operatorHealthPort}, + {registryDeployment(p), "/v2/", "/v2/", p.RegistryPort}, + } + // resolve maps a probe target (by number or container-port name) to the + // declared container port it denotes. + resolve := func(port intstr.IntOrString, ports []corev1.ContainerPort) (int32, bool) { + if port.IntValue() != 0 { + return int32(port.IntValue()), true + } + for _, cp := range ports { + if cp.Name == port.StrVal { + return cp.ContainerPort, true + } + } + return 0, false + } + for _, tc := range cases { + _, c := podSpec(t, tc.dep) + if c.ReadinessProbe == nil || c.ReadinessProbe.HTTPGet == nil { + t.Fatalf("%s: readiness probe missing or not an HTTP GET", tc.dep.Name) + } + if c.LivenessProbe == nil || c.LivenessProbe.HTTPGet == nil { + t.Fatalf("%s: liveness probe missing or not an HTTP GET", tc.dep.Name) + } + for _, probe := range []struct { + kind string + p *corev1.Probe + path string + }{ + {"readiness", c.ReadinessProbe, tc.readyPath}, + {"liveness", c.LivenessProbe, tc.livePath}, + } { + if got := probe.p.HTTPGet.Path; got != probe.path { + t.Errorf("%s: %s probe path = %q, want %q", tc.dep.Name, probe.kind, got, probe.path) + } + got, ok := resolve(probe.p.HTTPGet.Port, c.Ports) + if !ok { + t.Errorf("%s: %s probe targets %v, which the container does not declare", tc.dep.Name, probe.kind, probe.p.HTTPGet.Port) + } else if got != tc.port { + t.Errorf("%s: %s probe port = %d, want %d", tc.dep.Name, probe.kind, got, tc.port) + } + } + } +} + // TestWorkloads_BundleContents sanity-checks the slice Workloads returns: the two // control-plane Deployments, the api external+internal Services, and the registry // Deployment/Service/PVC, every one with TypeMeta (so its YAML header renders). The