Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ansible/artifactory/artifactory_nginx/postgres] Refactoring #398

Closed
wants to merge 2 commits into from
Closed

[ansible/artifactory/artifactory_nginx/postgres] Refactoring #398

wants to merge 2 commits into from

Conversation

EmptyByte
Copy link

PR Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Title of the PR starts with installer/product name (e.g. [ansible/artifactory])
  • CHANGELOG.md updated
  • Variables and other changes are documented in the README.md

What this PR does / why we need it:
This PR makes the roles more maintainable by including tasks rather than duplicating them. Support for RHEL9 was added and RHEL7 was dropped (EOL June 30). artifactory_nginx_ssl role is merged in nginx. Some bugs fixed (uid/guid were never used; xray role changed the permissions of all directories under /opt/jfrog instead of /opt/jfrog/xray so for aio it was failing; crontab was disabled by default but artifactory was complaining). Creating repositories for NGINX and PostgreSQL is now optional and not enabled by default has to be enabled with: artifactory_nginx_use_official_repos and postgresql_use_official_repos. PGSQL HBA entries can be dynamically updated using postgresql_hba_add_inventory_hosts. Tasks were streamlined and other fixes included to avoid issues. Platform deployment succeeds on an AIO but will ultimately fail because each service tries to use the same ports. So the serviceyaml variable will require some tuning.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #
N/A

Special notes for your reviewer:
Linted and tested on RHEL9

@vasukinjfrog
Copy link
Collaborator

Thank you for submitting, there are many changes under this one PR, can each type/category of change be submitted as a separate PR, it promotes for better review opportunities and limits the scope. Hope this can be resubmitted with the change requested.

@EmptyByte
Copy link
Author

@vasukinjfrog Hello, indeed this PR is quite big. I will split each role in a single PR as soon as I have the opportunity.

@vasukinjfrog
Copy link
Collaborator

vasukinjfrog commented Jul 5, 2024

@EmptyByte thank you for considering the suggestion. We will await your submissions to get separate PRs based on the role. Until then, we will have this PR action paused.

@EmptyByte EmptyByte closed this by deleting the head repository Jul 7, 2024
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