From cb55d11fb40d5b05a8344a310a1b43094bbbb8f9 Mon Sep 17 00:00:00 2001 From: Max Begenau Date: Sun, 4 Dec 2022 22:32:29 +0100 Subject: [PATCH 1/2] feat(498): Add ownerReferences to managed entities --- pkg/cluster/k8sres.go | 65 +++++++-------- pkg/cluster/k8sres_test.go | 162 ++++++++++++++++++++++--------------- 2 files changed, 128 insertions(+), 99 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 8be32f09c..d83646cb0 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1442,10 +1442,11 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef statefulSet := &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ - Name: c.statefulSetName(), - Namespace: c.Namespace, - Labels: c.labelsSet(true), - Annotations: c.AnnotationsToPropagate(c.annotationsSet(nil)), + Name: c.statefulSetName(), + Namespace: c.Namespace, + Labels: c.labelsSet(true), + Annotations: c.AnnotationsToPropagate(c.annotationsSet(nil)), + OwnerReferences: c.ownerReferences(), }, Spec: appsv1.StatefulSetSpec{ Replicas: &numberOfInstances, @@ -1835,10 +1836,11 @@ func (c *Cluster) generateSingleUserSecret(namespace string, pgUser spec.PgUser) secret := v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: c.credentialSecretName(username), - Namespace: pgUser.Namespace, - Labels: lbls, - Annotations: c.annotationsSet(nil), + Name: c.credentialSecretName(username), + Namespace: pgUser.Namespace, + Labels: lbls, + Annotations: c.annotationsSet(nil), + OwnerReferences: c.ownerReferences(), }, Type: v1.SecretTypeOpaque, Data: map[string][]byte{ @@ -1896,10 +1898,11 @@ func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec) service := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: c.serviceName(role), - Namespace: c.Namespace, - Labels: c.roleLabelsSet(true, role), - Annotations: c.annotationsSet(c.generateServiceAnnotations(role, spec)), + Name: c.serviceName(role), + Namespace: c.Namespace, + Labels: c.roleLabelsSet(true, role), + Annotations: c.annotationsSet(c.generateServiceAnnotations(role, spec)), + OwnerReferences: c.ownerReferences(), }, Spec: serviceSpec, } @@ -1965,9 +1968,10 @@ func (c *Cluster) getCustomServiceAnnotations(role PostgresRole, spec *acidv1.Po func (c *Cluster) generateEndpoint(role PostgresRole, subsets []v1.EndpointSubset) *v1.Endpoints { endpoints := &v1.Endpoints{ ObjectMeta: metav1.ObjectMeta{ - Name: c.endpointName(role), - Namespace: c.Namespace, - Labels: c.roleLabelsSet(true, role), + Name: c.endpointName(role), + Namespace: c.Namespace, + Labels: c.roleLabelsSet(true, role), + OwnerReferences: c.ownerReferences(), }, } if len(subsets) > 0 { @@ -2121,10 +2125,11 @@ func (c *Cluster) generatePodDisruptionBudget() *policyv1.PodDisruptionBudget { return &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ - Name: c.podDisruptionBudgetName(), - Namespace: c.Namespace, - Labels: c.labelsSet(true), - Annotations: c.annotationsSet(nil), + Name: c.podDisruptionBudgetName(), + Namespace: c.Namespace, + Labels: c.labelsSet(true), + Annotations: c.annotationsSet(nil), + OwnerReferences: c.ownerReferences(), }, Spec: policyv1.PodDisruptionBudgetSpec{ MinAvailable: &minAvailable, @@ -2246,10 +2251,11 @@ func (c *Cluster) generateLogicalBackupJob() (*batchv1.CronJob, error) { cronJob := &batchv1.CronJob{ ObjectMeta: metav1.ObjectMeta{ - Name: c.getLogicalBackupJobName(), - Namespace: c.Namespace, - Labels: c.labelsSet(true), - Annotations: c.annotationsSet(nil), + Name: c.getLogicalBackupJobName(), + Namespace: c.Namespace, + Labels: c.labelsSet(true), + Annotations: c.annotationsSet(nil), + OwnerReferences: c.ownerReferences(), }, Spec: batchv1.CronJobSpec{ Schedule: schedule, @@ -2385,17 +2391,12 @@ func (c *Cluster) getLogicalBackupJobName() (jobName string) { func (c *Cluster) ownerReferences() []metav1.OwnerReference { controller := true - if c.Statefulset == nil { - c.logger.Warning("Cannot get owner reference, no statefulset") - return []metav1.OwnerReference{} - } - return []metav1.OwnerReference{ { - UID: c.Statefulset.ObjectMeta.UID, - APIVersion: "apps/v1", - Kind: "StatefulSet", - Name: c.Statefulset.ObjectMeta.Name, + UID: c.Postgresql.ObjectMeta.UID, + APIVersion: acidv1.SchemeGroupVersion.Identifier(), + Kind: acidv1.PostgresCRDResourceKind, + Name: c.Postgresql.ObjectMeta.Name, Controller: &controller, }, } diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index a88320deb..e95dd97c9 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -1491,9 +1491,9 @@ func TestPodAffinity(t *testing.T) { func testDeploymentOwnerReference(cluster *Cluster, deployment *appsv1.Deployment) error { 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 @@ -1502,9 +1502,9 @@ func testDeploymentOwnerReference(cluster *Cluster, deployment *appsv1.Deploymen func testServiceOwnerReference(cluster *Cluster, service *v1.Service, role PostgresRole) error { 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 @@ -2201,13 +2201,65 @@ 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 { + 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 !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 { + 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{ @@ -2215,23 +2267,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{ @@ -2239,23 +2284,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{ @@ -2263,23 +2301,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{ @@ -2287,26 +2318,23 @@ 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, }, }, } 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) + } } } } From 99b56cfac2b423b827791c8aa3190204e7e64c61 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Mon, 1 Jul 2024 19:09:23 +0200 Subject: [PATCH 2/2] Update pkg/cluster/k8sres.go --- pkg/cluster/k8sres.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 42cac5921..bdd35940d 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -2068,7 +2068,7 @@ func (c *Cluster) generateEndpoint(role PostgresRole, subsets []v1.EndpointSubse Namespace: c.Namespace, Annotations: c.annotationsSet(nil), Labels: c.roleLabelsSet(true, role), - OwnerReferences: c.ownerReferences(), + OwnerReferences: c.ownerReferences(), }, } if len(subsets) > 0 {