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

Nginx: default_server fact wins over inventory configuration #247

Open
gaudenz opened this issue Mar 21, 2018 · 1 comment
Open

Nginx: default_server fact wins over inventory configuration #247

gaudenz opened this issue Mar 21, 2018 · 1 comment

Comments

@gaudenz
Copy link
Contributor

gaudenz commented Mar 21, 2018

Currently the default server which gets saved to a fact during the first run has precedence over the inventory configuration with the nginx_default_name and nginx_default_ssl_name variables. This leads to surprising results when setting or changing these variables as these changes do not have any effect. The same issue also exists when changing the order in the nginx__servers list which would have an effect on the selection on the first run, but not on subsequent runs.

IMO the Ansible inventory should always be the definitive source of configuration and configuration should not be different whether this is the first run or not. The default server selection process ("Spin the default_server roulette!") is deterministic and does not have any random element (not really a "roulette").

I suggest to completely remove this fact and to select the default server only based on the nginx_default_name variable and if this is absent, choose the first server. If changing the default_server depending on the order of the server list is deemed too surprising (I don't think it is), I suggest select in the following order:

  1. nginx_default_name
  2. default server fact
  3. first in server list

I'm opening this as an issue as I saw that this was done on purpose in b8f6e67. If you agree I can create a pull request to implement this.

@drybjed
Copy link
Member

drybjed commented Mar 21, 2018

I agree, the current mechanism of default server selection is very badly designed, and there are multiple factors at play here.

The debops.nginx local facts where designed long ago when most of the facts were static. Today, the facts are usually Python scripts which can be more dynamic. The debops.nginx fact needs to be rewritten in Python, in which case it could help select the default nginx server by scanning the existing configuration and returning the selection to the Ansible Controller for consideration.

Remember that setting default_server also sets ipv6only=off option, and there can be only one instance of that option per listening port. Another important fact is that the debops.nginx role can be used in multiple playbooks, each one having a different list of servers during a given run. That's why the current selection is saved in the Ansible local facts and is supposed to be static - if you change the default server from run to run, and don't remove ipv6only=off from the configuration files that aren't managed at a given moment (because they were managed by a different role in different playbook), you will end up with broken nginx server. It's tricky and I think that updated debops.nginx fact script would be a first step to resolve this problem.

Due to above issue I don't think that setting a default explicitly in the inventory is always a good idea. What if during a particular run the server selected as the default through the inventory is not in the list of the servers the role knows about? Should it pick another server and set is as default (broken nginx)? Should it not pick any server (broken nginx on the first run)?

So as I said, if you have ideas to resolve these issues, go ahead and make a PR. It will be definitely discussed at length. :-)

@drybjed drybjed added this to To do in Webserver infrastructure via automation Mar 21, 2018
@drybjed drybjed added this to the DebOps v1.0.0 milestone Mar 21, 2018
@drybjed drybjed modified the milestones: DebOps v1.0.0, DebOps v1.2.0 Aug 6, 2019
@drybjed drybjed modified the milestones: DebOps v1.2.0, DebOps v2.0.0 Dec 1, 2019
@drybjed drybjed added bug something is not working as expected priority: high tag: nginx tag: webserver and removed tag: enhancement labels Feb 11, 2020
@drybjed drybjed modified the milestones: DebOps v2.2.0, DebOps v3.2.0 Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is not working as expected priority: high tag: nginx tag: webserver
Projects
Development

No branches or pull requests

2 participants