Move messaging configuration to file and deprecate deployMessagingService flag by nasark · Pull Request #1041 · ManageIQ/manageiq-pods

nasark · 2024-01-26T20:35:17Z

move messaging config from env vars to file
the configs will be stored in secrets and mounted as a projected volume since the kafka user password is located in another secret created by strimzi
kafka user secret naming convention normalized to match downstream cr.Spec.AppName-user -> cr.Spec.AppName-kafka-admin so that upstream and downstream can both use func MessagingEnvSecret
moving the config to secrets allows us to check the hostname to see whether kafka resources should be deployed or not in the case of an external kafka. This allows us to fully deprecate cr.Spec.DeployMessagingService

Depends on:

Mount messaging configuration on podified manageiq#22859

@miq-bot assign @bdunne
@miq-bot add_reviewer @Fryguy

miq-bot · 2024-01-26T20:36:07Z

@nasark unrecognized command 'enhancement', ignoring...

Accepted commands are: add_label, add_reviewer, request_review, assign, close_issue, cross_repo_test, move_issue, remove_label, rm_label, remove_reviewer, set_milestone, unassign

nasark · 2024-01-26T20:37:47Z

@miq-bot add_label quinteros/yes?

miq-bot · 2024-01-26T20:40:23Z

Checked commits nasark/manageiq-pods@6c880dd~...21cd053 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. ⭐

nasark · 2024-01-26T20:41:08Z

+	hostName := string(secret.Data["hostname"])
+	if hostName != "manageiq-kafka-bootstrap" {


I followed the same methodology we use for postgresql but in this case the hostname also depends on cr.Spec.AppName. Would this be an issue? For example if a user decides to change the default app name for some reason then this wouldn't work, however this allows us to not deploy upstream kafka in the downstream scenario

nasark · 2024-01-26T20:45:55Z

+messaging_hostname=${MESSAGING_HOSTNAME:-$messaging_hostname_file}
+messaging_hostname=${messaging_hostname:-localhost}
+messaging_username=${MESSAGING_USERNAME:-$messaging_username_file}
+messaging_password=${MESSAGING_PASSWORD:-$messaging_password_file}
+messaging_port=${MESSAGING_PORT:-$messaging_port_file}
+messaging_port=${messaging_port:-9093}
+messaging_sasl_mechanism=${MESSAGING_SASL_MECHANISM:-$messaging_sasl_mechanism_file}


Since the env vars won't be used, should they be removed from these conditional assignments? I see that the database env vars are still checked in these conditional assignments https://github.com/ManageIQ/manageiq-pods/blob/master/images/manageiq-base/container-assets/container_env#L12-L20 so I followed suite but didn't understand the reasoning

We kept them more as a convenience for developers - if you run it locally, it's just a lot easier to pass them as -e on the command line

Fryguy · 2024-01-26T21:13:18Z

+[[ -f /run/secrets/messaging/MESSAGING_SASL_MECHANISM ]] && messaging_sasl_mechanism_file=$(cat /run/secrets/messaging/MESSAGING_SASL_MECHANISM)
+[[ -f /etc/pki/ca-trust/source/anchors/root.crt ]] && messaging_ca_path=/etc/pki/ca-trust/source/anchors/root.crt
+messaging_hostname=${MESSAGING_HOSTNAME:-$messaging_hostname_file}
+messaging_hostname=${messaging_hostname:-localhost}


Is this right? I thought localhost didn't work?

It should probably default to the name of the service used to reach kafka

miq-bot · 2024-06-10T00:00:23Z