Optimize nginx vhost generation #863
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request refactors the Ansible roles for Nginx virtual host configuration by removing redundant tasks and consolidating the configuration into templates. The key changes include the removal of individual fragment tasks, the creation of comprehensive Jinja2 templates, and the introduction of macros to streamline the configuration process.
Refactoring and Consolidation
ansible/roles/nginx_vhost/tasks/defaulthost.yml: Removed multiple tasks related to creating and managing vhost fragments and consolidated them into a single task that uses the newnginx_vhost_default.j2template. [1] [2]ansible/roles/nginx_vhost/tasks/main.yml: Removed tasks for creating and managing various vhost fragments and replaced them with a single task using thenginx_vhost.j2template. [1] [2]Template Creation
ansible/roles/nginx_vhost/templates/nginx_vhost.j2: Created a new template that imports macros and includes logic for both HTTP and HTTPS configurations based on the provided variables.ansible/roles/nginx_vhost/templates/nginx_vhost_default.j2: Added a template specifically for the default vhost configuration, incorporating the necessary fragments through macros.Macro Introduction
ansible/roles/nginx_vhost/templates/nginx_vhost_macros.j2: Introduced macros for each fragment type to be included in the main templates, simplifying the inclusion and maintenance of configuration fragments.Performance Impact
The refactored approach reduced the Nginx vhost generation time for biocache-service from 137.24 seconds (249 in a second run) to 27.41 seconds addressing the slowest tasks of ala-install (see #693).
Tested
This is tested with several roles with different
subdomain-a.l-a.site,subdomain-b.l-a.siteetc, using SSL and not using SSL. The nginx vhost generated (and the default host conf) are equals to the ones generated without this PR (with minimal indentation differences).Not supported yet
Sites using a
samedomain.l-a.site/app-1andsamedomain.l-a.site/app-2, etc. in the same server. I'm not sure if this is used by ALA. If we still need this, this PR needs more work to support it.