Skip to content

Feature 15: Add RabbitMQ to helm chart, add consumer to helm chart, establish queue#17

Open
RWShelton wants to merge 13 commits into
developfrom
feature-15-create-queue-helm
Open

Feature 15: Add RabbitMQ to helm chart, add consumer to helm chart, establish queue#17
RWShelton wants to merge 13 commits into
developfrom
feature-15-create-queue-helm

Conversation

@RWShelton

Copy link
Copy Markdown

What

This PR adds both a rabbitmq instance to the helm chart as well as the consumer pod. The consumer currently creates the queue on startup and reads messages, but doesn't perform any operations with them. Failed messages are logged, but not requeued because we currently don't have a dead message queue or any other mechanism to handle them.

How

The chart makes use of the rabbitmq operator to create the queue pod at startup. It get parameters (vhost, queue name, etc) from a new rmq section of the values file, and the connection details to the queue that the consumer uses are in a subsection of that rmq section called rmqconnection. The consumer itself has its own section, nsconsumer, that configures resources, probes, autoscaling, and other details.

These details are injected into the pods via a configmap property file in the config folder of the helm chart. The only two remaining parts that are not managed by that file are the rabbitmq username and password, which come from a secret that rmq generates automatically on startup. The credentials in that secret are read in as environment variables on the consumer pod. This is the default behavior, but if the secret is not created and those environment variables are null as a result, the connection will look at the properties file instead.

I also added an initcontainer to the consumer that waits for the queue to come up to avoid trying and failing to connect, or pulling null credentials before the secret is created.

Why

The queue will manage messages from Metacat instances so that the consumer can deliver them to the controller/email servers.

Testing

I deployed the chart via rancher desktop, and tested the consumer with both valid and invalid messages, verifying that bad messages are logged and valid messages are processed (currently just logging them as successful, next step is to send them to the controller).

RWShelton added 5 commits May 4, 2026 16:10
I had to move the notes text file to get the chart to install, still needs to be moved back after it's fixed.
Adjusting the configmap and the environment variables to pull rmq secret directly. For some reason the consumer is closing without erroring immediately when the connectionfactory is opened, without actually trying to connect. Still troubleshooting. There's a bunch of log statements in here that I'll remove before the PR.
Adding a queuedeclare call when the channel is created, automatically creating the correct queue based on provided params when the chart is installed. A note: it doesn't appear we can create vhosts on the fly, so I've reset all the values and properties files to the default / vhost.
@RWShelton RWShelton requested a review from artntek June 9, 2026 18:48
@RWShelton

Copy link
Copy Markdown
Author

One issue I've found that I'm aware of with this change is sometimes on upgrade, the consumer starts a new pod without shutting down the old one. Wanted to get some eyes on this stuff while I work on that.

@artntek artntek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

General Note: See our Java Coding Guidelines - all lines should be <= 100 chars in length. I flagged one that was longer, but there may be others - so please reformat accordingly before merging. Thanks!

Comment thread ns-common/src/main/resources/log4j2.yaml Outdated
Comment thread ns-common/src/main/java/org/dataone/notifications/NsConfig.java Outdated
Comment thread ns-common/src/main/java/org/dataone/notifications/NsConfig.java Outdated
Comment thread ns-common/src/main/java/org/dataone/notifications/NsConfig.java Outdated
Comment thread helm/templates/deployment.yaml Outdated
Comment thread ns-common/src/main/resources/properties.yaml Outdated
Comment thread helm/values.yaml Outdated
Comment thread helm/values.yaml Outdated
Comment thread helm/values.yaml Outdated
create: false
name: ""

autoscaling:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

FYI, this won't do anything on our clusters, siince we dont' yet support autoscaling. Probably better to remove it for simplicity?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. There's another autoscaling section in the root values section, I see it's commented that it's currently ignored, do you want me to leave that one there?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't mind either way, and simpler is better, so maybe delete?

Comment thread helm/templates/deployment-consumer.yaml Outdated
…ride

I've been building the images locally for testing, so I put my image override in the rancher file. Can be replaced with the remote one if someone wanted to use that. Also confirmed that the pod will still pull the correct credentials even if the queue hasn't finished loading yet, so we don't need the initcontainer.
…at's the deployment template that uses them.

@artntek artntek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment thread helm/values.yaml
timeoutSeconds: 5

# @param extraEnvVars Optional additional env vars to be added to the container.
# Format: { EXAMPLE_ENV_VAR: "example_value", ANOTHER_ENV_VAR: "another_value" }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

could you show a yaml example? e.g. is it one of these?:

extraEnvVars:
  - EXAMPLE_ENV_VAR: "example_value"
  - ANOTHER_ENV_VAR: "another_value"

## or:
extraEnvVars:
  EXAMPLE_ENV_VAR: "example_value"
  ANOTHER_ENV_VAR: "another_value"

## or literally:
extraEnvVars: EXAMPLE_ENV_VAR: "example_value", ANOTHER_ENV_VAR: "another_value"

Comment thread helm/values.yaml Outdated
create: false
name: ""

autoscaling:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't mind either way, and simpler is better, so maybe delete?

* Quick check whether an active connection exists.
*/
public synchronized boolean isConnected() {
log.info("Checking connection status: " +

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think info is too verbose for a health-check - would fill logs. Can we make it debug?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants