Skip to content
This repository has been archived by the owner on Dec 16, 2024. It is now read-only.

Can no longer highstate without using the /tmp override #793

Open
jdm opened this issue Feb 6, 2018 · 8 comments
Open

Can no longer highstate without using the /tmp override #793

jdm opened this issue Feb 6, 2018 · 8 comments

Comments

@jdm
Copy link
Member

jdm commented Feb 6, 2018

When I run salt 'servo-master1' state.highstate test=True, I now get:

servo-master1:
----------
          ID: states
    Function: no.None
      Result: False
     Comment: No Top file or external nodes data matches found.
     Started:
    Duration:
     Changes:

Summary for servo-master1
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:   0.000 ms

If I set up the override in /tmp/salt-testing-root, I can highstate fine. As soon as I clean up the override, I cannot. @aneeshusa I'm not sure what could have caused this; #786 is the only PR that looks remotely related, but I don't see a smoking gun yet.

@aneeshusa
Copy link
Contributor

I checked via salt 'servo-master1' state.show_top to confirm no states listed. My first guess for the cause was a stale Salt fileserver cache, so per our docs, I ran salt-run fileserver.update on servo-master1, which spit out:

root@servo-master1:~# salt-run fileserver.update
[WARNING ] /usr/lib/python2.7/dist-packages/salt/grains/core.py:1493: DeprecationWarning: The "osmajorrelease" will be a type of an integer.

Exception occurred in runner fileserver.update: Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/client/mixins.py", line 356, in low
    data[return] = self.functions[fun](*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/salt/runners/fileserver.py", line 258, in update
    fileserver.update(back=backend)
  File "/usr/lib/python2.7/dist-packages/salt/fileserver/__init__.py", line 457, in update
    self.servers[fstr]()
  File "/usr/lib/python2.7/dist-packages/salt/fileserver/gitfs.py", line 124, in update
    gitfs.update()
  File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 2225, in update
    self.find_file
  File "/usr/lib/python2.7/dist-packages/salt/fileserver/__init__.py", line 244, in reap_fileserver_cache_dir
    ret = find_func(filename, saltenv=saltenv)
  File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 2584, in find_file
    blob, blob_hexsha = repo.find_file(repo_path, tgt_env)
  File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 851, in find_file
    tree = self.get_tree(tgt_env)
  File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 899, in get_tree
    return ref.commit.tree
  File "/usr/lib/python2.7/dist-packages/gitdb/util.py", line 238, in __getattr__
    self._set_cache_(attr)
  File "/usr/lib/python2.7/dist-packages/git/objects/commit.py", line 132, in _set_cache_
    self._deserialize(StringIO(stream.read()))
  File "/usr/lib/python2.7/dist-packages/git/objects/commit.py", line 443, in _deserialize
    self.author.name = self.author.name.decode(self.encoding)
LookupError: unknown encoding: -----BEGIN PGP SIGNATURE-----

So it looks like the error is related to Salt (or rather git-python) not being able to parse GPG signed git commits. Looking at recent commits, it seems 1f62195 is the first time we see a signed merge commit. This was not triggered by previous signed commits, because those would be commits on a branch, and not merge commits created by Homu, so they would never be at the tip of master, and thus never parsed by git-python.

Looking at the git-python issue tracker for gpg related issues (as well as the git-python source as of 0.3.2-RC1, the version closest to the 0.3.2-RC3 reported by dpkg -l python-git on servo-master), my current belief is that our current git-python is too old and we will need a newer git-python with support for parsing GPG signed commits.

We could install via pip globally for a short term fix, but a long term fix is probably to update to xenial (#462), which has version 1.0.1 - not the newest but new enough for the GPG support, seemingly added in 0.3.

Side question: How is Homu making merge commits that are signed with your key @jdm? I don't know how it would do that without access to your private GPG key.

@jdm
Copy link
Member Author

jdm commented Feb 7, 2018

Fascinating; the web UI for editing files on github signs the resulting commits apparently. It's still not clear how that propagates to homu's merge; maybe it's related to the fact that I recently logged into the bors-servo account.

@jdm
Copy link
Member Author

jdm commented Jul 27, 2018

I tried running pip install python-git on servo-master1 and it did not make a difference. Perhaps I'm misunderstanding how the short term fix is expected to apply.

@jdm
Copy link
Member Author

jdm commented Sep 25, 2018

Salt no longer complains about the fileserver update:

root@servo-master1:~# salt-run fileserver.update
[WARNING ] /usr/lib/python2.7/dist-packages/salt/utils/gitfs.py:400: DeprecationWarning: The gitfs_env_whitelist config option (and env_whitelist per-remote config option) have been renamed to gitfs_saltenv_whitelist (and saltenv_whitelist). Please update your configuration.

True

@jdm
Copy link
Member Author

jdm commented Sep 25, 2018

Maybe this just works now!

root@servo-master1:~# salt 'servo-master1' state.show_top
servo-master1:
    ----------
    base:
        - admin
        - common
        - python
        - salt.common
        - ubuntu
        - git
        - buildbot.master
        - homu
        - intermittent-tracker
        - intermittent-failure-tracker
        - upstream-wpt-webhook
        - nginx
        - salt.master

@jdm
Copy link
Member Author

jdm commented Sep 25, 2018

Nevermind, when I remove the tmp clone it's back to being empty.

@jdm
Copy link
Member Author

jdm commented Apr 9, 2019

Still seeing lots of entries like this in the salt logs at /var/log/salt/master:

2019-04-09 12:14:34,275 [salt.master      :1783][ERROR   ][29328] Error in function _file_list:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/master.py", line 1776, in run_func
    ret = getattr(self, func)(load)
  File "/usr/lib/python2.7/dist-packages/salt/utils/decorators/__init__.py", line 594, in wrapped
    **salt.utils.data.decode_dict(kwargs)
  File "/usr/lib/python2.7/dist-packages/salt/fileserver/__init__.py", line 752, in file_list
    ret.update(self.servers[fstr](load))
  File "/usr/lib/python2.7/dist-packages/salt/fileserver/gitfs.py", line 185, in file_list
    return _gitfs().file_list(load)
  File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 2929, in file_list
    return self._file_lists(load, 'files')
  File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 2906, in _file_lists
    repo_files, repo_symlinks = repo.file_list(load['saltenv'])
  File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 1271, in file_list
    tree = self.get_tree(tgt_env)
  File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 1012, in get_tree
    candidate = func(tgt_ref)
  File "/usr/lib/python2.7/dist-packages/salt/utils/gitfs.py", line 1350, in get_tree_from_branch
    'refs/remotes/origin/{0}'.format(ref)).commit.tree
  File "/usr/lib/python2.7/dist-packages/gitdb/util.py", line 238, in __getattr__
    self._set_cache_(attr)
  File "/usr/lib/python2.7/dist-packages/git/objects/commit.py", line 132, in _set_cache_
    self._deserialize(StringIO(stream.read()))
  File "/usr/lib/python2.7/dist-packages/git/objects/commit.py", line 443, in _deserialize
    self.author.name = self.author.name.decode(self.encoding)
LookupError: unknown encoding: -----BEGIN PGP SIGNATURE-----

@jdm
Copy link
Member Author

jdm commented Apr 9, 2019

I tried upgrading GitPython to 0.3.2rc1 (pip install -U GitPython) and the problem does not appear to occur in the logs any more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants