Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add owner references to all resources #2199

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 30 additions & 29 deletions pkg/cluster/k8sres.go
Original file line number Diff line number Diff line change
Expand Up @@ -1530,10 +1530,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,
Expand Down Expand Up @@ -1931,10 +1932,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{
Expand Down Expand Up @@ -1992,10 +1994,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,
}
Expand Down Expand Up @@ -2065,6 +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(),
},
}
if len(subsets) > 0 {
Expand Down Expand Up @@ -2225,10 +2229,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,
Expand Down Expand Up @@ -2361,10 +2366,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,
Expand Down Expand Up @@ -2521,17 +2527,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,
},
}
Expand Down
162 changes: 95 additions & 67 deletions pkg/cluster/k8sres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1568,9 +1568,9 @@
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
Expand All @@ -1579,9 +1579,9 @@
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
Expand Down Expand Up @@ -2320,109 +2320,133 @@
}

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{
Comment on lines +2347 to +2354
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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{
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{

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And final test case must be changed to

		{
			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),
			check: []func(cluster *Cluster, podDisruptionBudget *policyv1.PodDisruptionBudget) error{
				testPodDisruptionBudgetOwnerReference,
				hasName("postgres-myapp-database-pdb"),
				hasMinAvailable(1),
				testLabelsAndSelectors,
			},
		},

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{
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{"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{
ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"},
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{
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(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{
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-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(

Check failure on line 2449 in pkg/cluster/k8sres_test.go

View workflow job for this annotation

GitHub Actions / Unit tests and coverage

cannot use New(Config{…}, k8sutil.KubernetesClient{}, acidv1.Postgresql{…}, logger, eventRecorder) (value of type *Cluster) as string value in struct literal

Check failure on line 2449 in pkg/cluster/k8sres_test.go

View workflow job for this annotation

GitHub Actions / End-2-End tests

cannot use New(Config{…}, k8sutil.KubernetesClient{}, acidv1.Postgresql{…}, logger, eventRecorder) (value of type *Cluster) as string value in struct literal
Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb", PDBMasterLabelSelector: util.False()}},
k8sutil.KubernetesClient{},
acidv1.Postgresql{
Expand All @@ -2430,7 +2454,7 @@
Spec: acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}},
logger,
eventRecorder),
policyv1.PodDisruptionBudget{

Check failure on line 2457 in pkg/cluster/k8sres_test.go

View workflow job for this annotation

GitHub Actions / Unit tests and coverage

cannot use policyv1.PodDisruptionBudget{…} (value of type "k8s.io/api/policy/v1".PodDisruptionBudget) as *Cluster value in struct literal

Check failure on line 2457 in pkg/cluster/k8sres_test.go

View workflow job for this annotation

GitHub Actions / End-2-End tests

cannot use policyv1.PodDisruptionBudget{…} (value of type "k8s.io/api/policy/v1".PodDisruptionBudget) as *Cluster value in struct literal
ObjectMeta: metav1.ObjectMeta{
Name: "postgres-myapp-database-pdb",
Namespace: "myapp",
Expand All @@ -2443,13 +2467,17 @@
},
},
},
},

Check failure on line 2470 in pkg/cluster/k8sres_test.go

View workflow job for this annotation

GitHub Actions / Unit tests and coverage

too few values in struct literal of type struct{scenario string; spec *Cluster; check []func(cluster *Cluster, podDisruptionBudget *"k8s.io/api/policy/v1".PodDisruptionBudget) error}

Check failure on line 2470 in pkg/cluster/k8sres_test.go

View workflow job for this annotation

GitHub Actions / End-2-End tests

too few values in struct literal of type struct{scenario string; spec *Cluster; check []func(cluster *Cluster, podDisruptionBudget *"k8s.io/api/policy/v1".PodDisruptionBudget) error}
}

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)
}
}
}
}
Expand Down
Loading