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

Prevent crash if pygit2 package is requesting re-compilation of the e… #32652

Merged
merged 4 commits into from
Apr 18, 2016

Conversation

isbm
Copy link
Contributor

@isbm isbm commented Apr 18, 2016

What does this PR do?

Bugfix.

What issues does this PR fix or reference?

Previous Behavior

bash-4.2# salt-master
usr/lib64/python2.7/site-packages/pygit2/__pycache__/_cffi__x50f7320ax7286955d.c:2:20: fatal error: Python.h: No such file or directory
 #include <Python.h>
                    ^
compilation terminated.
[ERROR   ] An un-handled exception was caught by salt's global exception handler:
VerificationError: CompileError: command 'gcc' failed with exit status 1
Traceback (most recent call last):
  File "/usr/bin/salt-master", line 22, in <module>
    salt_master()
  File "/usr/lib/python2.7/site-packages/salt/scripts.py", line 47, in salt_master
    master.start()
  File "/usr/lib/python2.7/site-packages/salt/cli/daemons.py", line 196, in start
    self.prepare()
  File "/usr/lib/python2.7/site-packages/salt/cli/daemons.py", line 176, in prepare
    import salt.master
  File "/usr/lib/python2.7/site-packages/salt/master.py", line 46, in <module>
    import salt.key
  File "/usr/lib/python2.7/site-packages/salt/key.py", line 23, in <module>
    import salt.daemons.masterapi
  File "/usr/lib/python2.7/site-packages/salt/daemons/masterapi.py", line 37, in <module>
    from salt.pillar import git_pillar
  File "/usr/lib/python2.7/site-packages/salt/pillar/git_pillar.py", line 188, in <module>
    import salt.utils.gitfs
  File "/usr/lib/python2.7/site-packages/salt/utils/gitfs.py", line 77, in <module>
    import pygit2
  File "/usr/lib64/python2.7/site-packages/pygit2/__init__.py", line 35, in <module>
    from .blame import Blame, BlameHunk
  File "/usr/lib64/python2.7/site-packages/pygit2/blame.py", line 32, in <module>
    from .errors import check_error
  File "/usr/lib64/python2.7/site-packages/pygit2/errors.py", line 29, in <module>
    from .ffi import ffi, C
  File "/usr/lib64/python2.7/site-packages/pygit2/ffi.py", line 36, in <module>
    C = ffi.verify(preamble, **C_KEYWORDS)
  File "/usr/lib64/python2.7/site-packages/cffi/api.py", line 373, in verify
    lib = self.verifier.load_library()
  File "/usr/lib64/python2.7/site-packages/cffi/verifier.py", line 96, in load_library
    self._compile_module()
  File "/usr/lib64/python2.7/site-packages/cffi/verifier.py", line 192, in _compile_module
    outputfilename = ffiplatform.compile(tmpdir, self.get_extension())
  File "/usr/lib64/python2.7/site-packages/cffi/ffiplatform.py", line 38, in compile
    outputfilename = _build(tmpdir, ext)
  File "/usr/lib64/python2.7/site-packages/cffi/ffiplatform.py", line 65, in _build
    raise VerificationError('%s: %s' % (e.__class__.__name__, e))
