Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

[timescale-multinode]feat: Add possibility to specify internal ip range #311

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

griseau
Copy link

@griseau griseau commented Oct 29, 2021

When running inside a kubernetes cluster, internal ip range could be different (for example could be 10.108.0.0/14 on GCP).

This PR allow you to set the internal IP range so that it's added inside the pg_hba.conf file during init.

Happy to take feedback :)

@CLAassistant
Copy link

CLAassistant commented Oct 29, 2021

CLA assistant check
All committers have signed the CLA.

@griseau griseau changed the title feat: Add possibility to specify internal ip range [timescale-multinode]feat: Add possibility to specify internal ip range Oct 29, 2021
@@ -79,6 +79,10 @@ spec:
echo "*:*:*:postgres:${POSTGRES_PASSWORD_DATA_NODE}" > "${PGDATA}/../.pgpass"
chown postgres:postgres "${PGDATA}/../.pgpass" "${PGDATA}/postgresql_helm_customizations.conf"
chmod 0600 "${PGDATA}/../.pgpass"
{{- if .Values.internalIpRange }}
echo "Adding {{ .Values.internalIpRange }} in pg_hba.conf"
echo "host all all {{ .Values.internalIpRange }} trust" >> ${PGDATA}/pg_hba.conf
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't that be run on pod restarts too?

Copy link
Author

Choose a reason for hiding this comment

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

I did not think about this case, but you're right, coming from Kubernetes documentation :
"If the Pod restarts, or is restarted, all init containers must execute again." https://kubernetes.io/docs/concepts/workloads/pods/init-containers/

It's also the case for the command executed just above this one. How would you handle it ? Override the whole file instead of just appending this new line ?

Copy link
Author

Choose a reason for hiding this comment

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

Or I can do the same as it's done here

Copy link
Author

Choose a reason for hiding this comment

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

@paulfantom updated according to your comment :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about it a bit more and even with grep this may go sideways.

Let's assume someone is doing an upgrade of the helm chart and changed internalIpRange value. In such a scenario grep would fail and echo would be run again, which in turn would result in pg_hba.conf having old and new configuration options.

For me it look like we have 2 options:

  1. Force full management of pg_hba.conf via ConfigMap. (I am not really a fan of this)
  2. Create line markers and modify only options between line markers.

A bit more explanation on option 2. On the first run of the helm chart, we would add 3 lines, something like:

# START of the section managed by helm chart
host all all {{ .Values.internalIpRange }} trust
# END of the section managed by helm chart

Later in all subsequent runs, we can use grep to look for such a section and edit only the content of this section. This way even if internalIpRange changed on upgrade, helm chart would be able to update it accordingly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants