Feat: enable owner references (#2688)

* feat(498): Add ownerReferences to managed entities * empty owner reference for cross namespace secret and more tests * update ownerReferences of existing resources * removing ownerReference requires Update API call * CR ownerReference on PVC blocks pvc retention policy of statefulset * make ownerreferences optional and disabled by default * update unit test to check len ownerReferences * update codegen * add owner references e2e test * update unit test * add block_owner_deletion field to test owner reference * fix typos and update docs once more * reflect code feedback --------- Co-authored-by: Max Begenau <max@begenau.com>
2026-10-22 10:24:58 +02:00 · 2024-08-09 17:58:25 +02:00
parent d5a88f571a
commit a87307e56b
28 changed files with 535 additions and 206 deletions
@@ -1566,22 +1566,28 @@ func TestPodAffinity(t *testing.T) {
 }

 func testDeploymentOwnerReference(cluster *Cluster, deployment *appsv1.Deployment) error {
+	if len(deployment.ObjectMeta.OwnerReferences) == 0 {
+		return nil
+	}
 	owner := deployment.ObjectMeta.OwnerReferences[0]

-	if owner.Name != cluster.Statefulset.ObjectMeta.Name {
-		return fmt.Errorf("Ownere reference is incorrect, got %s, expected %s",
-			owner.Name, cluster.Statefulset.ObjectMeta.Name)
+	if owner.Name != cluster.Postgresql.ObjectMeta.Name {
+		return fmt.Errorf("Owner reference is incorrect, got %s, expected %s",
+			owner.Name, cluster.Postgresql.ObjectMeta.Name)
 	}

 	return nil
 }

 func testServiceOwnerReference(cluster *Cluster, service *v1.Service, role PostgresRole) error {
+	if len(service.ObjectMeta.OwnerReferences) == 0 {
+		return nil
+	}
 	owner := service.ObjectMeta.OwnerReferences[0]

-	if owner.Name != cluster.Statefulset.ObjectMeta.Name {
-		return fmt.Errorf("Ownere reference is incorrect, got %s, expected %s",
-			owner.Name, cluster.Statefulset.ObjectMeta.Name)
+	if owner.Name != cluster.Postgresql.ObjectMeta.Name {
+		return fmt.Errorf("Owner reference is incorrect, got %s, expected %s",
+			owner.Name, cluster.Postgresql.ObjectMeta.Name)
 	}

 	return nil
@@ -2320,13 +2326,69 @@ func TestSidecars(t *testing.T) {
 }

 func TestGeneratePodDisruptionBudget(t *testing.T) {
+	testName := "Test PodDisruptionBudget spec generation"
+
+	hasName := func(pdbName string) func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error {
+		return func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error {
+			if pdbName != podDisruptionBudget.ObjectMeta.Name {
+				return fmt.Errorf("PodDisruptionBudget name is incorrect, got %s, expected %s",
+					podDisruptionBudget.ObjectMeta.Name, pdbName)
+			}
+			return nil
+		}
+	}
+
+	hasMinAvailable := func(expectedMinAvailable int) func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error {
+		return func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error {
+			actual := podDisruptionBudget.Spec.MinAvailable.IntVal
+			if actual != int32(expectedMinAvailable) {
+				return fmt.Errorf("PodDisruptionBudget MinAvailable is incorrect, got %d, expected %d",
+					actual, expectedMinAvailable)
+			}
+			return nil
+		}
+	}
+
+	testLabelsAndSelectors := func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error {
+		masterLabelSelectorDisabled := cluster.OpConfig.PDBMasterLabelSelector != nil && !*cluster.OpConfig.PDBMasterLabelSelector
+		if podDisruptionBudget.ObjectMeta.Namespace != "myapp" {
+			return fmt.Errorf("Object Namespace incorrect.")
+		}
+		if !reflect.DeepEqual(podDisruptionBudget.Labels, map[string]string{"team": "myapp", "cluster-name": "myapp-database"}) {
+			return fmt.Errorf("Labels incorrect.")
+		}
+		if !masterLabelSelectorDisabled &&
+			!reflect.DeepEqual(podDisruptionBudget.Spec.Selector, &metav1.LabelSelector{
+				MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"}}) {
+
+			return fmt.Errorf("MatchLabels incorrect.")
+		}
+
+		return nil
+	}
+
+	testPodDisruptionBudgetOwnerReference := func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error {
+		if len(podDisruptionBudget.ObjectMeta.OwnerReferences) == 0 {
+			return nil
+		}
+		owner := podDisruptionBudget.ObjectMeta.OwnerReferences[0]
+
+		if owner.Name != cluster.Postgresql.ObjectMeta.Name {
+			return fmt.Errorf("Owner reference is incorrect, got %s, expected %s",
+				owner.Name, cluster.Postgresql.ObjectMeta.Name)
+		}
+
+		return nil
+	}
+
 	tests := []struct {
-		c   *Cluster
-		out policyv1.PodDisruptionBudget
+		scenario string
+		spec     *Cluster
+		check    []func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error
 	}{
-		// With multiple instances.
 		{
-			New(
+			scenario: "With multiple instances",
+			spec: New(
 				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb"}},
 				k8sutil.KubernetesClient{},
 				acidv1.Postgresql{
@@ -2334,23 +2396,16 @@ func TestGeneratePodDisruptionBudget(t *testing.T) {
 					Spec:       acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}},
 				logger,
 				eventRecorder),
-			policyv1.PodDisruptionBudget{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "postgres-myapp-database-pdb",
-					Namespace: "myapp",
-					Labels:    map[string]string{"team": "myapp", "cluster-name": "myapp-database"},
-				},
-				Spec: policyv1.PodDisruptionBudgetSpec{
-					MinAvailable: util.ToIntStr(1),
-					Selector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"},
-					},
-				},
+			check: []func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error{
+				testPodDisruptionBudgetOwnerReference,
+				hasName("postgres-myapp-database-pdb"),
+				hasMinAvailable(1),
+				testLabelsAndSelectors,
 			},
 		},
-		// With zero instances.
 		{
-			New(
+			scenario: "With zero instances",
+			spec: New(
 				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb"}},
 				k8sutil.KubernetesClient{},
 				acidv1.Postgresql{
@@ -2358,23 +2413,16 @@ func TestGeneratePodDisruptionBudget(t *testing.T) {
 					Spec:       acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 0}},
 				logger,
 				eventRecorder),
-			policyv1.PodDisruptionBudget{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "postgres-myapp-database-pdb",
-					Namespace: "myapp",
-					Labels:    map[string]string{"team": "myapp", "cluster-name": "myapp-database"},
-				},
-				Spec: policyv1.PodDisruptionBudgetSpec{
-					MinAvailable: util.ToIntStr(0),
-					Selector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"},
-					},
-				},
+			check: []func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error{
+				testPodDisruptionBudgetOwnerReference,
+				hasName("postgres-myapp-database-pdb"),
+				hasMinAvailable(0),
+				testLabelsAndSelectors,
 			},
 		},
-		// With PodDisruptionBudget disabled.
 		{
-			New(
+			scenario: "With PodDisruptionBudget disabled",
+			spec: New(
 				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb", EnablePodDisruptionBudget: util.False()}},
 				k8sutil.KubernetesClient{},
 				acidv1.Postgresql{
@@ -2382,23 +2430,16 @@ func TestGeneratePodDisruptionBudget(t *testing.T) {
 					Spec:       acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}},
 				logger,
 				eventRecorder),
-			policyv1.PodDisruptionBudget{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "postgres-myapp-database-pdb",
-					Namespace: "myapp",
-					Labels:    map[string]string{"team": "myapp", "cluster-name": "myapp-database"},
-				},
-				Spec: policyv1.PodDisruptionBudgetSpec{
-					MinAvailable: util.ToIntStr(0),
-					Selector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"},
-					},
-				},
+			check: []func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error{
+				testPodDisruptionBudgetOwnerReference,
+				hasName("postgres-myapp-database-pdb"),
+				hasMinAvailable(0),
+				testLabelsAndSelectors,
 			},
 		},
-		// With non-default PDBNameFormat and PodDisruptionBudget explicitly enabled.
 		{
-			New(
+			scenario: "With non-default PDBNameFormat and PodDisruptionBudget explicitly enabled",
+			spec: New(
 				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-databass-budget", EnablePodDisruptionBudget: util.True()}},
 				k8sutil.KubernetesClient{},
 				acidv1.Postgresql{
@@ -2406,50 +2447,57 @@ func TestGeneratePodDisruptionBudget(t *testing.T) {
 					Spec:       acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}},
 				logger,
 				eventRecorder),
-			policyv1.PodDisruptionBudget{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "postgres-myapp-database-databass-budget",
-					Namespace: "myapp",
-					Labels:    map[string]string{"team": "myapp", "cluster-name": "myapp-database"},
-				},
-				Spec: policyv1.PodDisruptionBudgetSpec{
-					MinAvailable: util.ToIntStr(1),
-					Selector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"},
-					},
-				},
+			check: []func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error{
+				testPodDisruptionBudgetOwnerReference,
+				hasName("postgres-myapp-database-databass-budget"),
+				hasMinAvailable(1),
+				testLabelsAndSelectors,
 			},
 		},
-		// With PDBMasterLabelSelector disabled.
 		{
-			New(
-				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb", PDBMasterLabelSelector: util.False()}},
+			scenario: "With PDBMasterLabelSelector disabled",
+			spec: New(
+				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb", EnablePodDisruptionBudget: util.True(), PDBMasterLabelSelector: util.False()}},
 				k8sutil.KubernetesClient{},
 				acidv1.Postgresql{
 					ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"},
 					Spec:       acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}},
 				logger,
 				eventRecorder),
-			policyv1.PodDisruptionBudget{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "postgres-myapp-database-pdb",
-					Namespace: "myapp",
-					Labels:    map[string]string{"team": "myapp", "cluster-name": "myapp-database"},
-				},
-				Spec: policyv1.PodDisruptionBudgetSpec{
-					MinAvailable: util.ToIntStr(1),
-					Selector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{"cluster-name": "myapp-database"},
-					},
-				},
+			check: []func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error{
+				testPodDisruptionBudgetOwnerReference,
+				hasName("postgres-myapp-database-pdb"),
+				hasMinAvailable(1),
+				testLabelsAndSelectors,
+			},
+		},
+		{
+			scenario: "With OwnerReference enabled",
+			spec: New(
+				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role", EnableOwnerReferences: util.True()}, PDBNameFormat: "postgres-{cluster}-pdb", EnablePodDisruptionBudget: util.True()}},
+				k8sutil.KubernetesClient{},
+				acidv1.Postgresql{
+					ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"},
+					Spec:       acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}},
+				logger,
+				eventRecorder),
+			check: []func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error{
+				testPodDisruptionBudgetOwnerReference,
+				hasName("postgres-myapp-database-pdb"),
+				hasMinAvailable(1),
+				testLabelsAndSelectors,
 			},
 		},
 	}

 	for _, tt := range tests {
-		result := tt.c.generatePodDisruptionBudget()
-		if !reflect.DeepEqual(*result, tt.out) {
-			t.Errorf("Expected PodDisruptionBudget: %#v, got %#v", tt.out, *result)
+		result := tt.spec.generatePodDisruptionBudget()
+		for _, check := range tt.check {
+			err := check(tt.spec, result)
+			if err != nil {
+				t.Errorf("%s [%s]: PodDisruptionBudget spec is incorrect, %+v",
+					testName, tt.scenario, err)
+			}
 		}
 	}
 }
@@ -3541,6 +3589,11 @@ func TestGenerateLogicalBackupJob(t *testing.T) {
 		cluster.Spec.LogicalBackupSchedule = tt.specSchedule
 		cronJob, err := cluster.generateLogicalBackupJob()
 		assert.NoError(t, err)
+
+		if !reflect.DeepEqual(cronJob.ObjectMeta.OwnerReferences, cluster.ownerReferences()) {
+			t.Errorf("%s - %s: expected owner references %#v, got %#v", t.Name(), tt.subTest, cluster.ownerReferences(), cronJob.ObjectMeta.OwnerReferences)
+		}
+
 		if cronJob.Spec.Schedule != tt.expectedSchedule {
 			t.Errorf("%s - %s: expected schedule %s, got %s", t.Name(), tt.subTest, tt.expectedSchedule, cronJob.Spec.Schedule)
 		}