VerificationError: CompileError: command 'gcc' failed with exit status 1
Traceback (most recent call last):
  File "/usr/bin/salt-master", line 22, in <module>
    salt_master()
  File "/usr/lib/python2.7/site-packages/salt/scripts.py", line 47, in salt_master
    master.start()
  File "/usr/lib/python2.7/site-packages/salt/cli/daemons.py", line 196, in start
    self.prepare()
  File "/usr/lib/python2.7/site-packages/salt/cli/daemons.py", line 176, in prepare
    import salt.master
  File "/usr/lib/python2.7/site-packages/salt/master.py", line 46, in <module>
    import salt.key
  File "/usr/lib/python2.7/site-packages/salt/key.py", line 23, in <module>
    import salt.daemons.masterapi
  File "/usr/lib/python2.7/site-packages/salt/daemons/masterapi.py", line 37, in <module>
    from salt.pillar import git_pillar
  File "/usr/lib/python2.7/site-packages/salt/pillar/git_pillar.py", line 188, in <module>
    import salt.utils.gitfs
  File "/usr/lib/python2.7/site-packages/salt/utils/gitfs.py", line 77, in <module>
    import pygit2
  File "/usr/lib64/python2.7/site-packages/pygit2/__init__.py", line 35, in <module>
    from .blame import Blame, BlameHunk
  File "/usr/lib64/python2.7/site-packages/pygit2/blame.py", line 32, in <module>
    from .errors import check_error
  File "/usr/lib64/python2.7/site-packages/pygit2/errors.py", line 29, in <module>
    from .ffi import ffi, C
  File "/usr/lib64/python2.7/site-packages/pygit2/ffi.py", line 36, in <module>
    C = ffi.verify(preamble, **C_KEYWORDS)
  File "/usr/lib64/python2.7/site-packages/cffi/api.py", line 373, in verify
    lib = self.verifier.load_library()
  File "/usr/lib64/python2.7/site-packages/cffi/verifier.py", line 96, in load_library
    self._compile_module()
  File "/usr/lib64/python2.7/site-packages/cffi/verifier.py", line 192, in _compile_module
    outputfilename = ffiplatform.compile(tmpdir, self.get_extension())
  File "/usr/lib64/python2.7/site-packages/cffi/ffiplatform.py", line 38, in compile
    outputfilename = _build(tmpdir, ext)
  File "/usr/lib64/python2.7/site-packages/cffi/ffiplatform.py", line 65, in _build
    raise VerificationError('%s: %s' % (e.__class__.__name__, e))
cffi.ffiplatform.VerificationError: CompileError: command 'gcc' failed with exit status 1

And then a complete Salt Master or Salt Minion stop (crash).

New Behavior

# salt-master
usr/lib64/python2.7/site-packages/pygit2/__pycache__/_cffi__x50f7320ax7286955d.c:2:20: fatal error: Python.h: No such file or directory
 #include <Python.h>
                    ^
compilation terminated.
[ERROR   ] Import pygit2 failed: CompileError: command 'gcc' failed with exit status 1

Still throws an error to the STDERR (pygit2 part), but at least no crash and an error log message is put in case of Exception had happened for other than ImportError reasons.

Tests written?

No

@cachedout cachedout merged commit 8d46244 into saltstack:2015.8 Apr 18, 2016
@cachedout
Copy link
Contributor

Looks good. Thanks, @isbm

@isbm
Copy link
Contributor Author

isbm commented Apr 18, 2016

@cachedout new hash-tag: #squash 😆

@cachedout
Copy link
Contributor

I forgot. My fault. Apologies.

@isbm
Copy link
Contributor Author

isbm commented Apr 18, 2016

🙇

@damon-atkins
Copy link
Contributor

there also needs to be a comment in the doco that pygit2 needs gcc as well... An RPM that installs, but really does not install because at first run, it needs to compile itself.

@isbm
Copy link
Contributor Author

isbm commented Apr 19, 2016

@damon-atkins pygit2 is a 3rd party software, so has nothing to do with the Salt. Salt only sees that 3rd party module pygit2 is installed in the system, assuming the rest of the packaging of the pygit2 is done right (i.e. pre-compilation is done during the package build). If the import fails for whatever reason, this is a problem of pygit2 module.

@damon-atkins
Copy link
Contributor

damon-atkins commented Apr 19, 2016

You can install it, think that it will work, and then try to use it only to find it needs gcc to finish it off. It's odd. It should at least compile it during the actual install, not wait for it to be used.

None of this is Salts fault, but a warning on Salt Doco would be helpful by the Salt Team. Rather people wast time with it. A lot of companies will not install compiles in production.

@isbm
Copy link
Contributor Author

isbm commented Apr 19, 2016

@damon-atkins Why gcc issue is any special? There can be other problems with the pygit2. E.g. bad package dependencies and no libgit2 installed. Or good package dependencies and libgit2 yanked by an RPM manually. Or pygit2 version is different from the libgit2 version and you will have a rock-n-roll during the successful runtime. And many other problems.

I would prefer simply ignore pygit2 for whatever reason it fails. It just fails, period. So as an admin, you will notice an error log message and then just go and figure out why it is failing, doing that off the Salt context.

At SUSE we removing the recommendation of pygit2 and recommending gitpython instead. Not a bindings, but works always stable.

rallytime pushed a commit that referenced this pull request Apr 25, 2016
* json encode arguments passed to an execution module function call

this fixes problems where you could pass a string to a module function,
which thanks to the yaml decoder which is used when parsing command line
arguments could change its type entirely. for example:

__salt__['test.echo')('{foo: bar}')

the test.echo function just returns the argument it's given. however,
because it's being called through a salt-call process like this:

salt-call --local test.echo {foo: bar}

salt thinks it's yaml and therefore yaml decodes it. the return value
from the test.echo call above is therefore a dict, not a string.

* Prevent crash if pygit2 package is requesting re-compilation of the e… (#32652)

* Prevent crash if pygit2 package is requesting re-compilation of the entire library on production systems (no *devel packages)

* Fix PEP8: move imports to the top of the file

* Move logger up

* Add log error message in case if exception is not an ImportError

* align OS grains from older SLES with current one (#32649)

* Fixing critical bug to remove only the specified Host instead of the entire Host cluster (#32640)

* yumpkg: Ignore epoch in version comparison for explict versions without an epoch (#32563)

* yumpkg: Ignore epoch in version comparison for explict versions without an epoch

Also properly handle comparisions for packages with multiple versions.

Resolves #32229

* Don't attempt downgrade for kernel and its subpackages

Multiple versions are supported since their paths do not conflict.

* Lower log level for pillar cache (#32655)

This shouldn't show up on salt-call runs

* Don't access deprecated Exception.message attribute. (#32556)

* Don't access deprecated Exception.message attribute.

To avoid a deprecation warning message in logs.
There is a new function salt.exceptions.get_error_message(e) instead.

* Fixed module docs test.

* Fix for issue 32523 (#32672)

* Fix routes for redhat < 6

* Handle a couple of arguments better (Azure) (#32683)

* backporting a fix from develop where the use of splay would result in seconds=0 in the schedule.list when there was no seconds specified in the origina schedule

* Handle when beacon not configured and we try to enable/disable them (#32692)

* Handle the situation when the beacon is not configured and we try to disable it

* a couple more missing returns in the enable & disable

* Check dependencies type before appling str operations (#32693)

* Update external auth documentation to list supported matcher. (#32733)

Thanks to #31598, all matchers are supported for eauth configuration.
But we still have no way to use compound matchers in eauth configuration.
Update the documentation to explicitly express this limitation.

* modules.win_dacl: consistent case of dacl constants (#32720)

* Document pillar cache options (#32643)

* Add note about Pillar data cache requirement for Pillar targeting method

* Add `saltutil.refresh_pillar` function to the scheduled Minion jobs

* Minor fixes in docs

* Add note about relations between `pillar_cache` option and Pillar Targeting
to Master config comments with small reformatting

* Document Pillar Cache Options for Salt Master

* Document Minions Targeting with Mine

* Remove `saltutil.refresh_pillar` scheduled persistent job

* Properly handle minion failback failure. (#32749)

* Properly handle minion failback failure.

Initiate minion restart if all masters down on __master_disconnect like
minion does on the initial master connect on start.

* Fixed unit test

* Improve documentation on pygit2 versions (#32779)

This adds an explanation of the python-cffi dep added in pygit2 0.21.0,
and recommends 0.20.3 for LTS distros. It also links to the salt-pack
issue which tracks the progress of adding pygit2 to our Debian and
Ubuntu repositories.

* Pylint fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants