From 24bf2f3fa0544b5b667e88535b071cc4c65e4bc2 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 4 Sep 2023 10:03:37 +0200 Subject: [PATCH] Fix bug with kubernetes.standby_leader_label_value (#2832) When running with the leader lock Patroni was just setting the `role` label to `master` and effectively `kubernetes.standby_leader_label_value` feature never worked. Now it is fixed, but in order to not introduce breaking changes we just update default value of the `standby_leader_label_value` to the `master`. --- docs/ENVIRONMENT.rst | 2 +- docs/yaml_configuration.rst | 2 +- patroni/dcs/kubernetes.py | 9 +++------ tests/test_kubernetes.py | 12 +++++++----- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/docs/ENVIRONMENT.rst b/docs/ENVIRONMENT.rst index 00595cf7..e3762aca 100644 --- a/docs/ENVIRONMENT.rst +++ b/docs/ENVIRONMENT.rst @@ -115,7 +115,7 @@ Kubernetes - **PATRONI\_KUBERNETES\_ROLE\_LABEL**: (optional) name of the label containing role (master or replica or other custom value). Patroni will set this label on the pod it runs in. Default value is ``role``. - **PATRONI\_KUBERNETES\_LEADER\_LABEL\_VALUE**: (optional) value of the pod label when Postgres role is `master`. Default value is `master`. - **PATRONI\_KUBERNETES\_FOLLOWER\_LABEL\_VALUE**: (optional) value of the pod label when Postgres role is `replica`. Default value is `replica`. -- **PATRONI\_KUBERNETES\_STANDBY\_LEADER\_LABEL\_VALUE**: (optional) value of the pod label when Postgres role is ``standby-leader``. Default value is ``standby-leader``. +- **PATRONI\_KUBERNETES\_STANDBY\_LEADER\_LABEL\_VALUE**: (optional) value of the pod label when Postgres role is ``standby_leader``. Default value is ``master``. - **PATRONI\_KUBERNETES\_TMP\_ROLE\_LABEL**: (optional) name of the temporary label containing role (master or replica). Value of this label will always use the default of corresponding role. Set only when necessary. - **PATRONI\_KUBERNETES\_USE\_ENDPOINTS**: (optional) if set to true, Patroni will use Endpoints instead of ConfigMaps to run leader elections and keep cluster state. - **PATRONI\_KUBERNETES\_POD\_IP**: (optional) IP address of the pod Patroni is running in. This value is required when `PATRONI_KUBERNETES_USE_ENDPOINTS` is enabled and is used to populate the leader endpoint subsets when the pod's PostgreSQL is promoted. diff --git a/docs/yaml_configuration.rst b/docs/yaml_configuration.rst index ab5b5587..e78e3515 100644 --- a/docs/yaml_configuration.rst +++ b/docs/yaml_configuration.rst @@ -158,7 +158,7 @@ Kubernetes - **role\_label**: (optional) name of the label containing role (master or replica or other custom value). Patroni will set this label on the pod it runs in. Default value is ``role``. - **leader\_label\_value**: (optional) value of the pod label when Postgres role is ``master``. Default value is ``master``. - **follower\_label\_value**: (optional) value of the pod label when Postgres role is ``replica``. Default value is ``replica``. -- **standby\_leader\_label\_value**: (optional) value of the pod label when Postgres role is ``standby-leader``. Default value is ``standby-leader``. +- **standby\_leader\_label\_value**: (optional) value of the pod label when Postgres role is ``standby_leader``. Default value is ``master``. - **tmp_\role\_label**: (optional) name of the temporary label containing role (master or replica). Value of this label will always use the default of corresponding role. Set only when necessary. - **use\_endpoints**: (optional) if set to true, Patroni will use Endpoints instead of ConfigMaps to run leader elections and keep cluster state. - **pod\_ip**: (optional) IP address of the pod Patroni is running in. This value is required when `use_endpoints` is enabled and is used to populate the leader endpoint subsets when the pod's PostgreSQL is promoted. diff --git a/patroni/dcs/kubernetes.py b/patroni/dcs/kubernetes.py index 4a66df71..d51aa0ff 100644 --- a/patroni/dcs/kubernetes.py +++ b/patroni/dcs/kubernetes.py @@ -756,7 +756,7 @@ class Kubernetes(AbstractDCS): self._role_label = config.get('role_label', 'role') self._leader_label_value = config.get('leader_label_value', 'master') self._follower_label_value = config.get('follower_label_value', 'replica') - self._standby_leader_label_value = config.get('standby_leader_label_value', 'standby-leader') + self._standby_leader_label_value = config.get('standby_leader_label_value', 'master') self._tmp_role_label = config.get('tmp_role_label') self._ca_certs = os.environ.get('PATRONI_KUBERNETES_CACERT', config.get('cacert')) or SERVICE_CERT_FILENAME super(Kubernetes, self).__init__({**config, 'namespace': ''}) @@ -1269,13 +1269,10 @@ class Kubernetes(AbstractDCS): def touch_member(self, data: Dict[str, Any]) -> bool: cluster = self.cluster if cluster and cluster.leader and cluster.leader.name == self._name: - role = self._leader_label_value + role = self._standby_leader_label_value if data['role'] == 'standby_leader' else self._leader_label_value tmp_role = 'master' elif data['state'] == 'running' and data['role'] not in ('master', 'primary'): - role = { - 'replica': self._follower_label_value, - 'standby-leader': self._standby_leader_label_value, - }.get(data['role'], data['role']) + role = {'replica': self._follower_label_value}.get(data['role'], data['role']) tmp_role = data['role'] else: role = None diff --git a/tests/test_kubernetes.py b/tests/test_kubernetes.py index 9eaa173a..19f868c8 100644 --- a/tests/test_kubernetes.py +++ b/tests/test_kubernetes.py @@ -308,13 +308,15 @@ class TestKubernetesConfigMaps(BaseTestKubernetes): mock_patch_namespaced_pod.assert_called() self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['isMaster'], 'false') self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['tmp_role'], 'replica') - - self.k.touch_member({'state': 'running', 'role': 'standby-leader'}) - mock_patch_namespaced_pod.assert_called() - self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['isMaster'], 'false') - self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['tmp_role'], 'standby-leader') + mock_patch_namespaced_pod.rest_mock() self.k._name = 'p-0' + self.k.touch_member({'role': 'standby_leader'}) + mock_patch_namespaced_pod.assert_called() + self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['isMaster'], 'false') + self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['tmp_role'], 'master') + mock_patch_namespaced_pod.rest_mock() + self.k.touch_member({'role': 'primary'}) mock_patch_namespaced_pod.assert_called() self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['isMaster'], 'true')