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

Add minimal support for docker-py-2.x module #663

Merged
merged 1 commit into from Aug 1, 2017
Merged

Add minimal support for docker-py-2.x module #663

merged 1 commit into from Aug 1, 2017

Conversation

twaugh
Copy link
Member

@twaugh twaugh commented Mar 6, 2017

@maxamillion, something to test when using a rawhide- or f26-based buildroot.

@vrutkovs
Copy link
Contributor

It looks fine to me, maybe its worth adding a new configuration to travis where docker-py 2 would be installed?

@twaugh
Copy link
Member Author

twaugh commented Mar 21, 2017

Not sure it's worth adding it to Travis: we mock the docker module completely in unit tests.

@maxamillion
Copy link
Contributor

I'll try to test this asap, kind of under water with some other stuff though so it'll be a bit.

@twaugh
Copy link
Member Author

twaugh commented Jun 27, 2017

@maxamillion, had any time for this?

@maxamillion
Copy link
Contributor

@twaugh I have not, but I'd be amazed if this caused any issues so please don't hold this up on my account. Apologies on the lag time as well as if this has been blocked by me for any reason.

docker.errors.APIError, "xyz", response)
else:
flexmock(docker.Client).should_receive(method).and_raise(docker.errors.APIError, "xyz",
flexmock(docker.APIClient).should_receive(method).and_raise(docker.errors.APIError, "xyz",

Choose a reason for hiding this comment

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

E501 line too long (102 > 100 characters)


kwargs = {}
if timeout is not None:
kwargs['timeout'] = timeout

DockerTasker(**kwargs)
t = DockerTasker(**kwargs)

Choose a reason for hiding this comment

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

F841 local variable 't' is assigned to but never used

@twaugh
Copy link
Member Author

twaugh commented Jul 14, 2017

Rebased, and updated to make the test suite pass when python-docker 2.x is installed.

@vrutkovs
Copy link
Contributor

Looks good so far, I'll check if it works in latest rawhide on osbs box

@vrutkovs
Copy link
Contributor

vrutkovs commented Jul 14, 2017

Crashes here - https://gist.github.com/vrutkovs/bdb44bb52b305c70d26516898d24a347

Tested on F26 osbs-box with Dockerfile: https://gist.github.com/vrutkovs/e6ddfba090f8214d1b31dedf4b3d44fa.

UPD. Seems to come from docker-squash though :/

@vrutkovs
Copy link
Contributor

vrutkovs commented Jul 14, 2017

Okay, now got a valid crash:

2017-07-14 13:10:44,030 platform:- - atomic_reactor.plugin - DEBUG - running plugin 'pull_base_image'
2017-07-14 13:10:44,030 platform:- - atomic_reactor.plugin - INFO - running plugin instance with args: '{u'parent_registry_insecure': True, u'parent_registry': u'registry.fedoraproject.org'}'
2017-07-14 13:10:44,031 platform:- - atomic_reactor.plugins.pull_base_image - INFO - pulling base image 'fedora:latest' from registry 'registry.fedoraproject.org'
2017-07-14 13:10:44,031 platform:- - atomic_reactor.core - INFO - pulling image 'registry.fedoraproject.org/fedora:latest' from registry
2017-07-14 13:10:44,031 platform:- - atomic_reactor.core - DEBUG - image = 'registry.fedoraproject.org/fedora:latest', insecure = 'True'
2017-07-14 13:10:44,086 platform:- - atomic_reactor.plugin - DEBUG - Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/atomic_reactor/plugin.py", line 241, in run
    plugin_response = plugin_instance.run()
  File "/usr/lib/python2.7/site-packages/atomic_reactor/plugins/pre_pull_base_image.py", line 63, in run
    insecure=self.parent_registry_insecure)
  File "/usr/lib/python2.7/site-packages/atomic_reactor/core.py", line 435, in pull_image
    insecure_registry=insecure, stream=True)
  File "/usr/lib/python2.7/site-packages/docker/api/image.py", line 358, in pull
    header = auth.get_config_header(self, registry)
AttributeError: 'module' object has no attribute 'get_config_header'
2017-07-14 13:10:44,106 platform:- - atomic_reactor.plugin - ERROR - plugin 'pull_base_image' raised an exception: AttributeError("'module' object has no attribute 'get_config_header'",)
2017-07-14 13:10:44,107 platform:- - atomic_reactor.inner - ERROR - one or more prebuild plugins failed: plugin 'pull_base_image' raised an exception: AttributeError("'module' object has no attribute 'get_config_header'",)

Since Tim is on PTO next week I'll try to come up with a fix and push it to the branch

UPD. This is docker-py weirdness - docker/compose#4344 (comment)

Copy link
Contributor

@vrutkovs vrutkovs left a comment

Choose a reason for hiding this comment

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

Worked flawlessly for me, with docker-squash from master

@twaugh twaugh changed the title [not tested] Add minimal support for docker-py-2.x module Add minimal support for docker-py-2.x module Jul 14, 2017
Signed-off-by: Tim Waugh <twaugh@redhat.com>
@twaugh twaugh merged commit 2014a74 into master Aug 1, 2017
@twaugh twaugh deleted the docker-py-2 branch August 1, 2017 14:38
@twaugh
Copy link
Member Author

twaugh commented Aug 3, 2017

Draft release notes updated.

@maxamillion
Copy link
Contributor

FWIW, this is live in production in Fedora Infra now and it works great.

@twaugh
Copy link
Member Author

twaugh commented Oct 24, 2017

Great! Thanks for letting us know.

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

5 participants