Add 'disable_tls' and 'dev_deployment' modifications #282

pvannierop · 2024-07-01T19:28:59Z

For local development I introduce the following modifications to the default RADAR-base deployment:

Disable TLS: this removes the TLS-redirection by the nginx reverse proxy. Also, prevents installation of the cert-manager and lets-encrypt cert resolver components
Disable monitoring and logging: prevents installation of the prometheus and monitoring/logging sidecar containers and graylog components.
Local development: Sets the number of brokers of all components to 1 (kafka, minio). Minimizes the JVM memory requirements of memory heavy components. Sets the server name to localhost. Disables atomicInstall.

keyvaann · 2024-07-11T08:07:53Z

environments.yaml.tmpl

+   - ../etc/_mods/disable_tls.yaml
+   {{ end }}
+   {{ if .Values.dev_deployment }}
+   - ../etc/_mods/disable_tls.yaml


As the number of helmfile related configs have increased I suggest moving all of them to a different root directory, it will make things more clear and more easy to manage.

I moved the files to a new directory mods in the root.

keyvaann · 2024-07-11T08:14:05Z

etc/_mods/local_development.yaml

+  configurationOverrides:
+    "offsets.topic.replication.factor": 1
+cp_zookeeper:
+  servers: 1


Can we move the configs that make the apps run only one replica to the base.yaml? As we try to have a minimal working config in the default configuration I think it's good to have them there.

I would rather not do that in this PR. I would need to very carefully inspect the consequences because we change default behavior. Would be good to create a new issue for this.

keyvaann · 2024-07-11T08:17:50Z

etc/_mods/disable_logging_and_monitoring.yaml

+cert_manager:
+  prometheus:
+    servicemonitor:
+      enabled: true


Shouldn't this be false?

I guess you are right.... Chnaged

keyvaann · 2024-07-11T08:20:06Z

etc/_mods/disable_logging_and_monitoring.yaml

+    sidecars: []
+radar_grafana:
+  _install: false
+radar_jdbc_connector:


Aren't these part of RADAR-Base apps? Why are they being disabled?

Ah yes, I mistook it for graylog... updated.

keyvaann · 2024-07-11T08:29:39Z

environments.yaml.tmpl

+   {{ end }}
+   {{ if .Values.dev_deployment }}
+   - ../etc/_mods/disable_tls.yaml
+   - ../etc/_mods/disable_logging_and_monitoring.yaml


I think there should be a separate flag for disabling logging and monitoring and it should be true since it doesn't need to be enabled for a simple default installation.

I disabled logging and monitoring in the default deployment. We need to be careful here, because this change is not backwards compatible. We should provide a migration note here?

Yes it's good to mention in release notes to manually enable these components during upgrade.

I also created a separate flag to disable monitoring and logging.

keyvaann · 2024-07-11T12:23:40Z

Closes #12

keyvaann · 2024-07-18T19:19:21Z

environments.yaml.tmpl

-   - ../etc/secrets.yaml
+  default:
+    values:
+      - ../etc/base.yaml


Do you think it would be good to move these files helmfile related files to a separate directory as well? Probably something like helmfile_configs to put all of it's configuration related files in it.

I think this would make sense yes. Should we do this now? We have to make sure all the scripts in the /bin folder play nice with this change.

It would be probably better in the long run make things more organized but we can do it later if it is easier.

keyvaann · 2024-07-18T19:20:12Z

environments.yaml.tmpl

+      - ../etc/production.yaml.gotmpl
+      - ../etc/secrets.yaml
+    {{ if not .Values.enable_tls }}
+      - ../mods/disable_tls.yaml


To reduce confusion I think these 2 lines should either be enable_tls or disable_tls

It is a matter of API design, I guess. The config file disables tls and is, therefore, correctly named. What are the config options that we would like to present:

A:

# Enable logging and monitoring enable_logging_monitoring: false # Enable TLS redirection and retrieval of Let's Encrypt certificates. # Can be disabled when TLS termination is handled upstream of the on-cluster Nginx reverse proxy. enable_tls: true

B:

# Disable logging and monitoring disable_logging_monitoring: true # Disable TLS redirection and retrieval of Let's Encrypt certificates. # Can be disabled when TLS termination is handled upstream of the on-cluster Nginx reverse proxy. disable_tls: false

I am fine either way.

If there is a /mods/disable_tls.yaml and I think the variable should also be disable_tls

pvannierop self-assigned this Jul 1, 2024

pvannierop requested a review from keyvaann July 1, 2024 19:29

pvannierop changed the base branch from main to dev July 1, 2024 19:29

pvannierop force-pushed the local-k3d-dev branch 3 times, most recently from cb67401 to 946ac46 Compare July 2, 2024 05:09

pvannierop changed the title ~~Add 'disable_tls' and 'minimal_deployment' modifications~~ Add 'disable_tls' and 'dev_deployment' modifications Jul 2, 2024

This was referenced Jul 4, 2024

[GitHub Action] Make sure CI tests cover all components #284

Merged

Fix nesting 00 init #273

Closed

keyvaann reviewed Jul 11, 2024

View reviewed changes

pvannierop force-pushed the local-k3d-dev branch 3 times, most recently from 77fa8c8 to 2f4c0c7 Compare July 18, 2024 12:38

keyvaann reviewed Jul 18, 2024

View reviewed changes

pvannierop force-pushed the local-k3d-dev branch from 2f4c0c7 to 7676cc2 Compare July 19, 2024 07:26

pvannierop force-pushed the local-k3d-dev branch from fc9415e to b98490e Compare August 1, 2024 15:03

pvannierop added 2 commits August 2, 2024 08:55

Up various chart versions

c15ffe9

Use added 'disable_tls' option for new charts

a943bdf

pvannierop force-pushed the local-k3d-dev branch from b98490e to 5776a12 Compare August 2, 2024 07:25

Add helmfile modifications for local development and testing

1be83bc

pvannierop force-pushed the local-k3d-dev branch from 5776a12 to 1be83bc Compare August 2, 2024 07:29

pvannierop closed this Aug 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 'disable_tls' and 'dev_deployment' modifications #282

Add 'disable_tls' and 'dev_deployment' modifications #282

pvannierop commented Jul 1, 2024 •

edited

Loading

keyvaann Jul 11, 2024

pvannierop Jul 18, 2024

keyvaann Jul 11, 2024 •

edited

Loading

pvannierop Jul 18, 2024

keyvaann Jul 11, 2024

pvannierop Jul 18, 2024

keyvaann Jul 11, 2024

pvannierop Jul 18, 2024

keyvaann Jul 11, 2024 •

edited

Loading

pvannierop Jul 18, 2024

keyvaann Jul 18, 2024

pvannierop Jul 19, 2024

keyvaann commented Jul 11, 2024

keyvaann Jul 18, 2024

pvannierop Jul 19, 2024

keyvaann Jul 25, 2024

keyvaann Jul 18, 2024

pvannierop Jul 19, 2024 •

edited

Loading

keyvaann Jul 25, 2024

Add 'disable_tls' and 'dev_deployment' modifications #282

Add 'disable_tls' and 'dev_deployment' modifications #282

Conversation

pvannierop commented Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keyvaann Jul 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keyvaann Jul 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keyvaann commented Jul 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvannierop Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvannierop commented Jul 1, 2024 •

edited

Loading

keyvaann Jul 11, 2024 •

edited

Loading

keyvaann Jul 11, 2024 •

edited

Loading

pvannierop Jul 19, 2024 •

edited

Loading