Skip to content

fix(RHDHBUGS-2015): fix sonataflow DB ref name for replication architecture of PostgreSQL - #235

Merged
openshift-merge-bot[bot] merged 2 commits into
redhat-developer:mainfrom
pmacik:fix/RHDHBUGS-2015
Sep 9, 2025
Merged

fix(RHDHBUGS-2015): fix sonataflow DB ref name for replication architecture of PostgreSQL#235
openshift-merge-bot[bot] merged 2 commits into
redhat-developer:mainfrom
pmacik:fix/RHDHBUGS-2015

Conversation

@pmacik

@pmacik pmacik commented Sep 9, 2025

Copy link
Copy Markdown

Description of the change

This PR changes RHDH DB ref name (adds -primary suffix) if the PostgreSQL architecture is set to replication to match the name of the DB service.

Which issue(s) does this PR fix or relate to

How to test changes / Special notes to the reviewer

Set the upstream.postgresql.architecture=replication and upstream.postgresql.replication.enabled=true and other related Helm chart values and then enable Orchestrator plugin (orchestrator.enabled=true) - install Helm chart.

Checklist

  • For each Chart updated, version bumped in the corresponding Chart.yaml according to Semantic Versioning.
  • For each Chart updated, variables are documented in the values.yaml and added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Use pre-commit run -a to apply changes. The pre-commit Workflow will do this automatically for you if needed.
  • JSON Schema template updated and re-generated the raw schema via the pre-commit hook.
  • Tests pass using the Chart Testing tool and the ct lint command.
  • If you updated the orchestrator-infra chart, make sure the versions of the Knative CRDs are aligned with the versions of the CRDs installed by the OpenShift Serverless operators declared in the values.yaml file. See Installing Knative Eventing and Knative Serving CRDs for more details.

Summary by Sourcery

Fix SonataFlow Helm chart to reference the primary PostgreSQL instance when replication is enabled and bump the chart version

Bug Fixes:

  • Ensure serviceRef name, dbHost, and database creation commands append '-primary' suffix when PostgreSQL replication is enabled

Chores:

  • Bump charts/backstage version to 4.5.7
  • Update README version badge to 4.5.7

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@rm3l

rm3l commented Sep 9, 2025

Copy link
Copy Markdown
Member

/cc @elai-shalev

@openshift-ci

openshift-ci Bot commented Sep 9, 2025

Copy link
Copy Markdown

@rm3l: GitHub didn't allow me to request PR reviews from the following users: elai-shalev.

Note that only redhat-developer members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @elai-shalev

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

passwordKey: password
serviceRef:
name: {{ .Release.Name }}-postgresql
name: {{ .Release.Name }}-postgresql{{- if eq .Values.upstream.postgresql.architecture "replication" }}-primary{{- end }}

@rm3l rm3l Sep 9, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This works with the default settings, but I can see here that the -primary suffix depends on the primary.name value.
And indeed, the RHDH pod crashes if I deploy with --set upstream.postgresql.architecture=replication --set upstream.postgresql.primary.name=my-prim.
That said, I guess we would still have an issue as the same hardcoded suffix is used in the upstream Backstage chart: https://github.com/backstage/charts/blob/main/charts/backstage/templates/_helpers.tpl#L39C50-L39C58
So fine for now with the default settings, but I think this should be reported and fixed in the upstream Backstage chart so that users can use a different primary name.

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.

Good point, thx.

Comment thread charts/backstage/Chart.yaml Outdated
…ecture of PostgreSQL

Signed-off-by: Pavel Macík <pavel.macik@gmail.com>
Co-authored-by: pmacik <pmacik@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 9, 2025

Copy link
Copy Markdown

⚠️ Files changed after running the pre-commit hooks

Those changes should have been pushed automatically to your PR branch.

NOTE: If the PR checks are stuck after this additional commit, manually close the PR and immediately reopen it to trigger the checks again.

@rm3l rm3l closed this Sep 9, 2025
@rm3l rm3l reopened this Sep 9, 2025
@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2025

Copy link
Copy Markdown

@rm3l rm3l left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Sep 9, 2025
@openshift-merge-bot
openshift-merge-bot Bot merged commit c6b7328 into redhat-developer:main Sep 9, 2025
8 checks passed
@rm3l

rm3l commented Sep 10, 2025

Copy link
Copy Markdown
Member

Cherry-picking this to 1.7, as requested in this comment.

/cherry-pick release-1.7

@openshift-cherrypick-robot

Copy link
Copy Markdown

@rm3l: #235 failed to apply on top of branch "release-1.7":

Applying: fix(RHDHBUGS-2015): fix sonataflow DB ref name for replication architecture of PostgreSQL
Using index info to reconstruct a base tree...
M	charts/backstage/Chart.yaml
Falling back to patching base and 3-way merge...
Auto-merging charts/backstage/Chart.yaml
CONFLICT (content): Merge conflict in charts/backstage/Chart.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 fix(RHDHBUGS-2015): fix sonataflow DB ref name for replication architecture of PostgreSQL

Details

In response to this:

Cherry-picking this to 1.7, as requested in this comment.

/cherry-pick release-1.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

rm3l pushed a commit to rm3l/rhdh-chart that referenced this pull request Sep 10, 2025
…ecture of PostgreSQL (redhat-developer#235)

* fix(RHDHBUGS-2015): fix sonataflow DB ref name for replication architecture of PostgreSQL

Signed-off-by: Pavel Macík <pavel.macik@gmail.com>

* chore(pre-commit): Auto-fix hooks

Co-authored-by: pmacik <pmacik@users.noreply.github.com>

---------

Signed-off-by: Pavel Macík <pavel.macik@gmail.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: pmacik <pmacik@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants