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.

Sign up for GitHub

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

Jump to bottom

Bad gitfs_remote breaks sls-files in subdirectories for state.(sls|highstate) #21021

Closed
JPT580 opened this issue Feb 25, 2015 · 7 comments
Closed

Bad gitfs_remote breaks sls-files in subdirectories for state.(sls|highstate) #21021

JPT580 opened this issue Feb 25, 2015 · 7 comments
Labels
Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists
Milestone
Approved

Comments

@JPT580
Copy link

JPT580 commented Feb 25, 2015

Version

root@salt:/srv/git# salt --versions-report
           Salt: 2014.7.1
         Python: 2.7.3 (default, Mar 13 2014, 11:03:55)
         Jinja2: 2.6
       M2Crypto: 0.21.1
 msgpack-python: 0.1.10
   msgpack-pure: Not Installed
       pycrypto: 2.6
        libnacl: Not Installed
         PyYAML: 3.10
          ioflo: Not Installed
          PyZMQ: 13.1.0
           RAET: Not Installed
            ZMQ: 3.2.3
           Mako: 0.7.0

This may probably be obsolete with #6739 in mind.

Setup

  1. Have a salt-master, add valid gitfs_remotes which exist in the filesystem.
  2. Properly restart salt-master
  3. Delete one of the gitfs_remotes from the filesystem but do not remove it from the master config.

State tree in /srv/salt

root@salt:/srv/salt# find . | grep -v git
.
./_grains
./_grains/cfgnet-interface.py
./_grains/vmware-tools.py
./vmware-uptodate
./vmware-uptodate/init.sls
./vmware-uptodate/tmp
./vmware-uptodate/tmp/upgrade-vmware_x86_64.sh
./top.sls
./iptables-cfgnet
./iptables-cfgnet/init.sls
./iptables-cfgnet/tmp
./iptables-cfgnet/tmp/rules

Master configuration

/srv/git/vmware-tools-formula did not exist in my filesystem.

root@salt:/srv/salt# cat /etc/salt/master | grep -v '^#' | grep -v '^$'
interface: 10.1.10.1
ping_on_rotate: True
open_mode: False
auto_accept: False
file_recv: False
file_roots:
  base:
    - /srv/salt
fileserver_backend:
  - roots
  - git
gitfs_provider: gitpython
gitfs_remotes:
  - file:///srv/git/users-formula
  - file:///srv/git/elasticsearch-logstash-kibana-formula
  - file:///srv/git/build-essential-formula
  - file:///srv/git/vmware-tools-formula
  - file:///srv/git/iptables-formula
pillar_roots:
  base:
    - /srv/pillar

Expected result

Restarting the master fails / A warning like "Invalid gitfs_remote in config ... does not exist in filesystem" appears.

Observed result

  • Most things work.
  • Calling state.highstate or a specific state using state.sls fails with a trace, if the specific statefile is contained in a subdirectory.

Log excerpt from an affected minion:

Command used: salt 'salt-minion-1*' state.sls 'vmware-uptodate'

2015-02-25 14:45:27,879 [salt.crypt                                  ][DEBUG   ] Decrypting the current master AES key
2015-02-25 14:45:27,879 [salt.crypt                                  ][DEBUG   ] Loaded minion key: /etc/salt/pki/minion/minion.pem
2015-02-25 14:45:33,775 [salt.minion                                 ][INFO    ] User sudo_jpt Executing command state.sls with jid 20150225144534113680
2015-02-25 14:45:33,776 [salt.minion                                 ][DEBUG   ] Command details {'tgt_type': 'glob', 'jid': '20150225144534113680', 'tgt': 'salt-minion-1*', 'ret': '', 'user': 'sudo_jpt', 'arg': ['vmware-uptodate'], 'fun': 'state.sls'}
2015-02-25 14:45:33,786 [salt.minion                                 ][INFO    ] Starting a new job with PID 23520
2015-02-25 14:45:33,972 [salt.crypt                                  ][DEBUG   ] Decrypting the current master AES key
2015-02-25 14:45:33,973 [salt.crypt                                  ][DEBUG   ] Loaded minion key: /etc/salt/pki/minion/minion.pem
2015-02-25 14:45:35,252 [salt.config                                 ][DEBUG   ] Reading configuration from /etc/salt/minion
2015-02-25 14:45:35,854 [salt.crypt                                  ][DEBUG   ] Decrypting the current master AES key
2015-02-25 14:45:35,855 [salt.crypt                                  ][DEBUG   ] Loaded minion key: /etc/salt/pki/minion/minion.pem
2015-02-25 14:45:36,116 [salt.crypt                                  ][DEBUG   ] Loaded minion key: /etc/salt/pki/minion/minion.pem
2015-02-25 14:45:36,215 [salt.state                                  ][INFO    ] Loading fresh modules for state activity
2015-02-25 14:45:36,670 [salt.fileclient                             ][DEBUG   ] Fetching file from saltenv 'base', ** attempting ** 'salt://vmware-uptodate.sls'
2015-02-25 14:45:36,749 [salt.fileclient                             ][ERROR   ] Data is 
2015-02-25 14:45:36,750 [salt.minion                                 ][WARNING ] TypeError encountered executing state.sls: string indices must be integers, not str. See debug log for more info.
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/minion.py", line 1022, in _thread_return
    return_data = func(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/salt/modules/state.py", line 461, in sls
    high_, errors = st_.render_highstate({saltenv: mods})
  File "/usr/lib/python2.7/dist-packages/salt/state.py", line 2724, in render_highstate
    sls, saltenv, mods, matches)
  File "/usr/lib/python2.7/dist-packages/salt/state.py", line 2407, in render_state
    state_data = self.client.get_state(sls, saltenv)
  File "/usr/lib/python2.7/dist-packages/salt/fileclient.py", line 422, in get_state
    dest = self.cache_file(path, saltenv)
  File "/usr/lib/python2.7/dist-packages/salt/fileclient.py", line 147, in cache_file
    return self.get_url(path, '', True, saltenv)
  File "/usr/lib/python2.7/dist-packages/salt/fileclient.py", line 521, in get_url
    return self.get_file(url, dest, makedirs, saltenv)
  File "/usr/lib/python2.7/dist-packages/salt/fileclient.py", line 991, in get_file
    if not data['data']:
TypeError: string indices must be integers, not str
2015-02-25 14:45:36,752 [salt.minion                                 ][INFO    ] Returning information for job: 20150225144534113680
2015-02-25 14:45:36,921 [salt.crypt                                  ][DEBUG   ] Decrypting the current master AES key
@rallytime
Copy link
Contributor

Thanks for this excellent report @JPT580! I can totally see your point. I think what is happening here is that we didn't want to make a bad gitfs remote fatal and just log the error. But if it isn't fatal, then people aren't necessarily looking at the log.

I can also see your reasoning about wanting the master to fail to start when the config is bad. I don't think there currently is a method to make that fatal. The only fatality that occurs now is if no fileserver backends are configured for the situation where a user has roots and git configured but git is broken, we still have roots available.

Anyhow, this is a great point that will take some careful consideration. I've discussed this issue with @terminalmage, our main gitfs man.

@rallytime rallytime added Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists labels Feb 25, 2015
@rallytime rallytime added this to the Approved milestone Feb 25, 2015
@terminalmage
Copy link
Contributor

@JPT580 I'll discuss this with some of the other core devs and try to come up with a way to make invalid FS backend configuration fatal. I really like the idea.

@terminalmage terminalmage mentioned this issue Feb 25, 2015
Merged
@terminalmage
Copy link
Contributor

@JPT580 What do you think of this?

# salt-master -l debug 2>&1 | tee ~erik/master.log
[DEBUG   ] Reading configuration from /etc/salt/master
[DEBUG   ] Using cached minion ID from /home/erik/saltdev/etc/salt/minion_id: saltmine
[DEBUG   ] Configuration file path: /etc/salt/master
[INFO    ] Setting up the Salt Master
[DEBUG   ] Loaded master key: /home/erik/saltdev/etc/salt/pki/master/master.pem
[INFO    ] Preparing the root key for local communication
[DEBUG   ] Removing stale keyfile: /home/erik/saltdev/var/cache/salt/master/.root_key
[DEBUG   ] Created pidfile: /home/erik/saltdev/var/run/salt-master.pid
[DEBUG   ] gitpython gitfs_provider enabled
[ERROR   ] Invalid input: None
[ERROR   ] Input must be a list of strings/dicts
[ERROR   ] Invalid per-remote configuration for remote https://github.com/terminalmage/gitfs-test1.git. If no per-remote parameters are being specified, there may be a trailing colon after the URL, which should be removed. Check the master configuration file.
[ERROR   ] Failed to load git fileserver backend
[ERROR   ] Master failed pre flight checks, exiting

@JPT580
Copy link
Author

JPT580 commented Feb 25, 2015

This should be better to debug for most people.
It may be better to have the salt-master refuse to start, but this should suffice.
Thanks a lot! 👍

@terminalmage
Copy link
Contributor

@JPT580 no, that's exactly what is happening. hence the "exiting" message

@JPT580
Copy link
Author

JPT580 commented Feb 25, 2015

Sorry, i misread that. It's perfect!

terminalmage added a commit to terminalmage/salt that referenced this issue Feb 25, 2015
@terminalmage
Catch FileserverConfigError exceptions on master startup
d54a04d 
This causes the master to fail to start when there are fileserver config
issues, leading people quicker to the log to investigate, where they
will find a meaningful error message. This should it much quicker and
easier for users to realize there are gitfs/hgfs/svnfs fileserver config
problems.

Fixes saltstack#21021.
@terminalmage
Copy link
Contributor

Fix was merged. Closing. Thanks again for the suggestion!

cro pushed a commit to cro/salt that referenced this issue Feb 27, 2015
@terminalmage @cro
Catch FileserverConfigError exceptions on master startup
6be3bff 
This causes the master to fail to start when there are fileserver config
issues, leading people quicker to the log to investigate, where they
will find a meaningful error message. This should it much quicker and
easier for users to realize there are gitfs/hgfs/svnfs fileserver config
problems.

Fixes saltstack#21021.
cro pushed a commit to cro/salt that referenced this issue Feb 27, 2015
@terminalmage @cro
Catch FileserverConfigError exceptions on master startup
be8c392 
This causes the master to fail to start when there are fileserver config
issues, leading people quicker to the log to investigate, where they
will find a meaningful error message. This should it much quicker and
easier for users to realize there are gitfs/hgfs/svnfs fileserver config
problems.

Fixes saltstack#21021.
cro pushed a commit to cro/salt that referenced this issue Feb 28, 2015
@terminalmage @cro
Catch FileserverConfigError exceptions on master startup
8e7158d 
This causes the master to fail to start when there are fileserver config
issues, leading people quicker to the log to investigate, where they
will find a meaningful error message. This should it much quicker and
easier for users to realize there are gitfs/hgfs/svnfs fileserver config
problems.

Fixes saltstack#21021.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug broken, incorrect, or confusing behavior severity-low 4th level, cosemtic problems, work around exists
Projects
None yet
Milestone
Approved
Development

No branches or pull requests
3 participants
@terminalmage @JPT580 @rallytime