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

Fix docker swarm mode #227

Merged
merged 3 commits into from
Feb 20, 2019
Merged

Fix docker swarm mode #227

merged 3 commits into from
Feb 20, 2019

Conversation

mathcantin
Copy link
Contributor

Fixes #216

The fix is instead of using service.update(image=tag) we need to use service.update(image=f"{tag}@sha256:{latest_image_sha256}").

With a little bit of refractor to remove duplicate code.

@mathcantin
Copy link
Contributor Author

On the issues, there are this comment

as another way to look at it he is saying it tries to update something that doesnt need updating, which is odd since its checking the sha when it DOES exist initially. Maybe the root can be found there

Still a good idea, but that not cover by this PR.

@dirtycajunrice
Copy link
Member

have you ran this code? i like the baseobject but a lot of stuff just wont work. e.g. digest or something doesnt return digest or something. it returns true or false

@mathcantin
Copy link
Contributor Author

Yes, I try with and without swarm mode and that works. The method _get_digest, return the sha256 of the image, in all cases (dryrun on or off with auth or not). I don't know where you see a method that return a boolean.

@mathcantin
Copy link
Contributor Author

mathcantin commented Feb 19, 2019

I understand, the code:

        digest = image.attrs.get("Descriptor", {}).get("digest")
        digest = digest or image.attrs.get("RepoDigests")[0].split('@')[1]
        digest = digest or image.id

        return self._remove_sha_prefix(digest)

is exactly the same thing then that

    digest = image.attrs.get("Descriptor", {}).get("digest")
    if not digest:
            digest = image.attrs.get("RepoDigests")[0].split('@')[1]
    if not digest
        digest = image.id

    return self._remove_sha_prefix(digest)

Because the or keyword isn't a comparator and don't return True or False. I can change it, if you prefer the second solution.

@dirtycajunrice
Copy link
Member

dirtycajunrice commented Feb 19, 2019

Yeah i spoke before i ran the code myself. its completely fine. Im reading over the change right now. Give me a bit its a large change.

Copy link
Member

@dirtycajunrice dirtycajunrice left a comment

Choose a reason for hiding this comment

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

Awaiting feedback

pyouroboros/dockerclient.py Outdated Show resolved Hide resolved
pyouroboros/dockerclient.py Show resolved Hide resolved
@dirtycajunrice dirtycajunrice merged commit 549fc9a into pyouroboros:develop Feb 20, 2019
@dirtycajunrice dirtycajunrice added the bug Something isn't working label Feb 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants