Skip to content

Conversation

@DrFaust92
Copy link
Contributor

What this PR does / why we need it

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

  • fixes #

Special notes for your reviewer

Checklist

  • DCO signed
  • Chart Version bumped
  • Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

Copy link
Contributor

@zeritti zeritti left a comment

Choose a reason for hiding this comment

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

The other thing with respect to Bitnami's images in this chart is the postgresql subchart (dependency) and the corresponding image which this PR does not address.

My suggestion would be to remove the dependency and the extra container from the CI configuration entirely. This would cause no problems in CI as PgBouncer exporter does not require PgBouncer to start successfully. In fact, the CI jobs do not validate nor test whether the exporter functions as expected - launching the target service in CI seems in this sense and in this case unnecessary.

@DrFaust92
Copy link
Contributor Author

zeritti let me try and if so ill take a look doing it more globally with other such deps (kafka, etc)

@DrFaust92 DrFaust92 changed the title [prometheus-pgbouncer-exporter] enable pgsql dep only in CI + move refs to bitnami legacy [prometheus-pgbouncer-exporter] remove bitnami deps and simplify CI deps Aug 23, 2025
@DrFaust92 DrFaust92 requested review from jkroepke and zeritti August 23, 2025 12:24
@DrFaust92
Copy link
Contributor Author

Thanks @zeritti should be even cleaner now

@jkroepke jkroepke removed their request for review August 23, 2025 12:58
Copy link
Contributor

@zeritti zeritti left a comment

Choose a reason for hiding this comment

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

PgBouncer does not start without a PostgreSQL instance to connect to leading to a failure in the CI test case. I have therefore removed the extra container hoping it is ok with you, @DrFaust92. Cheers!

@zeritti zeritti merged commit abab9d5 into prometheus-community:main Oct 9, 2025
6 checks passed
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.

3 participants