diff --git a/internal/controller/databasecluster_probes.go b/internal/controller/databasecluster_probes.go index becc103..06ded53 100644 --- a/internal/controller/databasecluster_probes.go +++ b/internal/controller/databasecluster_probes.go @@ -27,18 +27,27 @@ import ( // addDatabaseProbes attaches startup, readiness, and liveness probes // to the database container. // -// The probe is a single pg_isready check against the role's local port. -// Port discovery: +// Probe strategy by role: // -// coordinator, standby → fixed 5432 -// segment → 6000 + ordinal (derived from $HOSTNAME) -// mirror → 7000 + ordinal (derived from $HOSTNAME) +// coordinator, segment → pg_isready against the local port (5432 / 6000). +// These roles accept normal SQL connections, so +// pg_isready returns "accepting connections" once +// postgres is up. +// mirror, standby → NO probes. These roles run postgres in continuous +// recovery mode (replaying WAL from a primary) and +// deliberately do NOT accept normal connections. +// pg_isready returns non-zero against them, which +// triggers restart loops in Kubernetes. Mirror and +// standby health is detected by the operator via +// gp_segment_configuration (status='u', mode='s'), +// so K8s-level probes are both unnecessary and +// actively harmful here. // -// 3-probe layout: +// 3-probe layout for coordinator/segment: // // StartupProbe — generous boot window (~10 min) to absorb gpinitsystem // on coord-0 and the wait-for-coordinator phase on -// segments/mirrors. Once it succeeds, readiness + liveness +// segments. Once it succeeds, readiness + liveness // take over. // ReadinessProbe — tight. Pod only appears in Service endpoints when // postgres truly accepts connections. @@ -48,14 +57,18 @@ import ( // All probes pass cleanly through StatefulSet rollouts and operator restarts // because they read state from the pod itself, not from operator status. func addDatabaseProbes(c *corev1.Container, suffix string, image *keldoniov1alpha1.DatabaseImage) { + // Skip probes for replicas in recovery mode. See doc comment above. + if suffix == "mirror" || suffix == "mirrors" || + suffix == "standby" { + return + } + var portExpr string switch suffix { - case "coordinator", "standby": + case "coordinator": portExpr = "5432" case "segment", "segments": portExpr = "6000" - case "mirror", "mirrors": - portExpr = "7000" default: portExpr = "5432" } diff --git a/internal/controller/databasecluster_service.go b/internal/controller/databasecluster_service.go index 2b95b45..8db3d83 100644 --- a/internal/controller/databasecluster_service.go +++ b/internal/controller/databasecluster_service.go @@ -89,6 +89,7 @@ func (r *DatabaseClusterReconciler) ensureServicesClient(ctx context.Context, cl TargetPort: intstr.FromInt(5432), }, } + Service.Labels = ClusterLabels(cluster, "") Service.Spec.Selector = desiredSelector ctrl.SetControllerReference(cluster, &Service, r.Scheme) return r.Create(ctx, &Service) @@ -133,6 +134,7 @@ func (r *DatabaseClusterReconciler) ensureHeadlessService(ctx context.Context, c headlessService.Name = headlessServiceNamespacedName.Name headlessService.Namespace = headlessServiceNamespacedName.Namespace headlessService.Spec.ClusterIP = "None" + headlessService.Labels = ClusterLabels(cluster, suffix) headlessService.Spec.PublishNotReadyAddresses = true headlessService.Spec.Selector = map[string]string{ "app": cluster.Name + "-" + suffix, diff --git a/internal/controller/databasecluster_ssh.go b/internal/controller/databasecluster_ssh.go index d3278de..f49cc09 100644 --- a/internal/controller/databasecluster_ssh.go +++ b/internal/controller/databasecluster_ssh.go @@ -325,10 +325,12 @@ func (c *certificateSSHTrust) ensureOneCA( return fmt.Errorf("marshal CA to Secret: %w", err) } - if secret.Labels == nil { - secret.Labels = map[string]string{} + for k, v := range ClusterLabels(cluster, "ssh") { + if secret.Labels == nil { + secret.Labels = map[string]string{} + } + secret.Labels[k] = v } - secret.Labels["keldon.io/cluster"] = cluster.Name secret.Labels["keldon.io/ssh-ca"] = strings.TrimPrefix(suffix, "-ssh-") if err := controllerutil.SetControllerReference(cluster, secret, c.r.Scheme); err != nil { diff --git a/internal/controller/databasecluster_statefulset.go b/internal/controller/databasecluster_statefulset.go index 0551f00..7c8bf70 100644 --- a/internal/controller/databasecluster_statefulset.go +++ b/internal/controller/databasecluster_statefulset.go @@ -178,17 +178,22 @@ func (r *DatabaseClusterReconciler) buildStatefulset( statefulset.Name = cluster.Name + "-" + suffix statefulset.Namespace = cluster.Namespace + // Canonical keldon.io/* labels on the StatefulSet itself so it shows up + // in `kubectl get sts -l keldon.io/cluster=` queries. + statefulset.Labels = ClusterLabels(cluster, suffix) statefulset.Spec.PodManagementPolicy = appsv1.ParallelPodManagement + // Selector is immutable on existing StatefulSets — keep using the legacy + // `app:` label so v0.11.0 → v0.11.1 upgrades don't break. statefulset.Spec.Selector = &metav1.LabelSelector{ MatchLabels: map[string]string{ "app": cluster.Name + "-" + suffix, }, } - statefulset.Spec.Template.Labels = map[string]string{ - "app": cluster.Name + "-" + suffix, - } + // Pod template labels include both the legacy `app:` (required by the + // selector above) and the canonical keldon.io/* labels. + statefulset.Spec.Template.Labels = WithLegacyApp(cluster, suffix, suffix) // Init container: create the three data-dir parents and chown to UID 1000 // (the adminUser's expected UID inside the image — every Greenplum-family @@ -250,7 +255,10 @@ func (r *DatabaseClusterReconciler) buildStatefulset( statefulset.Spec.VolumeClaimTemplates = []corev1.PersistentVolumeClaim{ { - ObjectMeta: metav1.ObjectMeta{Name: "data"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "data", + Labels: ClusterLabels(cluster, suffix), + }, Spec: corev1.PersistentVolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, StorageClassName: storageClassName, diff --git a/internal/controller/labels.go b/internal/controller/labels.go new file mode 100644 index 0000000..7b44bbe --- /dev/null +++ b/internal/controller/labels.go @@ -0,0 +1,48 @@ +package controller + +import ( + keldonv1alpha1 "github.com/keldonio/keldon-operator/api/v1alpha1" +) + +// Label keys emitted by the operator on all cluster resources. +// Use these as the source of truth — do not hard-code label strings. +const ( + LabelCluster = "keldon.io/cluster" + LabelRole = "keldon.io/role" + LabelManagedBy = "keldon.io/managed-by" + LabelAppName = "app.kubernetes.io/name" + LabelAppInstance = "app.kubernetes.io/instance" + LabelAppComponent = "app.kubernetes.io/component" + LabelAppManagedBy = "app.kubernetes.io/managed-by" + + ValueManagedBy = "keldon-operator" + ValueAppName = "keldon" +) + +// ClusterLabels returns the full metadata.labels set for any resource +// belonging to a cluster. Use for resource ObjectMeta.Labels and pod +// template metadata. Do NOT use for Selector.MatchLabels. +func ClusterLabels(cluster *keldonv1alpha1.DatabaseCluster, role string) map[string]string { + labels := map[string]string{ + LabelCluster: cluster.Name, + LabelManagedBy: ValueManagedBy, + LabelAppName: ValueAppName, + LabelAppInstance: cluster.Name, + LabelAppManagedBy: ValueManagedBy, + } + if role != "" { + labels[LabelRole] = role + labels[LabelAppComponent] = role + } + return labels +} + +// WithLegacyApp returns a label set that includes both ClusterLabels +// and the legacy `app: -` label required by existing +// StatefulSet selectors. Use for pod template metadata where the +// StatefulSet selector still uses the legacy `app:` label. +func WithLegacyApp(cluster *keldonv1alpha1.DatabaseCluster, role, appSuffix string) map[string]string { + labels := ClusterLabels(cluster, role) + labels["app"] = cluster.Name + "-" + appSuffix + return labels +}