Skip to content

Conversation

@gionn
Copy link
Member

@gionn gionn commented May 8, 2025

@gionn gionn self-assigned this May 8, 2025
@pmacius pmacius force-pushed the OPSEXP-3235-db-common branch from 6cb9024 to e7a1695 Compare May 16, 2025 12:27
@pmacius pmacius added the ec2-test Triggers ec2 integrations tests label May 16, 2025
@pmacius pmacius requested a review from Copilot May 16, 2025 14:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors database configuration variables by extracting common DB connection settings from individual roles into the main playbook and central variable definitions. Key changes include:

  • Removal of inline DB URL fallback logic in favor of globally computed DB URL variables.
  • Introduction of new variables (repository_db_host, repository_db_port, ports_cfg_postgres_sql) and associated argument specs for more flexible configuration.
  • Updates across roles and playbooks to consistently reference the computed DB URLs and updated naming for search enterprise settings.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
roles/sync/tasks/configure.yml Removed fallback formatting in sync_db_url in favor of an externally computed variable.
roles/sync/molecule/default/host_vars/syncservice-instance.yml Introduced sync_db_url with a simple jdbc URL format.
roles/search_enterprise/templates/elasticsearch-connector-reindex.service.j2 Updated datasource password and URL references to use the new search enterprise variables.
roles/search_enterprise/meta/argument_specs.yml Renamed and updated descriptions for search enterprise DB connection variables.
roles/search_enterprise/defaults/main.yml Added default for search_enterprise_repo_db_password while removing the old DB name variable.
roles/repository/tasks/main.yml Switched wait_for task to use repository_db_host and repository_db_port.
roles/repository/meta/argument_specs.yml Added repository_db_host and repository_db_port specs.
roles/repository/defaults/main.yml Added repository_db_host and repository_db_port default values.
roles/common/defaults/main.yml Removed common definitions for db_host and ports_cfg.postgres.sql.
playbooks/prerun-network-checks.yml Updated port checking to reference the new ports_cfg_postgres_sql variable.
playbooks/group_vars/repository.yml Changed repository DB URL to use the computed acs_play_computed_repo_db_url.
playbooks/group_vars/all.yml Defined ports_cfg_postgres_sql as a global variable.
playbooks/facts.yml Added set_fact tasks to compute db_host and derive the computed repository and sync DB URLs.
playbooks/acs.yml Updated tasks to use the computed DB URL variables and adjusted repository credentials.
.secrets.baseline Updated generated timestamp.
Comments suppressed due to low confidence (10)

roles/search_enterprise/meta/argument_specs.yml:26

  • [nitpick] The updated parameter descriptions and variable names (e.g., search_enterprise_repo_db_username and search_enterprise_repo_db_password) differ from the previous naming; please confirm that these new names accurately reflect their intended usage.
URL of the database to use for the Search Enterprise Bulk Ingester

roles/search_enterprise/defaults/main.yml:19

  • [nitpick] The addition of search_enterprise_repo_db_password (and removal of search_enterprise_repo_db_name) should be checked for naming consistency with other DB configuration variables.
search_enterprise_repo_db_password: ""

roles/common/defaults/main.yml:40

  • Removal of these common definitions may have wider implications; please ensure that any dependent roles have alternative definitions available.
Removed db_host and ports_cfg.postgres.sql

playbooks/facts.yml:25

  • Verify that the computed repository DB URL correctly prioritizes acs_play_repo_db_url, and that db_host and ports_cfg_postgres_sql are always available when acs_play_repo_db_url is not set.
acs_play_computed_repo_db_url: >-  {{ acs_play_repo_db_url if acs_play_repo_db_url else 'jdbc:postgresql://' + db_host + ':' + ports_cfg_postgres_sql | string + '/' + acs_play_repo_db_name }}

roles/sync/tasks/configure.yml:10

  • Removing the inline fallback logic may break functionality if sync_db_url is not provided; please ensure that an appropriate computed value is always set externally.
{{ sync_db_url }}

roles/search_enterprise/templates/elasticsearch-connector-reindex.service.j2:14

  • Verify that changing the datasource password variable from repo_db_password to search_enterprise_repo_db_password aligns with the intended configuration across environments.
Environment=SPRING_DATASOURCE_PASSWORD={{ search_enterprise_repo_db_password | replace('%', '%%') }}

roles/repository/tasks/main.yml:519

  • The new usage of repository_db_host (and repository_db_port) in the wait_for task requires confirmation that these variables are always set when needed.
host: "{{ repository_db_host }}"

playbooks/prerun-network-checks.yml:24

  • Ensure that the new variable ports_cfg_postgres_sql is consistently defined and used across all playbooks where a database port is required.
checked_port: "{{ ports_cfg_postgres_sql }}"

playbooks/acs.yml:337

  • Switching to the computed repository DB URL is fine as long as all roles consuming repository_db_url are aware of this change.
repository_db_url: "{{ acs_play_computed_repo_db_url }}"

playbooks/acs.yml:542

  • Ensure that using repo_db_password for search enterprise configuration matches the intended credential source for the bulk ingester setup.
search_enterprise_repo_db_password: "{{ repo_db_password }}"

@pmacius pmacius marked this pull request as ready for review May 16, 2025 15:50
@pmacius pmacius requested review from a team, alxgomz and pmacius May 16, 2025 15:50
@pmacius pmacius requested a review from alxgomz May 21, 2025 14:10
@pmacius pmacius force-pushed the OPSEXP-3235-db-common branch from f245b41 to 5f67a31 Compare May 23, 2025 15:12
@pmacius pmacius merged commit 7f46a6d into master May 23, 2025
69 checks passed
@pmacius pmacius deleted the OPSEXP-3235-db-common branch May 23, 2025 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ec2-test Triggers ec2 integrations tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants