Unverified Commit 0c8e29b0 authored by Lemon-miaow's avatar Lemon-miaow
Browse files

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.
parent edefc34a

cmd/felis/operator.go

+21 −0
Changes for cmd/felis/operator.go: 21 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -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
@@ -53,6 +59,7 @@ func cmdOperator(args []string, _, stderr io.Writer) int { 
	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{

		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

internal/platform/workloads.go

+74 −0
Changes for internal/platform/workloads.go: 74 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -66,6 +66,11 @@ const ( 
	apiInternalPort     int32 = 8081

	apiHTTPSPort        int32 = 8443

	operatorMetricsPort int32 = 8080

	// 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"
@@ -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(),

	}

internal/platform/workloads_test.go

+63 −0
Changes for internal/platform/workloads_test.go: 63 added lines, 0 removed lines.
Original line number Diff line number Diff line 
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