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

[BUG] Fileclient requests generated as a result of Pillar SLS rendering cause fileserver backends to be refreshed #65990

Closed
terminalmage opened this issue Feb 6, 2024 · 1 comment · Fixed by #65991
Labels
Bug broken, incorrect, or confusing behavior needs-triage

Comments

@terminalmage
Copy link
Contributor

terminalmage commented Feb 6, 2024

Description

When a pillar SLS file includes a call-out to a function from the cp module (either directly, or by calling a salt function which calls out to the cp module), this spawns a new instance of the fileclient using the current opts. When file_client is set to local (as it is during PIllar rendering), this fileclient is an instance of salt.fileclient.FSClient. During instantiation of this instance, the FSClient instantiates an instance of salt.fileserver.FSChan, which in turn has its own instance of the fileserver.

On a masterless salt run, this is how masterless minions interface with the fileserver backends specified in the minion config. For that reason, salt.fileserver.FSChan refreshes all of its fileserver backends, allowing for gitfs to be successfully used in masterless.

However, this refresh is not needed during Pillar rendering in a traditional master/minion setup, because the master has a maintenance thread which handles keeping fileserver backends up-to-date. Not only is it unneeded, it slows down Pillar rendering performing unneeded refreshes. If Pillar rendering coincides with a normal fileserver update, this slowdown is exacerbated due to the fact that the act of refreshing a gitfs remote obtains a blocking lock. As the number of gitfs_remotes increases, this increases the likelihood that a pillar refresh (or the initial pillar obtained when a minion connects to the master) will timeout.

Setup

/etc/salt/master

# This is just here to simplify key acceptance for the purposes of reproducing this bug
auto_accept: True

# Use gitfs only
fileserver_backend:
  - git

gitfs_remotes:
  # This repo must have a master branch containing a simple JSON file (e.g. foo.json)
  - https://github.com/someuser/somerepo.git:
    # Interval increased to an absurd length so a normal maintenance thread
    # fileserver update won't be confused for one started in pillar rendering  
    - update_interval: 1800

/srv/pillar/top.sls

base:
  '*':
    - test

/srv/pillar/test.sls

# The path after salt:// is the relative path to the JSON file in your gitfs repo 
foo: {{ salt.cp.get_file_str("salt://path/to/foo.json") }}
# NOTE: This is a contrived example but serves to reproduce the behavior

Steps to Reproduce the behavior

  1. Bring up the master in the foreground using salt-master -l debug
  2. Configure a minion to connect to your master's IP address.

Expected behavior

Wait for the minion to connect after the key has been accepted. When it does, it will request Pillar data from the master.

Because the pillar SLS file you created is making a fileclient request, there will be several lines in the master's debug logging (check the scrollback) which mention gitfs. It is normal to see some gitfs log entries here, because Salt is attaching to all the local gitfs cachedirs. However, we should not see any log entries for repo locking and fetching, since those updates are handled by the maintenance thread. Nonetheless, with the above setup, you will see the following:

[DEBUG   ][40978] Set update lock for gitfs remote 'https://github.com/someuser/somerepo.git'
[DEBUG   ][40978] Fetching gitfs remote 'https://github.com/someuser/somerepo.git'
[DEBUG   ][40978] gitfs remote 'https://github.com/someuser/somerepo.git' is up-to-date
[DEBUG   ][40978] Removed update lock for gitfs remote 'https://github.com/someuser/somerepo.git'

Versions Report

This bug would affect all salt-master versions going back to when this fileserver update was added in fe4537f, over 9 years ago.

@terminalmage
Copy link
Contributor Author

This was fixed by merging #65991, will mark this as closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior needs-triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant