From 4d252177775dd69256b9cda878e138762b061c07 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Mon, 28 Mar 2022 11:28:23 +0200 Subject: [PATCH 1/9] define readinessProbe on statefulSet when Patroni uses ConfigMaps --- pkg/cluster/cluster.go | 8 +++++--- pkg/cluster/k8sres.go | 39 ++++++++++++++++++++++++++++++------- pkg/util/patroni/patroni.go | 4 ++-- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index dcef602b9..922fa463e 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -258,6 +258,8 @@ func (c *Cluster) Create() error { for _, role := range []PostgresRole{Master, Replica} { + // if kubernetes_use_configmaps is set Patroni will create configmaps + // otherwise it will use endpoints if !c.patroniKubernetesUseConfigMaps() { if c.Endpoints[role] != nil { return fmt.Errorf("%s endpoint already exists in the cluster", role) @@ -1576,10 +1578,10 @@ func (c *Cluster) deletePatroniClusterObjects() error { c.logger.Infof("not cleaning up Etcd Patroni objects on cluster delete") } - if !c.patroniKubernetesUseConfigMaps() { - actionsList = append(actionsList, c.deletePatroniClusterEndpoints) - } else { + if c.patroniKubernetesUseConfigMaps() { actionsList = append(actionsList, c.deletePatroniClusterServices, c.deletePatroniClusterConfigMaps) + } else { + actionsList = append(actionsList, c.deletePatroniClusterEndpoints) } c.logger.Debugf("removing leftover Patroni objects (endpoints / services and configmaps)") diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index e545be7ef..3bde89ed2 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -26,6 +26,7 @@ import ( "github.com/zalando/postgres-operator/pkg/util/config" "github.com/zalando/postgres-operator/pkg/util/constants" "github.com/zalando/postgres-operator/pkg/util/k8sutil" + "github.com/zalando/postgres-operator/pkg/util/patroni" "github.com/zalando/postgres-operator/pkg/util/retryutil" batchv1 "k8s.io/api/batch/v1" batchv1beta1 "k8s.io/api/batch/v1beta1" @@ -111,7 +112,7 @@ func (c *Cluster) servicePort(role PostgresRole) int32 { return service.Spec.Ports[0].Port } - c.logger.Warningf("No service for role %s - defaulting to port 5432", role) + c.logger.Warningf("No service for role %s - defaulting to port %d", role, pgPort) return pgPort } @@ -558,15 +559,15 @@ func generateContainer( Resources: *resourceRequirements, Ports: []v1.ContainerPort{ { - ContainerPort: 8008, + ContainerPort: patroni.ApiPort, Protocol: v1.ProtocolTCP, }, { - ContainerPort: 5432, + ContainerPort: pgPort, Protocol: v1.ProtocolTCP, }, { - ContainerPort: 8080, + ContainerPort: patroni.ApiPort, Protocol: v1.ProtocolTCP, }, }, @@ -1058,6 +1059,22 @@ func extractPgVersionFromBinPath(binPath string, template string) (string, error return fmt.Sprintf("%v", pgVersion), nil } +func generateSpiloReadinessProbe() *v1.Probe { + return &v1.Probe{ + Handler: v1.Handler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/readiness", + Port: intstr.IntOrString{IntVal: patroni.ApiPort}, + }, + }, + InitialDelaySeconds: 6, + PeriodSeconds: 10, + TimeoutSeconds: 5, + SuccessThreshold: 1, + FailureThreshold: 3, + } +} + func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.StatefulSet, error) { var ( @@ -1239,6 +1256,12 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef generateCapabilities(c.OpConfig.AdditionalPodCapabilities), ) + // if kubernetes_use_configmaps define a readinessProbe since the master service has a selector + // Patroni responds 200 to probe only if it either owns the leader lock or postgres is running and DCS is accessible + if c.patroniKubernetesUseConfigMaps() { + spiloContainer.ReadinessProbe = generateSpiloReadinessProbe() + } + // generate container specs for sidecars specified in the cluster manifest clusterSpecificSidecars := []v1.Container{} if spec.Sidecars != nil && len(spec.Sidecars) > 0 { @@ -1708,10 +1731,12 @@ func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *ac func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec) *v1.Service { serviceSpec := v1.ServiceSpec{ - Ports: []v1.ServicePort{{Name: "postgresql", Port: 5432, TargetPort: intstr.IntOrString{IntVal: 5432}}}, + Ports: []v1.ServicePort{{Name: "postgresql", Port: pgPort, TargetPort: intstr.IntOrString{IntVal: pgPort}}}, Type: v1.ServiceTypeClusterIP, } + // no selector for master, see https://github.com/zalando/postgres-operator/issues/340 + // if kubernetes_use_configmaps is set master service needs a selector if role == Replica || c.patroniKubernetesUseConfigMaps() { serviceSpec.Selector = c.roleLabelsSet(false, role) } @@ -1989,7 +2014,7 @@ func (c *Cluster) generatePodDisruptionBudget() *policybeta1.PodDisruptionBudget // TODO: handle clusters in different namespaces func (c *Cluster) getClusterServiceConnectionParameters(clusterName string) (host string, port string) { host = clusterName - port = "5432" + port = fmt.Sprintf("%d", pgPort) return } @@ -2170,7 +2195,7 @@ func (c *Cluster) generateLogicalBackupPodEnvVars() []v1.EnvVar { }, { Name: "PGPORT", - Value: "5432", + Value: fmt.Sprintf("%d", pgPort), }, { Name: "PGUSER", diff --git a/pkg/util/patroni/patroni.go b/pkg/util/patroni/patroni.go index d3b2f28f0..8126eddc7 100644 --- a/pkg/util/patroni/patroni.go +++ b/pkg/util/patroni/patroni.go @@ -25,7 +25,7 @@ const ( clusterPath = "/cluster" statusPath = "/patroni" restartPath = "/restart" - apiPort = 8008 + ApiPort = 8008 timeout = 30 * time.Second ) @@ -74,7 +74,7 @@ func apiURL(masterPod *v1.Pod) (string, error) { return "", fmt.Errorf("%s is not a valid IPv4/IPv6 address", masterPod.Status.PodIP) } } - return fmt.Sprintf("http://%s", net.JoinHostPort(ip.String(), strconv.Itoa(apiPort))), nil + return fmt.Sprintf("http://%s", net.JoinHostPort(ip.String(), strconv.Itoa(ApiPort))), nil } func (p *Patroni) httpPostOrPatch(method string, url string, body *bytes.Buffer) (err error) { From 4b893f9267f2ece2591afa79f7daaa8ac0d0500a Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Mon, 28 Mar 2022 11:30:07 +0200 Subject: [PATCH 2/9] fix patronit_test.go --- pkg/util/patroni/patroni_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/util/patroni/patroni_test.go b/pkg/util/patroni/patroni_test.go index 60f289c6f..aa6ad9206 100644 --- a/pkg/util/patroni/patroni_test.go +++ b/pkg/util/patroni/patroni_test.go @@ -36,17 +36,17 @@ func TestApiURL(t *testing.T) { }{ { "127.0.0.1", - fmt.Sprintf("http://127.0.0.1:%d", apiPort), + fmt.Sprintf("http://127.0.0.1:%d", ApiPort), nil, }, { "0000:0000:0000:0000:0000:0000:0000:0001", - fmt.Sprintf("http://[::1]:%d", apiPort), + fmt.Sprintf("http://[::1]:%d", ApiPort), nil, }, { "::1", - fmt.Sprintf("http://[::1]:%d", apiPort), + fmt.Sprintf("http://[::1]:%d", ApiPort), nil, }, { From 2c4e5db33499c9e70b9ea937fe9397fca60df35f Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Mon, 28 Mar 2022 12:01:21 +0200 Subject: [PATCH 3/9] enable readinessProbe either way --- pkg/cluster/k8sres.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 3bde89ed2..625c6a813 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1256,11 +1256,8 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef generateCapabilities(c.OpConfig.AdditionalPodCapabilities), ) - // if kubernetes_use_configmaps define a readinessProbe since the master service has a selector // Patroni responds 200 to probe only if it either owns the leader lock or postgres is running and DCS is accessible - if c.patroniKubernetesUseConfigMaps() { - spiloContainer.ReadinessProbe = generateSpiloReadinessProbe() - } + spiloContainer.ReadinessProbe = generateSpiloReadinessProbe() // generate container specs for sidecars specified in the cluster manifest clusterSpecificSidecars := []v1.Container{} From aa8b4bf365d8f6c8791a26a542920d3323e3f42c Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Tue, 29 Mar 2022 11:05:06 +0200 Subject: [PATCH 4/9] do not error out on deleting Patroni cluster objects --- pkg/cluster/cluster.go | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 922fa463e..c4b7fd831 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1564,7 +1564,7 @@ func (c *Cluster) Unlock() { c.mu.Unlock() } -type simpleActionWithResult func() error +type simpleActionWithResult func() type clusterObjectGet func(name string) (spec.NamespacedName, error) @@ -1586,38 +1586,38 @@ func (c *Cluster) deletePatroniClusterObjects() error { c.logger.Debugf("removing leftover Patroni objects (endpoints / services and configmaps)") for _, deleter := range actionsList { - if err := deleter(); err != nil { - return err - } + deleter() } return nil } -func (c *Cluster) deleteClusterObject( +func deleteClusterObject( get clusterObjectGet, del clusterObjectDelete, - objType string) error { + objType string, + clusterName string, + logger *logrus.Entry) { for _, suffix := range patroniObjectSuffixes { - name := fmt.Sprintf("%s-%s", c.Name, suffix) + name := fmt.Sprintf("%s-%s", clusterName, suffix) - if namespacedName, err := get(name); err == nil { - c.logger.Debugf("deleting Patroni cluster object %q with name %q", + namespacedName, err := get(name) + if err == nil { + logger.Debugf("deleting %s %q", objType, namespacedName) if err = del(name); err != nil { - return fmt.Errorf("could not delete Patroni cluster object %q with name %q: %v", + logger.Warningf("could not delete %s %q: %v", objType, namespacedName, err) } } else if !k8sutil.ResourceNotFound(err) { - return fmt.Errorf("could not fetch Patroni Endpoint %q: %v", - namespacedName, err) + logger.Warningf("could not fetch %s %q: %v", + objType, namespacedName, err) } } - return nil } -func (c *Cluster) deletePatroniClusterServices() error { +func (c *Cluster) deletePatroniClusterServices() { get := func(name string) (spec.NamespacedName, error) { svc, err := c.KubeClient.Services(c.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) return util.NameFromMeta(svc.ObjectMeta), err @@ -1627,10 +1627,10 @@ func (c *Cluster) deletePatroniClusterServices() error { return c.KubeClient.Services(c.Namespace).Delete(context.TODO(), name, c.deleteOptions) } - return c.deleteClusterObject(get, deleteServiceFn, "service") + deleteClusterObject(get, deleteServiceFn, "service", c.Name, c.logger) } -func (c *Cluster) deletePatroniClusterEndpoints() error { +func (c *Cluster) deletePatroniClusterEndpoints() { get := func(name string) (spec.NamespacedName, error) { ep, err := c.KubeClient.Endpoints(c.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) return util.NameFromMeta(ep.ObjectMeta), err @@ -1640,10 +1640,10 @@ func (c *Cluster) deletePatroniClusterEndpoints() error { return c.KubeClient.Endpoints(c.Namespace).Delete(context.TODO(), name, c.deleteOptions) } - return c.deleteClusterObject(get, deleteEndpointFn, "endpoint") + deleteClusterObject(get, deleteEndpointFn, "endpoint", c.Name, c.logger) } -func (c *Cluster) deletePatroniClusterConfigMaps() error { +func (c *Cluster) deletePatroniClusterConfigMaps() { get := func(name string) (spec.NamespacedName, error) { cm, err := c.KubeClient.ConfigMaps(c.Namespace).Get(context.TODO(), name, metav1.GetOptions{}) return util.NameFromMeta(cm.ObjectMeta), err @@ -1653,5 +1653,5 @@ func (c *Cluster) deletePatroniClusterConfigMaps() error { return c.KubeClient.ConfigMaps(c.Namespace).Delete(context.TODO(), name, c.deleteOptions) } - return c.deleteClusterObject(get, deleteConfigMapFn, "configmap") + deleteClusterObject(get, deleteConfigMapFn, "configmap", c.Name, c.logger) } From d643ad5e212a5a63d4bc93f01f85ccbfb5e83271 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Tue, 29 Mar 2022 15:25:59 +0200 Subject: [PATCH 5/9] add code to sync config maps --- .../templates/clusterrole-postgres-pod.yaml | 8 +- .../templates/clusterrole.yaml | 6 - docs/quickstart.md | 14 +- e2e/tests/test_e2e.py | 2 + manifests/configmap.yaml | 2 +- ...erator-service-account-rbac-openshift.yaml | 283 ++++++++++++++++++ pkg/cluster/cluster.go | 27 +- pkg/cluster/k8sres.go | 21 +- pkg/cluster/resources.go | 36 ++- pkg/cluster/sync.go | 40 ++- pkg/cluster/types.go | 2 + pkg/controller/postgresql.go | 3 +- 12 files changed, 409 insertions(+), 35 deletions(-) create mode 100644 manifests/operator-service-account-rbac-openshift.yaml diff --git a/charts/postgres-operator/templates/clusterrole-postgres-pod.yaml b/charts/postgres-operator/templates/clusterrole-postgres-pod.yaml index 33c43822f..fdccf16d3 100644 --- a/charts/postgres-operator/templates/clusterrole-postgres-pod.yaml +++ b/charts/postgres-operator/templates/clusterrole-postgres-pod.yaml @@ -9,7 +9,7 @@ metadata: app.kubernetes.io/managed-by: {{ .Release.Service }} app.kubernetes.io/instance: {{ .Release.Name }} rules: -# Patroni needs to watch and manage endpoints +# Patroni needs to watch and manage config maps or endpoints {{- if toString .Values.configGeneral.kubernetes_use_configmaps | eq "true" }} - apiGroups: - "" @@ -24,12 +24,6 @@ rules: - patch - update - watch -- apiGroups: - - "" - resources: - - endpoints - verbs: - - get {{- else }} - apiGroups: - "" diff --git a/charts/postgres-operator/templates/clusterrole.yaml b/charts/postgres-operator/templates/clusterrole.yaml index 8b2e9136e..199086acc 100644 --- a/charts/postgres-operator/templates/clusterrole.yaml +++ b/charts/postgres-operator/templates/clusterrole.yaml @@ -89,12 +89,6 @@ rules: - patch - update - watch -- apiGroups: - - "" - resources: - - endpoints - verbs: - - get {{- else }} # to read configuration from ConfigMaps - apiGroups: diff --git a/docs/quickstart.md b/docs/quickstart.md index ed01367b7..7049a6ef9 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -37,7 +37,7 @@ The Postgres Operator can be deployed in the following ways: * Kustomization * Helm chart -### Manual deployment setup +### Manual deployment setup on Kubernetes The Postgres Operator can be installed simply by applying yaml manifests. Note, we provide the `/manifests` directory as an example only; you should consider @@ -71,6 +71,18 @@ manifest. ./run_operator_locally.sh ``` +### Manual deployment setup on OpenShift + +To install the Postgres Operator in OpenShift you have to change the config +parameter `kubernetes_use_configmaps` to `"true"`. Otherwise, the operator +and Patroni will store leader and config keys in `Endpoints` that are not +supported in OpenShift. This requires also a slightly different set of rules +for the `postgres-operator` and `postgres-pod` cluster roles. + +```bash +oc create -f manifests/operator-service-account-rbac-openshift.yaml +``` + ### Helm chart Alternatively, the operator can be installed by using the provided [Helm](https://helm.sh/) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index b2977b687..0f9515896 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -1759,6 +1759,8 @@ def test_zz_cluster_deletion(self): self.eventuallyEqual(lambda: len(k8s.api.custom_objects_api.list_namespaced_custom_object( "acid.zalan.do", "v1", "default", "postgresqls", label_selector="cluster-name=acid-minimal-cluster")["items"]), 0, "Manifest not deleted") + self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") + # check if everything has been deleted self.eventuallyEqual(lambda: k8s.count_pods_with_label(cluster_label), 0, "Pods not deleted") self.eventuallyEqual(lambda: k8s.count_services_with_label(cluster_label), 0, "Service not deleted") diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index 130a35176..8b392544b 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -63,13 +63,13 @@ data: # etcd_host: "" external_traffic_policy: "Cluster" # gcp_credentials: "" - # kubernetes_use_configmaps: "false" # ignored_annotations: "" # infrastructure_roles_secret_name: "postgresql-infrastructure-roles" # infrastructure_roles_secrets: "secretname:monitoring-roles,userkey:user,passwordkey:password,rolekey:inrole" # inherited_annotations: owned-by # inherited_labels: application,environment # kube_iam_role: "" + # kubernetes_use_configmaps: "false" # log_s3_bucket: "" logical_backup_docker_image: "registry.opensource.zalan.do/acid/logical-backup:v1.7.1" # logical_backup_google_application_credentials: "" diff --git a/manifests/operator-service-account-rbac-openshift.yaml b/manifests/operator-service-account-rbac-openshift.yaml new file mode 100644 index 000000000..e0e45cc54 --- /dev/null +++ b/manifests/operator-service-account-rbac-openshift.yaml @@ -0,0 +1,283 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: postgres-operator + namespace: default + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: postgres-operator +rules: +# all verbs allowed for custom operator resources +- apiGroups: + - acid.zalan.do + resources: + - postgresqls + - postgresqls/status + - operatorconfigurations + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch +# operator only reads PostgresTeams +- apiGroups: + - acid.zalan.do + resources: + - postgresteams + verbs: + - get + - list + - watch +# all verbs allowed for event streams (Zalando-internal feature) +# - apiGroups: +# - zalando.org +# resources: +# - fabriceventstreams +# verbs: +# - create +# - delete +# - deletecollection +# - get +# - list +# - patch +# - update +# - watch +# to create or get/update CRDs when starting up +- apiGroups: + - apiextensions.k8s.io + resources: + - customresourcedefinitions + verbs: + - create + - get + - patch + - update +# to read configuration and manage ConfigMaps used by Patroni +- apiGroups: + - "" + resources: + - configmaps + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch +# to send events to the CRs +- apiGroups: + - "" + resources: + - events + verbs: + - create + - get + - list + - patch + - update + - watch +# to CRUD secrets for database access +- apiGroups: + - "" + resources: + - secrets + verbs: + - create + - delete + - get + - update +# to check nodes for node readiness label +- apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list + - watch +# to read or delete existing PVCs. Creation via StatefulSet +- apiGroups: + - "" + resources: + - persistentvolumeclaims + verbs: + - delete + - get + - list + - patch + - update + # to read existing PVs. Creation should be done via dynamic provisioning +- apiGroups: + - "" + resources: + - persistentvolumes + verbs: + - get + - list + - update # only for resizing AWS volumes +# to watch Spilo pods and do rolling updates. Creation via StatefulSet +- apiGroups: + - "" + resources: + - pods + verbs: + - delete + - get + - list + - patch + - update + - watch +# to resize the filesystem in Spilo pods when increasing volume size +- apiGroups: + - "" + resources: + - pods/exec + verbs: + - create +# to CRUD services to point to Postgres cluster instances +- apiGroups: + - "" + resources: + - services + verbs: + - create + - delete + - get + - patch + - update +# to CRUD the StatefulSet which controls the Postgres cluster instances +- apiGroups: + - apps + resources: + - statefulsets + - deployments + verbs: + - create + - delete + - get + - list + - patch +# to CRUD cron jobs for logical backups +- apiGroups: + - batch + resources: + - cronjobs + verbs: + - create + - delete + - get + - list + - patch + - update +# to get namespaces operator resources can run in +- apiGroups: + - "" + resources: + - namespaces + verbs: + - get +# to define PDBs. Update happens via delete/create +- apiGroups: + - policy + resources: + - poddisruptionbudgets + verbs: + - create + - delete + - get +# to create ServiceAccounts in each namespace the operator watches +- apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - get + - create +# to create role bindings to the postgres-pod service account +- apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + verbs: + - get + - create +# to grant privilege to run privileged pods (not needed by default) +#- apiGroups: +# - extensions +# resources: +# - podsecuritypolicies +# resourceNames: +# - privileged +# verbs: +# - use + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: postgres-operator +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: postgres-operator +subjects: +- kind: ServiceAccount + name: postgres-operator + namespace: default + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: postgres-pod +rules: +# Patroni needs to watch and manage config maps +- apiGroups: + - "" + resources: + - configmaps + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch +# Patroni needs to watch pods +- apiGroups: + - "" + resources: + - pods + verbs: + - get + - list + - patch + - update + - watch +# to let Patroni create a headless service +- apiGroups: + - "" + resources: + - services + verbs: + - create +# to grant privilege to run privileged pods (not needed by default) +#- apiGroups: +# - extensions +# resources: +# - podsecuritypolicies +# resourceNames: +# - privileged +# verbs: +# - use diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index c4b7fd831..3475a8958 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -59,6 +59,7 @@ type Config struct { type kubeResources struct { Services map[PostgresRole]*v1.Service Endpoints map[PostgresRole]*v1.Endpoints + ConfigMaps map[PostgresRole]*v1.ConfigMap Secrets map[types.UID]*v1.Secret Statefulset *appsv1.StatefulSet PodDisruptionBudget *policybeta1.PodDisruptionBudget @@ -1484,22 +1485,29 @@ func (c *Cluster) GetCurrentProcess() Process { // GetStatus provides status of the cluster func (c *Cluster) GetStatus() *ClusterStatus { - return &ClusterStatus{ - Cluster: c.Spec.ClusterName, - Team: c.Spec.TeamID, - Status: c.Status, - Spec: c.Spec, - + status := &ClusterStatus{ + Cluster: c.Spec.ClusterName, + Team: c.Spec.TeamID, + Status: c.Status, + Spec: c.Spec, MasterService: c.GetServiceMaster(), ReplicaService: c.GetServiceReplica(), - MasterEndpoint: c.GetEndpointMaster(), - ReplicaEndpoint: c.GetEndpointReplica(), StatefulSet: c.GetStatefulSet(), PodDisruptionBudget: c.GetPodDisruptionBudget(), CurrentProcess: c.GetCurrentProcess(), Error: fmt.Errorf("error: %s", c.Error), } + + if c.patroniKubernetesUseConfigMaps() { + status.MasterEndpoint = c.GetEndpointMaster() + status.ReplicaEndpoint = c.GetEndpointReplica() + } else { + status.MasterConfigMap = c.GetConfigMapMaster() + status.ReplicaConfigMap = c.GetConfigMapReplica() + } + + return status } // Switchover does a switchover (via Patroni) to a candidate pod @@ -1579,10 +1587,11 @@ func (c *Cluster) deletePatroniClusterObjects() error { } if c.patroniKubernetesUseConfigMaps() { - actionsList = append(actionsList, c.deletePatroniClusterServices, c.deletePatroniClusterConfigMaps) + actionsList = append(actionsList, c.deletePatroniClusterConfigMaps) } else { actionsList = append(actionsList, c.deletePatroniClusterEndpoints) } + actionsList = append(actionsList, c.deletePatroniClusterServices) c.logger.Debugf("removing leftover Patroni objects (endpoints / services and configmaps)") for _, deleter := range actionsList { diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 625c6a813..cd47358c0 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -76,13 +76,12 @@ func (c *Cluster) statefulSetName() string { return c.Name } -func (c *Cluster) endpointName(role PostgresRole) string { - name := c.Name - if role == Replica { - name = name + "-repl" - } +func (c *Cluster) configMapName(role PostgresRole) string { + return c.serviceName(role) +} - return name +func (c *Cluster) endpointName(role PostgresRole) string { + return c.serviceName(role) } func (c *Cluster) serviceName(role PostgresRole) string { @@ -1821,6 +1820,16 @@ func (c *Cluster) generateEndpoint(role PostgresRole, subsets []v1.EndpointSubse return endpoints } +func (c *Cluster) generateConfigMap(role PostgresRole) *v1.ConfigMap { + return &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: c.configMapName(role), + Namespace: c.Namespace, + Labels: c.roleLabelsSet(true, role), + }, + } +} + func (c *Cluster) generateCloneEnvironment(description *acidv1.CloneDescription) []v1.EnvVar { result := make([]v1.EnvVar, 0) diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index 5e5c6156e..b2d8d672a 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -35,8 +35,14 @@ func (c *Cluster) listResources() error { c.logger.Infof("found secret: %q (uid: %q) namesapce: %s", util.NameFromMeta(obj.ObjectMeta), obj.UID, obj.ObjectMeta.Namespace) } - for role, endpoint := range c.Endpoints { - c.logger.Infof("found %s endpoint: %q (uid: %q)", role, util.NameFromMeta(endpoint.ObjectMeta), endpoint.UID) + if c.patroniKubernetesUseConfigMaps() { + for role, configMap := range c.ConfigMaps { + c.logger.Infof("found %s config map: %q (uid: %q)", role, util.NameFromMeta(configMap.ObjectMeta), configMap.UID) + } + } else { + for role, endpoint := range c.Endpoints { + c.logger.Infof("found %s endpoint: %q (uid: %q)", role, util.NameFromMeta(endpoint.ObjectMeta), endpoint.UID) + } } for role, service := range c.Services { @@ -402,6 +408,20 @@ func (c *Cluster) generateEndpointSubsets(role PostgresRole) []v1.EndpointSubset return result } +func (c *Cluster) createConfigMap(role PostgresRole) (*v1.ConfigMap, error) { + c.setProcessName("creating config map") + configMapSpec := c.generateConfigMap(role) + + configMap, err := c.KubeClient.ConfigMaps(configMapSpec.Namespace).Create(context.TODO(), configMapSpec, metav1.CreateOptions{}) + if err != nil { + return nil, fmt.Errorf("could not create %s config map: %v", role, err) + } + + c.ConfigMaps[role] = configMap + + return configMap, nil +} + func (c *Cluster) createPodDisruptionBudget() (*policybeta1.PodDisruptionBudget, error) { podDisruptionBudgetSpec := c.generatePodDisruptionBudget() podDisruptionBudget, err := c.KubeClient. @@ -589,11 +609,21 @@ func (c *Cluster) GetEndpointMaster() *v1.Endpoints { return c.Endpoints[Master] } -// GetEndpointReplica returns cluster's kubernetes master Endpoint +// GetEndpointReplica returns cluster's kubernetes replica Endpoint func (c *Cluster) GetEndpointReplica() *v1.Endpoints { return c.Endpoints[Replica] } +// GetConfigMapMaster returns cluster's kubernetes master ConfigMap +func (c *Cluster) GetConfigMapMaster() *v1.ConfigMap { + return c.ConfigMaps[Master] +} + +// GetConfigMapReplica returns cluster's kubernetes replica ConfigMap +func (c *Cluster) GetConfigMapReplica() *v1.ConfigMap { + return c.ConfigMaps[Replica] +} + // GetStatefulSet returns cluster's kubernetes StatefulSet func (c *Cluster) GetStatefulSet() *appsv1.StatefulSet { return c.Statefulset diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 7ff021ceb..f3f811e23 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -144,7 +144,11 @@ func (c *Cluster) syncServices() error { for _, role := range []PostgresRole{Master, Replica} { c.logger.Debugf("syncing %s service", role) - if !c.patroniKubernetesUseConfigMaps() { + if c.patroniKubernetesUseConfigMaps() { + if err := c.syncConfigMap(role); err != nil { + return fmt.Errorf("could not sync %s config map: %v", role, err) + } + } else { if err := c.syncEndpoint(role); err != nil { return fmt.Errorf("could not sync %s endpoint: %v", role, err) } @@ -234,6 +238,40 @@ func (c *Cluster) syncEndpoint(role PostgresRole) error { return nil } +func (c *Cluster) syncConfigMap(role PostgresRole) error { + var ( + cm *v1.ConfigMap + err error + ) + c.setProcessName("syncing %s config map", role) + + if cm, err = c.KubeClient.ConfigMaps(c.Namespace).Get(context.TODO(), c.configMapName(role), metav1.GetOptions{}); err == nil { + // TODO: No syncing of config map here, is this covered completely by updateService? + c.ConfigMaps[role] = cm + return nil + } + if !k8sutil.ResourceNotFound(err) { + return fmt.Errorf("could not get %s config map: %v", role, err) + } + // no existing config map, create new one + c.ConfigMaps[role] = nil + c.logger.Infof("could not find the cluster's %s config map", role) + + if cm, err = c.createConfigMap(role); err == nil { + c.logger.Infof("created missing %s config map %q", role, util.NameFromMeta(cm.ObjectMeta)) + } else { + if !k8sutil.ResourceAlreadyExists(err) { + return fmt.Errorf("could not create missing %s config map: %v", role, err) + } + c.logger.Infof("%s config map %q already exists", role, util.NameFromMeta(cm.ObjectMeta)) + if cm, err = c.KubeClient.ConfigMaps(c.Namespace).Get(context.TODO(), c.configMapName(role), metav1.GetOptions{}); err != nil { + return fmt.Errorf("could not fetch existing %s config map: %v", role, err) + } + } + c.ConfigMaps[role] = cm + return nil +} + func (c *Cluster) syncPodDisruptionBudget(isUpdate bool) error { var ( pdb *policybeta1.PodDisruptionBudget diff --git a/pkg/cluster/types.go b/pkg/cluster/types.go index 67b4ee395..c271fd019 100644 --- a/pkg/cluster/types.go +++ b/pkg/cluster/types.go @@ -63,6 +63,8 @@ type ClusterStatus struct { ReplicaService *v1.Service MasterEndpoint *v1.Endpoints ReplicaEndpoint *v1.Endpoints + MasterConfigMap *v1.ConfigMap + ReplicaConfigMap *v1.ConfigMap StatefulSet *appsv1.StatefulSet PodDisruptionBudget *policybeta1.PodDisruptionBudget diff --git a/pkg/controller/postgresql.go b/pkg/controller/postgresql.go index 590494412..aac078933 100644 --- a/pkg/controller/postgresql.go +++ b/pkg/controller/postgresql.go @@ -544,7 +544,8 @@ func (c *Controller) postgresqlCheck(obj interface{}) *acidv1.Postgresql { Ensures the pod service account and role bindings exists in a namespace before a PG cluster is created there so that a user does not have to deploy these credentials manually. StatefulSets require the service account to - create pods; Patroni requires relevant RBAC bindings to access endpoints. + create pods; Patroni requires relevant RBAC bindings to access endpoints + or config maps. The operator does not sync accounts/role bindings after creation. */ From 22b6abcf95b1dbdff78fcb9b2de6566a83da6c64 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Tue, 29 Mar 2022 20:27:00 +0200 Subject: [PATCH 6/9] master configmap has leader suffix --- pkg/cluster/k8sres.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index cd47358c0..276fcf87f 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -77,7 +77,15 @@ func (c *Cluster) statefulSetName() string { } func (c *Cluster) configMapName(role PostgresRole) string { - return c.serviceName(role) + name := c.Name + if role == Master { + name = name + "-leader" + } + if role == Replica { + name = name + "-repl" + } + + return name } func (c *Cluster) endpointName(role PostgresRole) string { From b40188f0df990eda0a51247487f18858a2ed8f60 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 30 Mar 2022 10:07:27 +0200 Subject: [PATCH 7/9] revert sync configmaps --- pkg/cluster/cluster.go | 5 +---- pkg/cluster/k8sres.go | 19 +------------------ pkg/cluster/resources.go | 30 +----------------------------- pkg/cluster/sync.go | 40 +--------------------------------------- pkg/cluster/types.go | 2 -- 5 files changed, 4 insertions(+), 92 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 3475a8958..52f291dce 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1499,12 +1499,9 @@ func (c *Cluster) GetStatus() *ClusterStatus { Error: fmt.Errorf("error: %s", c.Error), } - if c.patroniKubernetesUseConfigMaps() { + if !c.patroniKubernetesUseConfigMaps() { status.MasterEndpoint = c.GetEndpointMaster() status.ReplicaEndpoint = c.GetEndpointReplica() - } else { - status.MasterConfigMap = c.GetConfigMapMaster() - status.ReplicaConfigMap = c.GetConfigMapReplica() } return status diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 276fcf87f..625c6a813 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -76,11 +76,8 @@ func (c *Cluster) statefulSetName() string { return c.Name } -func (c *Cluster) configMapName(role PostgresRole) string { +func (c *Cluster) endpointName(role PostgresRole) string { name := c.Name - if role == Master { - name = name + "-leader" - } if role == Replica { name = name + "-repl" } @@ -88,10 +85,6 @@ func (c *Cluster) configMapName(role PostgresRole) string { return name } -func (c *Cluster) endpointName(role PostgresRole) string { - return c.serviceName(role) -} - func (c *Cluster) serviceName(role PostgresRole) string { name := c.Name if role == Replica { @@ -1828,16 +1821,6 @@ func (c *Cluster) generateEndpoint(role PostgresRole, subsets []v1.EndpointSubse return endpoints } -func (c *Cluster) generateConfigMap(role PostgresRole) *v1.ConfigMap { - return &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: c.configMapName(role), - Namespace: c.Namespace, - Labels: c.roleLabelsSet(true, role), - }, - } -} - func (c *Cluster) generateCloneEnvironment(description *acidv1.CloneDescription) []v1.EnvVar { result := make([]v1.EnvVar, 0) diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index b2d8d672a..9a57051df 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -35,11 +35,7 @@ func (c *Cluster) listResources() error { c.logger.Infof("found secret: %q (uid: %q) namesapce: %s", util.NameFromMeta(obj.ObjectMeta), obj.UID, obj.ObjectMeta.Namespace) } - if c.patroniKubernetesUseConfigMaps() { - for role, configMap := range c.ConfigMaps { - c.logger.Infof("found %s config map: %q (uid: %q)", role, util.NameFromMeta(configMap.ObjectMeta), configMap.UID) - } - } else { + if !c.patroniKubernetesUseConfigMaps() { for role, endpoint := range c.Endpoints { c.logger.Infof("found %s endpoint: %q (uid: %q)", role, util.NameFromMeta(endpoint.ObjectMeta), endpoint.UID) } @@ -408,20 +404,6 @@ func (c *Cluster) generateEndpointSubsets(role PostgresRole) []v1.EndpointSubset return result } -func (c *Cluster) createConfigMap(role PostgresRole) (*v1.ConfigMap, error) { - c.setProcessName("creating config map") - configMapSpec := c.generateConfigMap(role) - - configMap, err := c.KubeClient.ConfigMaps(configMapSpec.Namespace).Create(context.TODO(), configMapSpec, metav1.CreateOptions{}) - if err != nil { - return nil, fmt.Errorf("could not create %s config map: %v", role, err) - } - - c.ConfigMaps[role] = configMap - - return configMap, nil -} - func (c *Cluster) createPodDisruptionBudget() (*policybeta1.PodDisruptionBudget, error) { podDisruptionBudgetSpec := c.generatePodDisruptionBudget() podDisruptionBudget, err := c.KubeClient. @@ -614,16 +596,6 @@ func (c *Cluster) GetEndpointReplica() *v1.Endpoints { return c.Endpoints[Replica] } -// GetConfigMapMaster returns cluster's kubernetes master ConfigMap -func (c *Cluster) GetConfigMapMaster() *v1.ConfigMap { - return c.ConfigMaps[Master] -} - -// GetConfigMapReplica returns cluster's kubernetes replica ConfigMap -func (c *Cluster) GetConfigMapReplica() *v1.ConfigMap { - return c.ConfigMaps[Replica] -} - // GetStatefulSet returns cluster's kubernetes StatefulSet func (c *Cluster) GetStatefulSet() *appsv1.StatefulSet { return c.Statefulset diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index f3f811e23..7ff021ceb 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -144,11 +144,7 @@ func (c *Cluster) syncServices() error { for _, role := range []PostgresRole{Master, Replica} { c.logger.Debugf("syncing %s service", role) - if c.patroniKubernetesUseConfigMaps() { - if err := c.syncConfigMap(role); err != nil { - return fmt.Errorf("could not sync %s config map: %v", role, err) - } - } else { + if !c.patroniKubernetesUseConfigMaps() { if err := c.syncEndpoint(role); err != nil { return fmt.Errorf("could not sync %s endpoint: %v", role, err) } @@ -238,40 +234,6 @@ func (c *Cluster) syncEndpoint(role PostgresRole) error { return nil } -func (c *Cluster) syncConfigMap(role PostgresRole) error { - var ( - cm *v1.ConfigMap - err error - ) - c.setProcessName("syncing %s config map", role) - - if cm, err = c.KubeClient.ConfigMaps(c.Namespace).Get(context.TODO(), c.configMapName(role), metav1.GetOptions{}); err == nil { - // TODO: No syncing of config map here, is this covered completely by updateService? - c.ConfigMaps[role] = cm - return nil - } - if !k8sutil.ResourceNotFound(err) { - return fmt.Errorf("could not get %s config map: %v", role, err) - } - // no existing config map, create new one - c.ConfigMaps[role] = nil - c.logger.Infof("could not find the cluster's %s config map", role) - - if cm, err = c.createConfigMap(role); err == nil { - c.logger.Infof("created missing %s config map %q", role, util.NameFromMeta(cm.ObjectMeta)) - } else { - if !k8sutil.ResourceAlreadyExists(err) { - return fmt.Errorf("could not create missing %s config map: %v", role, err) - } - c.logger.Infof("%s config map %q already exists", role, util.NameFromMeta(cm.ObjectMeta)) - if cm, err = c.KubeClient.ConfigMaps(c.Namespace).Get(context.TODO(), c.configMapName(role), metav1.GetOptions{}); err != nil { - return fmt.Errorf("could not fetch existing %s config map: %v", role, err) - } - } - c.ConfigMaps[role] = cm - return nil -} - func (c *Cluster) syncPodDisruptionBudget(isUpdate bool) error { var ( pdb *policybeta1.PodDisruptionBudget diff --git a/pkg/cluster/types.go b/pkg/cluster/types.go index c271fd019..67b4ee395 100644 --- a/pkg/cluster/types.go +++ b/pkg/cluster/types.go @@ -63,8 +63,6 @@ type ClusterStatus struct { ReplicaService *v1.Service MasterEndpoint *v1.Endpoints ReplicaEndpoint *v1.Endpoints - MasterConfigMap *v1.ConfigMap - ReplicaConfigMap *v1.ConfigMap StatefulSet *appsv1.StatefulSet PodDisruptionBudget *policybeta1.PodDisruptionBudget From ebd0fee63a8af51ea1a41727d38163d9c4435a21 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 30 Mar 2022 10:09:34 +0200 Subject: [PATCH 8/9] one more revert --- pkg/cluster/cluster.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 52f291dce..451e1f3e8 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -59,7 +59,6 @@ type Config struct { type kubeResources struct { Services map[PostgresRole]*v1.Service Endpoints map[PostgresRole]*v1.Endpoints - ConfigMaps map[PostgresRole]*v1.ConfigMap Secrets map[types.UID]*v1.Secret Statefulset *appsv1.StatefulSet PodDisruptionBudget *policybeta1.PodDisruptionBudget From 91b49ee1c8dd09039bf1f7220b98071682252b3b Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 30 Mar 2022 14:21:22 +0200 Subject: [PATCH 9/9] change delete order for patroni objects --- pkg/cluster/cluster.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 451e1f3e8..2a08857e3 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -43,7 +43,7 @@ var ( alphaNumericRegexp = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9]*$") databaseNameRegexp = regexp.MustCompile("^[a-zA-Z_][a-zA-Z0-9_]*$") userRegexp = regexp.MustCompile(`^[a-z0-9]([-_a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-_a-z0-9]*[a-z0-9])?)*$`) - patroniObjectSuffixes = []string{"config", "failover", "sync", "leader"} + patroniObjectSuffixes = []string{"leader", "config", "sync", "failover"} ) // Config contains operator-wide clients and configuration used from a cluster. TODO: remove struct duplication. @@ -1582,12 +1582,12 @@ func (c *Cluster) deletePatroniClusterObjects() error { c.logger.Infof("not cleaning up Etcd Patroni objects on cluster delete") } + actionsList = append(actionsList, c.deletePatroniClusterServices) if c.patroniKubernetesUseConfigMaps() { actionsList = append(actionsList, c.deletePatroniClusterConfigMaps) } else { actionsList = append(actionsList, c.deletePatroniClusterEndpoints) } - actionsList = append(actionsList, c.deletePatroniClusterServices) c.logger.Debugf("removing leftover Patroni objects (endpoints / services and configmaps)") for _, deleter := range actionsList {