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.

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

Jump to bottom

Deprecate InfluxDB #133

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+59 −36
Open

Deprecate InfluxDB #133

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
653b524
feat: allow for PosgreSQL-based analytics, deprecate the in-chart Inf…
khvn26 Apr 26, 2023
f53961d
bump chart version
gagantrivedi May 15, 2024
e2a5a58
fixup! feat: allow for PosgreSQL-based analytics, deprecate the in-ch…
gagantrivedi May 15, 2024
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/flagsmith/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v2
name: flagsmith
description: Flagsmith
type: application
version: 0.39.0
version: 0.40.0
appVersion: 2.114.1
dependencies:
- name: postgresql
Expand Down
8 changes: 3 additions & 5 deletions charts/flagsmith/ci/e2e-test-values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ frontend:
SLACK_TOKEN: "${SLACK_TOKEN}"

api:
analytics:
enabled: true
extraEnv:
EMAIL_BACKEND: 'django.core.mail.backends.console.EmailBackend'
EMAIL_BACKEND: "django.core.mail.backends.console.EmailBackend"
FE_E2E_TEST_USER_EMAIL: [email protected]
influxdb2:
# Needed to set this for the tests to not fail. Possibly related to
# https://github.com/Flagsmith/flagsmith/issues/340
enabled: false
11 changes: 0 additions & 11 deletions charts/flagsmith/ci/influxdb-test-values.yaml
View file
Open in desktop

This file was deleted.

29 changes: 29 additions & 0 deletions charts/flagsmith/templates/NOTES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,35 @@ for information about how to gain web access to the application.

{{- end }}

{{- if not .Values.api.analytics.enabled }}

######################################################
##### Analytics capabilities are off! #####
##### Chart-provisioned InfluxDB is deprecated. #####
######################################################

If you were provisioning an InfluxDB instance for Flagsmith's
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know that we need to worry about this but, just so I have the full picture... is there a way that users will be able to configure the charts such that their chart-provisioned influxdb will continue to be used? Also, I guess this throws the question about migration. We haven't written anything to migrate from InfluxDB to the Postgres backed analytics solution. The experience will actually be that they appear to lose all of the data they had in Influx once the API starts returning data from the postgres DB rather than Influx. So, you're right that the data isn't 'lost' but it will appear to be from the users' perspectives.

Copy link
Member Author

@khvn26 khvn26 Apr 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

is there a way that users will be able to configure the charts such that their chart-provisioned influxdb will continue to be used?

I imagine two paths for this:

  1. Deploying an external InfluxDB instance and migrating the data.
  2. Configuring the ealier chart-provisioned instance as an external one (perhaps we should test and document this).

We haven't written anything to migrate from InfluxDB to the Postgres backed analytics solution.

True. No idea how hard would that be. I'm not sure there'd be high demand for this too, given that InfluxDB is still usable.

analytics with these charts, your data is safe, but no new data
will be written to it.

*In a subsequent release, chart-provisioned InfluxDB and all related
values under the `influxdb2` key will be removed.*

When you enable analytics by setting `api.analytics.enabled`
to `true`, the following database backend will be used for analytics,
in the order of preference:

1. External InfluxDB if `influxdbExternal.enabled` set to `true`
2. Dedicated PostgreSQL specified by the `ANALYTICS_DATABASE_URL`
environment variable
3. External PostgreSQL if `databaseExternal.enabled` set to `true`
4. Chart-provisioned PostgreSQL

Note that you can use a dedicated PostgreSQL instance by providing the
value for `ANALYTICS_DATABASE_URL` environment variable under the
`extraEnvFromSecret` key.
{{- end}}

{{- if not .Values.api.secretKey }}

######################################
Expand Down
27 changes: 9 additions & 18 deletions charts/flagsmith/templates/_api_environment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,8 @@
name: {{ template "flagsmith.fullname" . }}
key: DJANGO_SECRET_KEY
{{- end }}
{{- if .Values.influxdb2.enabled }}
- name: INFLUXDB_URL
value: http://{{- template "flagsmith.influxdb.hostname" . -}}:80
- name: INFLUXDB_BUCKET
value: {{ .Values.influxdb2.adminUser.bucket }}
- name: INFLUXDB_ORG
value: {{ .Values.influxdb2.adminUser.organization }}
- name: INFLUXDB_TOKEN
valueFrom:
secretKeyRef:
{{- if .Values.influxdb2.adminUser.existingSecret }}
name: {{ .Values.influxdb2.adminUser.existingSecret }}
key: admin-token
{{- else }}
name: {{ template "flagsmith.influxdb.fullname" . }}-auth
key: admin-token
{{- end }}
{{- else if .Values.influxdbExternal.enabled }}
{{- if .Values.api.analytics.enabled }}
{{- if .Values.influxdbExternal.enabled }}
- name: INFLUXDB_URL
value: {{ .Values.influxdbExternal.url | required "Must specify a URL for an external InfluxDB" }}
- name: INFLUXDB_BUCKET
Expand All @@ -52,6 +36,13 @@
name: {{ template "flagsmith.influxdb.fullname" . }}-external-auth
key: admin-token
{{- end }}
{{- else }}
- name: USE_POSTGRES_FOR_ANALYTICS
value: 'true'
{{- end }}
{{- else }}
- name: DISABLE_ANALYTICS_FEATURES
value: 'true'
{{- end }}
- name: DJANGO_ALLOWED_HOSTS
value: '*'
Expand Down
18 changes: 17 additions & 1 deletion charts/flagsmith/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ api:
tag: null # defaults to .Chart.AppVersion
imagePullPolicy: IfNotPresent
imagePullSecrets: []
# Note that if setting this to false, need to set
#
# Note that if `separateApiAndFrontend` is `false`, you'll need to set
# api.image.repository to flagsmith/flagsmith (or some other
# repository hosting the image with combined frontend and backend)
# and that the image tag exists (for flagsmith/flagsmith, >=2.10.0)
Expand All @@ -19,6 +20,21 @@ api:
# (unless explicitly switched off), but both are handled by the api
# deployment's pods.
separateApiAndFrontend: true
#
# If `analytics.enabled` flag is `true`, the following database backend
# will be used for analytics, in the order of preference:
#
# 1. External InfluxDB if `influxdbExternal.enabled` set to `true`
# 2. Dedicated PostgreSQL specified by the `ANALYTICS_DATABASE_URL`
# environment variable
# 3. External PostgreSQL if `databaseExternal.enabled` set to `true`
# 4. Chart-provisioned PostgreSQL
#
# Note that you can use a dedicated PostgreSQL instance by providing the
# value for `ANALYTICS_DATABASE_URL` environment variable under the
# `extraEnvFromSecret` key.
analytics:
enabled: false
replicacount: 1
deploymentStrategy: null
podAnnotations: {}
Expand Down
Loading