Extend this into a generic 'image publisher', including container images #4

New issue

Closed

opened 2024-05-06 23:03:11 +00:00 by adamwill · 6 comments

adamwill commented

2024-05-06 23:03:11 +00:00

Contributor

So, I have this ticket to replace the bash scripts we're currently using to publish container images to Fedora's registry and quay.io.

But...looking at this thing...it occurs to me, why have two Python thingies for publishing images to registries? Why not just extend this to also handle container images?

A very rough sketch in my head goes something like:

Replace the long if/elif/elif... block in handler.py with something a bit more heavy-duty which maps image properties to subclasses of some kinda "image handler" class
Have an AzureImage class which calls the existing azure() method then runs the playbook, as currently
Add one or more classes for handling container images

If we want to keep the 'generic' playbook handling we could have a generic AnsibleHandler class or something which AzureImage could inherit from, you know, details details. :P I don't know yet whether ansible is a sensible way to publish container images, but I don't think it is - there's https://docs.ansible.com/ansible/latest/collections/community/docker/index.html , but AFAICS that's very specific to the Docker registry...

Before I go any further working on it, WDYT? Does this seem like a reasonable idea or would you rather keep them separate?

CCing @kevin , @siosm , @pbrobinson for reference.

So, I have [this ticket](https://pagure.io/releng/issue/12082) to replace the bash scripts we're currently using to publish container images to Fedora's registry and quay.io. But...looking at this thing...it occurs to me, why have *two* Python thingies for publishing images to registries? Why not just extend this to also handle container images? A very rough sketch in my head goes something like: * Replace the long `if/elif/elif...` block in `handler.py` with something a bit more heavy-duty which maps image properties to subclasses of some kinda "image handler" class * Have an `AzureImage` class which calls the existing `azure()` method then runs the playbook, as currently * Add one or more classes for handling container images If we want to keep the 'generic' playbook handling we could have a generic `AnsibleHandler` class or something which `AzureImage` could inherit from, you know, details details. :P I don't know yet whether ansible is a sensible way to publish container images, but I don't *think* it is - there's https://docs.ansible.com/ansible/latest/collections/community/docker/index.html , but AFAICS that's very specific to the Docker registry... Before I go any further working on it, WDYT? Does this seem like a reasonable idea or would you rather keep them separate? CCing @kevin , @siosm , @pbrobinson for reference.

jcline commented

2024-05-07 17:29:23 +00:00

Owner

I'm fine with it turning into a bit more generic "push compose images around".

I'm a little nervous about getting too complicated with subclasses and all that, but I think we can figure out what makes the most sense as the interface becomes clear. I also don't think every one needs to do an Ansible playbook, if doing pure Python or shelling out to whatever works best, by all means do it.

I'm fine with it turning into a bit more generic "push compose images around". I'm a little nervous about getting too complicated with subclasses and all that, but I think we can figure out what makes the most sense as the interface becomes clear. I also don't think every one needs to do an Ansible playbook, if doing pure Python or shelling out to whatever works best, by all means do it.

adamwill commented

2024-05-07 17:55:03 +00:00

Author

Contributor

I think the right amount of classes just makes things clearer, not more complicated :P long conditionals get painful after a bit. Anyway, I'll try and hack something up and you can see what you think. I meant to work on this today, but the F38 EOL bug warning flood landed so I've been working on those instead, may start work on it later but probably won't be finished for a day or two.

I think the *right amount* of classes just makes things clearer, not more complicated :P long conditionals get painful after a bit. Anyway, I'll try and hack something up and you can see what you think. I meant to work on this today, but the F38 EOL bug warning flood landed so I've been working on those instead, may start work on it later but probably won't be finished for a day or two.

adamwill commented

2024-05-22 23:06:13 +00:00

Author

Contributor

status: jcline merged my refactor (thanks), I am working on adding container functionality on branch https://pagure.io/fork/adamwill/cloud-image-uploader/commits/handle-containers .