Skip to content
This repository has been archived by the owner on Mar 23, 2019. It is now read-only.

fixes #937 #938

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open

fixes #937 #938

wants to merge 15 commits into from

Conversation

zskulcsar
Copy link

ISSUE TYPE
  • Bugfix Pull Request
SUMMARY

The original code had an if condition for the debug flag resulting in a situation when the code was running a different set of api calls.

ansible-container build
Building Docker Engine context...
Starting Docker build of Ansible Container Conductor image (please be patient)...
ERROR	Unknown exception
Traceback (most recent call last):
  File "/Users/zsoltk/work/iw/petrol-test/venv/lib/python2.7/site-packages/container/cli.py", line 299, in __call__
    getattr(core, u'hostcmd_{}'.format(args.subcommand))(**vars(args))
  File "/Users/zsoltk/work/iw/petrol-test/venv/lib/python2.7/site-packages/container/__init__.py", line 28, in __wrapped__
    return fn(*args, **kwargs)
  File "/Users/zsoltk/work/iw/petrol-test/venv/lib/python2.7/site-packages/container/core.py", line 181, in hostcmd_build
    environment=env_vars
  File "/Users/zsoltk/work/iw/petrol-test/venv/lib/python2.7/site-packages/container/docker/engine.py", line 105, in __wrapped__
    return fn(self, *args, **kwargs)
  File "/Users/zsoltk/work/iw/petrol-test/venv/lib/python2.7/site-packages/container/__init__.py", line 28, in __wrapped__
    return fn(*args, **kwargs)
  File "/Users/zsoltk/work/iw/petrol-test/venv/lib/python2.7/site-packages/container/docker/engine.py", line 1070, in build_conductor_image
    return image.id
AttributeError: 'tuple' object has no attribute 'id'




ansible-container build
Building Docker Engine context...
Starting Docker build of Ansible Container Conductor image (please be patient)...
Parsing conductor CLI args.
Docker™ daemon integration engine loaded. Build starting.	project=petrol_nginx
Building service...	project=petrol_nginx service=web
Applied role nginx from cache	role=nginx service=web
Build complete.	service=web
All images successfully built.
Conductor terminated. Preserving as requested.	command_rc=0 conductor_id=75a1f0c0857a4e381763084248fad1faa84cd92d8576ce163c1dd461ae6cd41a save_container=yes

@gregdek
Copy link
Contributor

gregdek commented Jun 8, 2018

Thanks @zskulcsar -- would love to see some comments from other contributors on this one.

@Voronenko
Copy link
Contributor

@gregdek I confirm the issue. Have to use --debug in prod pipelines for a while - works robust.

@gregdek
Copy link
Contributor

gregdek commented Jun 14, 2018

Hmm, weird travis breakage in all the current jobs...

@Voronenko
Copy link
Contributor

Voronenko commented Jun 14, 2018

@gregdek Regarding weird breakage on develop - actually ubuntu:bionic caused that due to typo in tag (not sure how it was green in form of PR) - nevertheless - this is chore to address that #943 (again green - @gregdek can you merge chore ?)

Regarding this PR breakage - it should be rebased over recent develop. @zskulcsar can you rebase your PR so it could pass the checks ?

@Voronenko
Copy link
Contributor

@gregdek ^^ Green

@Voronenko
Copy link
Contributor

Generally, approved. Should be pretty safe to merge - depends on releasing approach.

Whenever 0.9.3 is a "service pack" - i.e. mostly 0.9.2 with pip fixes, and that one goes towards 0.9.4
or can go into 0.9.3

@Voronenko
Copy link
Contributor

@zskulcsar I am sorry, can you rebase your PR over recent develop once more?
I do expect to see only your scope of changes in diff.

@@ -15,6 +15,7 @@ env:
- BASE_DISTRO: ubuntu:precise
- BASE_DISTRO: ubuntu:trusty
- BASE_DISTRO: ubuntu:xenial
- BASE_DISTRO: ubuntu:beaver
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be 'bionic' ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment correct, but not from this pr. It just needs to be correctly rebased over develop

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

Successfully merging this pull request may close these issues.

5 participants