Skip to content

use cluster-name as default label everywhere #782

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Feb 19, 2020
Merged

use cluster-name as default label everywhere #782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fce6f2c
use cluster-name as default label everywhere
FxKu Jan 7, 2020
9950f8c
fix e2e test
FxKu Jan 8, 2020
c991651
merge with master and resolve conflicts
FxKu Feb 19, 2020
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
2 changes: 1 addition & 1 deletion charts/postgres-operator/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ configKubernetes:
# additional labels assigned to the cluster objects
cluster_labels: application:spilo
# label assigned to Kubernetes objects created by the operator
cluster_name_label: version
cluster_name_label: cluster-name
# annotations attached to each database pod
# custom_pod_annotations: "keya:valuea,keyb:valueb"

Expand Down
2 changes: 1 addition & 1 deletion docs/user.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ our test cluster.

```bash
# get name of master pod of acid-minimal-cluster
export PGMASTER=$(kubectl get pods -o jsonpath={.items..metadata.name} -l application=spilo,version=acid-minimal-cluster,spilo-role=master)
export PGMASTER=$(kubectl get pods -o jsonpath={.items..metadata.name} -l application=spilo,cluster-name=acid-minimal-cluster,spilo-role=master)

# set up port forward
kubectl port-forward $PGMASTER 6432:5432
Expand Down
22 changes: 11 additions & 11 deletions e2e/tests/test_e2e.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def test_enable_load_balancer(self):
'''

k8s = self.k8s
cluster_label = 'version=acid-minimal-cluster'
cluster_label = 'cluster-name=acid-minimal-cluster'

# enable load balancer services
pg_patch_enable_lbs = {
Expand Down Expand Up @@ -113,7 +113,7 @@ def test_min_resource_limits(self):
Lower resource limits below configured minimum and let operator fix it
'''
k8s = self.k8s
cluster_label = 'version=acid-minimal-cluster'
cluster_label = 'cluster-name=acid-minimal-cluster'
_, failover_targets = k8s.get_pg_nodes(cluster_label)

# configure minimum boundaries for CPU and memory limits
Expand Down Expand Up @@ -172,15 +172,15 @@ def test_multi_namespace_support(self):

k8s.create_with_kubectl("manifests/complete-postgres-manifest.yaml")
k8s.wait_for_pod_start("spilo-role=master", self.namespace)
self.assert_master_is_unique(self.namespace, version="acid-test-cluster")
self.assert_master_is_unique(self.namespace, "acid-test-cluster")

@timeout_decorator.timeout(TEST_TIMEOUT_SEC)
def test_scaling(self):
'''
Scale up from 2 to 3 and back to 2 pods by updating the Postgres manifest at runtime.
'''
k8s = self.k8s
labels = "version=acid-minimal-cluster"
labels = "cluster-name=acid-minimal-cluster"

k8s.wait_for_pg_to_scale(3)
self.assertEqual(3, k8s.count_pods_with_label(labels))
Expand All @@ -196,7 +196,7 @@ def test_taint_based_eviction(self):
Add taint "postgres=:NoExecute" to node with master. This must cause a failover.
'''
k8s = self.k8s
cluster_label = 'version=acid-minimal-cluster'
cluster_label = 'cluster-name=acid-minimal-cluster'

# get nodes of master and replica(s) (expected target of new master)
current_master_node, failover_targets = k8s.get_pg_nodes(cluster_label)
Expand Down Expand Up @@ -334,9 +334,9 @@ def test_service_annotations(self):
"foo": "bar",
}
self.assertTrue(k8s.check_service_annotations(
"version=acid-service-annotations,spilo-role=master", annotations))
"cluster-name=acid-service-annotations,spilo-role=master", annotations))
self.assertTrue(k8s.check_service_annotations(
"version=acid-service-annotations,spilo-role=replica", annotations))
"cluster-name=acid-service-annotations,spilo-role=replica", annotations))

# clean up
unpatch_custom_service_annotations = {
Expand All @@ -346,14 +346,14 @@ def test_service_annotations(self):
}
k8s.update_config(unpatch_custom_service_annotations)

def assert_master_is_unique(self, namespace='default', version="acid-minimal-cluster"):
def assert_master_is_unique(self, namespace='default', clusterName="acid-minimal-cluster"):
'''
Check that there is a single pod in the k8s cluster with the label "spilo-role=master"
To be called manually after operations that affect pods
'''

k8s = self.k8s
labels = 'spilo-role=master,version=' + version
labels = 'spilo-role=master,cluster-name=' + clusterName

num_of_master_pods = k8s.count_pods_with_label(labels, namespace)
self.assertEqual(num_of_master_pods, 1, "Expected 1 master pod, found {}".format(num_of_master_pods))
Expand Down Expand Up @@ -438,7 +438,7 @@ def wait_for_pg_to_scale(self, number_of_instances, namespace='default'):
_ = self.api.custom_objects_api.patch_namespaced_custom_object(
"acid.zalan.do", "v1", namespace, "postgresqls", "acid-minimal-cluster", body)

labels = 'version=acid-minimal-cluster'
labels = 'cluster-name=acid-minimal-cluster'
while self.count_pods_with_label(labels) != number_of_instances:
time.sleep(self.RETRY_TIMEOUT_SEC)

Expand All @@ -448,7 +448,7 @@ def count_pods_with_label(self, labels, namespace='default'):
def wait_for_master_failover(self, expected_master_nodes, namespace='default'):
pod_phase = 'Failing over'
new_master_node = ''
labels = 'spilo-role=master,version=acid-minimal-cluster'
labels = 'spilo-role=master,cluster-name=acid-minimal-cluster'
Copy link
Member

Choose a reason for hiding this comment

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

I dont like that this is hard coded. We should have the end to end test covering that the label can change to make sure config variable is picked up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but there are even more selectors hard coded here: spilo-role, name etc.
But true, I could add a test on changing such labels


while (pod_phase != 'Running') or (new_master_node not in expected_master_nodes):
pods = self.api.core_v1.list_namespaced_pod(namespace, label_selector=labels).items
Expand Down
2 changes: 1 addition & 1 deletion manifests/configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ data:
cluster_domain: cluster.local
cluster_history_entries: "1000"
cluster_labels: application:spilo
cluster_name_label: version
cluster_name_label: cluster-name
# custom_service_annotations: "keyx:valuez,keya:valuea"
# custom_pod_annotations: "keya:valuea,keyb:valueb"
db_hosted_zone: db.example.com
Expand Down
4 changes: 2 additions & 2 deletions pkg/cluster/k8sres.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1498,8 +1498,8 @@ func (c *Cluster) generateLogicalBackupJob() (*batchv1beta1.CronJob, error) {
)

labels := map[string]string{
"version": c.Name,
"application": "spilo-logical-backup",
c.OpConfig.ClusterNameLabel: c.Name,
"application": "spilo-logical-backup",
}
podAffinityTerm := v1.PodAffinityTerm{
LabelSelector: &metav1.LabelSelector{
Expand Down