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

Closed
wants to merge 3 commits into from

Conversation

mbegenau
Copy link
Contributor

Previously, only the Service and Deployment resource managed by the operator had an OwnerReference linking it to the StatefulSet. Now, all managed resources get an OwnerReference pointing to the Postgresql resource.

Fixes #498.

@mbegenau mbegenau force-pushed the feat-498-ownerReferences branch from 6a81980 to 47ad395 Compare February 2, 2023 18:25
@mbegenau mbegenau marked this pull request as ready for review February 2, 2023 18:26
@FxKu FxKu added this to the 2.0 milestone Feb 8, 2023
@mbegenau mbegenau force-pushed the feat-498-ownerReferences branch from 47ad395 to 1a91adb Compare April 7, 2023 19:06
@batazor
Copy link
Contributor

batazor commented Jun 9, 2023

It would be cool to add support for OwnerReferences references. Is there anything that prevents us from adopting this PR?

@sass1997
Copy link

+1

2 similar comments
@jhpinson
Copy link

+1

@drivebyer
Copy link
Contributor

+1

@mbegenau mbegenau force-pushed the feat-498-ownerReferences branch from fb4d2ed to cb55d11 Compare August 24, 2023 19:26
@Yingrjimsch
Copy link

+1

2 similar comments
@amitde69
Copy link

+1

@itayvolo
Copy link

+1

@metajiji
Copy link

+1 any news?

@FxKu FxKu modified the milestones: 1.11.0, 2.0 Jan 24, 2024
@FxKu FxKu modified the milestones: 2.0, 1.13.0 May 24, 2024
@Demch1k
Copy link
Contributor

Demch1k commented Jun 13, 2024

Hello everyone!
When we wil see it in postgres operator?
It's well needed feature for us, because we can't use ArgoCD with zalando operator without owner references.

@fm2022aa
Copy link

+1

@FxKu FxKu requested a review from macedigital as a code owner July 1, 2024 17:04
Comment on lines +2347 to +2354
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{
Copy link
Member

@FxKu FxKu Jul 1, 2024

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,
			},
		},

@FxKu FxKu closed this in #2688 Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ownerReference to managed entities