search

saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.11k stars 5.47k forks source link

[BUG] Conflict between minion and syndic configuration #65658

Open Guillaume-COEUGNET opened 10 months ago

Guillaume-COEUGNET commented 10 months ago

Description Hi. I wonder if there's existing a conflict between Minion config and Syndic config.

We use a multi-MoM with a multi-syndic configuration, to ensure high availability, under 3006.3 salt version. Our syndic servers, also have a minion on them.

To do so, I parametered two MoMs in the "syndic_master" directive of the syndic config file and two masters in the "master" directive of the minion config file. Minion is allowed to use random_master to split load accross the two syndics.

I wonder why I'm getting this warning in salt-syndic log file :

[salt.minion :780 ][WARNING ][295318] random_master is True but there is only one master specified".

The process who raise this warning is salt-minion. I guess this is because "A Syndic node can be thought of as a special passthrough Minion node" regarding the doc. There is no such warning in the minion logfile.

Why salt-syndic seems to use my minion "random_master" configuration ? Thanks a lot.

Setup


~]# cat /etc/salt/master.d/syndic.conf
syndic_master:
  - saltmaster1.it.fr
  - saltmaster2.it.fr

random_master: False                                          # I guess this one is not even used by syndic
syndic_log_file: /var/log/salt/syndic
syndic_pidfile: /var/run/salt-syndic.pid

gitfs_global_lock: False
git_pillar_global_lock: False
top_file_merging_strategy: same

tcp_keepalive: True
tcp_keepalive_intvl: 150
tcp_keepalive_idle: 150

log_level: warning
file_recv: True

worker_threads: 5
~]# cat /etc/salt/minion.d/minion.conf
master:
  - saltsyndic1.it.fr
  - saltsyndic2.it.fr

master_type: failover
master_failback: True
master_failback_interval: 120
random_master: True                                       # This one is used for the minion on the syndic server, not for the syndic itself
master_alive_interval: 60
master_tries: -1
retry_dns: 0

log_level: warning
pillarenv_from_saltenv: True
saltenv: horsprod
state_verbose: False

# Keep connexion on port 4505 alive
tcp_keepalive: True
tcp_keepalive_intvl: 150
tcp_keepalive_idle: 150

# Keep connexion on port 4506 alive
ping_interval: 10

# dispatch logins
random_startup_delay: 30
random_reauth_delay: 60
acceptance_wait_time: 10
auth_timeout: 40

use_superseded:
  - module.run

# Optional
# recon_default: 300000
# recon_max: 350000
# recon_randomize: True

Please be as specific as possible and give set-up details.

Steps to Reproduce the behavior Use previous config, restart salt-syndic service and take a look at syndic's logfile that will displays :

2023-12-01 11:01:25,042 [salt.minion :780 ][WARNING ][295318] random_master is True but there is only one master specified. Ignoring.

Expected behavior This warning is not supposed to appear because syndic should not take a look at random_master config option. Syndic have to initiate a connection with it's hot MoM to ensure high availability. Isn't it ?

Screenshots If applicable, add screenshots to help explain your problem.

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) ```yaml Salt Version: Salt: 3006.3 Python Version: Python: 3.10.13 (main, Sep 6 2023, 02:11:27) [GCC 11.2.0] Dependency Versions: cffi: 1.14.6 cherrypy: unknown dateutil: 2.8.1 docker-py: Not Installed gitdb: Not Installed gitpython: Not Installed Jinja2: 3.1.2 libgit2: 1.7.1 looseversion: 1.0.2 M2Crypto: Not Installed Mako: Not Installed msgpack: 1.0.2 msgpack-pure: Not Installed mysql-python: Not Installed packaging: 22.0 pycparser: 2.21 pycrypto: Not Installed pycryptodome: 3.9.8 pygit2: 1.13.0 python-gnupg: 0.4.8 PyYAML: 6.0.1 PyZMQ: 23.2.0 relenv: 0.13.10 smmap: Not Installed timelib: 0.2.4 Tornado: 4.5.3 ZMQ: 4.3.4 System Versions: dist: ubuntu 20.04.2 focal locale: utf-8 machine: x86_64 release: 5.4.0-167-generic system: Linux version: Ubuntu 20.04.2 focal ```

Additional context Add any other context about the problem here.

welcome[bot] commented 10 months ago

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

markschuh commented 5 months ago

Both salt-syndic and salt-minion share the code of a MinionBase. As the salt-syndic shall also read a few settings from the salt-master, the salt-syndic read first all salt-master options (or defaults). Afterwards it reads all salt-minion options. Even defaults of the latter will overwrite same salt-master options, I assume. On top of this some few special option handling is done, e.g. the masters are overwritten by the syndic_masters. See https://github.com/saltstack/salt/blob/483ca17006cfa9fd5615f62d09d6508c53fc741f/salt/config/__init__.py#L2434

"be a minion reporting to a master - a syndic_master in this case" is the core functionality of the salt-syndic and this code and option sharing was used for efficiency, when the salt-syndic was introduced long ago. But while the salt-minion code had gotten extension here and there in the last 10 years, the salt-syndic stayed seems mostly untouched. That has resulted in this small regression - the warning you see.

This is the relevant part of code: https://github.com/saltstack/salt/blob/483ca17006cfa9fd5615f62d09d6508c53fc741f/salt/minion.py#L545

        # check if master_type was altered from its default
        if opts["master_type"] != "str" and opts["__role"] != "syndic":
        . . .
        # single master sign in
        else:
            if opts["random_master"]:
                log.warning(
                    "random_master is True but there is only one master specified."
                    " Ignoring."
                )

A useless warning for the salt-syndic, because in the code the random_master option is not used at all by the salt-syndic.

Instead for the syndic there is an option syndic_failover with default value random, which could alternatively be set to ordered. https://docs.saltproject.io/en/latest/ref/configuration/master.html#syndic-failover

I don't see any method to suppress the warning by change of configuration. A code change were needed. As the log is not relevant for the salt-syndic, the condition could be changed to

            if opts["random_master"] and opts["__role"] != "syndic":

I have the feeling the code related to salt-syndic deserves meanwhile some redesign here and there. But as long as it runs, this may not be the most urgent code change in overall salt.