naftulikay / ansible-role-degoss

An Ansible role for installing, running, and removing Goss from a system without leaving any traces.
Other
42 stars 12 forks source link

Support Python 3 #47

Open nicholasamorim opened 5 years ago

nicholasamorim commented 5 years ago

There are several errors related bytes/str when running the role with Python3.

  1. Writing the goss binary using w fails - use wb
  2. self.fail call fails on stdout.split line
  3. p.communicate call fails due to json.dumps outputting str not bytes

Fixing all 3 above still gave me errors and I can't proceed on this right now, but I'll try to come up with a PR this week.

naftulikay commented 5 years ago

Interesting. As mentioned in the other issue, Travis CI has no problems running all this on Python 3.6. I'm wondering why this isn't working for you locally.

nicholasamorim commented 5 years ago

Yeah, I noticed the build runs fine after I created this ticket.

Investigating futher, I noticed the default python3 on the remote host is python 3.5 (ubuntu xenial). Changing to 3.7 made no difference. Using 2.7 does work, though. (changed with ansible_python_interpreter)

It fails on the run_tests step:

TASK [naftulikay.degoss : run tests] ***************************************************************************************************************************************************************************************************************************************************************************************************************************************
fatal: [pf-us1-smdev2]: FAILED! => {"changed": false, "module_stderr": "Shared connection to 192.168.51.118 closed.
", "module_stdout": "/home/nicholas/.ansible/tmp/ansible-tmp-1556737210.122899-108778717246323/AnsiballZ_degoss.py:17: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
  import imp
Traceback (most recent call last):
  File \"/home/nicholas/.ansible/tmp/ansible-tmp-1556737210.122899-108778717246323/AnsiballZ_degoss.py\", line 113, in <module>
    _ansiballz_main()
  File \"/home/nicholas/.ansible/tmp/ansible-tmp-1556737210.122899-108778717246323/AnsiballZ_degoss.py\", line 105, in _ansiballz_main
    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
  File \"/home/nicholas/.ansible/tmp/ansible-tmp-1556737210.122899-108778717246323/AnsiballZ_degoss.py\", line 48, in invoke_module
    imp.load_module('__main__', mod, module, MOD_DESC)
  File \"/usr/lib/python3.7/imp.py\", line 234, in load_module
    return load_source(name, filename, file)
  File \"/usr/lib/python3.7/imp.py\", line 169, in load_source
    module = _exec(spec, sys.modules[name])
  File \"<frozen importlib._bootstrap>\", line 630, in _exec
  File \"<frozen importlib._bootstrap_external>\", line 728, in exec_module
  File \"<frozen importlib._bootstrap>\", line 219, in _call_with_frames_removed
  File \"/tmp/ansible_degoss_payload_jjs18zl1/__main__.py\", line 444, in <module>
  File \"/tmp/ansible_degoss_payload_jjs18zl1/__main__.py\", line 91, in main
  File \"/tmp/ansible_degoss_payload_jjs18zl1/__main__.py\", line 295, in execute
  File \"/tmp/ansible_degoss_payload_jjs18zl1/__main__.py\", line 338, in install
TypeError: write() argument must be str, not bytes
", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1}
naftulikay commented 5 years ago

It's been a hot second since I last Python'd, does wb always work regardless of Python version? If I recall correctly, there was some weird incompatibilities between Python 2 and 3 with this option. The line triggering the issue is likely this one.

nicholasamorim commented 5 years ago

Yeah, I fixed that one but then 2 and 3 from the list appear - which I fixed as well but then there's more so I had to stop for the time being. The wb is fine for both 2.x and 3.x.

I just took a quick look at the tests folder - I'm wondering: the tests run on Python3.6 on the ansible host or on the remote host?

naftulikay commented 5 years ago

Tests run on the host. If you'd like, you can PR in changes to Travis which will run the tests on 2.7, 3.4, 3.5, 3.6, and if Travis supports it yet, 3.7. Build times will increase, but this should give better coverage across Python versions.

asciifaceman commented 3 years ago

I actually have a better solution to this on my local

        with open(self.executable, 'wb') as f:

            shutil.copyfileobj(response, f)
            response.close()

There is also a bug in the json parsing fixed by adding encoding

        p = subprocess.Popen(cli_arguments, cwd=self.test_dir, env=dict(os.environ), stdout=subprocess.PIPE,
            stderr=subprocess.STDOUT, stdin=subprocess.PIPE, encoding='utf8')