Add support to interact with a local podman installation #211
Conversation
NotTheEvilOne
force-pushed
the
feature/oci-podman-interaction
branch
2 times, most recently
from
6d29503 to
9aff29a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #211 +/- ##
==========================================
- Coverage 91.54% 91.18% -0.36%
==========================================
Files 42 44 +2
Lines 2070 2372 +302
==========================================
+ Hits 1895 2163 +268
- Misses 175 209 +34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
NotTheEvilOne
force-pushed
the
feature/oci-podman-interaction
branch
from
9aff29a to
41c4a3a
Compare
vivus-ignis
left a comment
vivus-ignis
left a comment
There was a problem hiding this comment.
Please add tests. I've also added some suggestions.
NotTheEvilOne
force-pushed
the
feature/oci-podman-interaction
branch
4 times, most recently
from
5eea9cf to
0f14ce7
Compare
NotTheEvilOne
force-pushed
the
feature/oci-podman-interaction
branch
18 times, most recently
from
4d6fba8 to
01a90ec
Compare
NotTheEvilOne
force-pushed
the
feature/oci-podman-interaction
branch
4 times, most recently
from
677b956 to
0915535
Compare
NotTheEvilOne
force-pushed
the
feature/oci-podman-interaction
branch
12 times, most recently
from
7aa9d6a to
7029e14
Compare
Signed-off-by: Tobias Wolf <wolf@b1-systems.de>
NotTheEvilOne
force-pushed
the
feature/oci-podman-interaction
branch
from
7029e14 to
b9039cc
Compare
Code is now ready for review, I would highly appreciate your feedback @vivus-ignis. |
NotTheEvilOne
requested review from
a team,
ByteOtter,
commel,
vivus-ignis and
yeoldegrove
| #!/usr/bin/env python3 | ||
|
|
||
| """ | ||
| gl-oci main entrypoint |
There was a problem hiding this comment.
Could you please replace this docstring with some useful description of what this script does? Because this text is displayed in click's autogenerated help message. Just a sentence for someone who has no idea about the codebase.
| """ | ||
| Build an OCI container based on the defined `Containerfile`. | ||
| :since: 1.0.0 |
There was a problem hiding this comment.
What is the point in this since metadata line? What this '1.0.0' version relates to?
There was a problem hiding this comment.
Yes, that is obvious, but what '1.0.0' refers to?
src/gardenlinux/oci/__main__.py
Outdated
| ) | ||
| def pull_container(container: str, tag: str, platform: str, insecure: bool) -> None: | ||
| """ | ||
| Push to an OCI registry. |
There was a problem hiding this comment.
"pull" would usually work as well. As we have "push" as well and are able to push multiple data like containers, metadata etc. it seems to be confusing to "pull" something. Therefore "container" has been added here for naming convention reasons as well.
There was a problem hiding this comment.
I mean your function is called "pull_container" but in the docstring you write "Push".
| default=False, | ||
| help="Use HTTP to communicate with the registry", | ||
| ) | ||
| def pull_container(container: str, tag: str, platform: str, insecure: bool) -> None: |
There was a problem hiding this comment.
When I try to run it like this:
$ poetry run gl-oci pull-container --container foo --tag ubuntu:16.04
the output is "None". I think there's a room for improvement for user interaction and error message (I'm not even sure if "None" here is considered an error).
And, as I said, naming needs more attention. Examples in help output would also help.
There was a problem hiding this comment.
This sounds like a bug, will have a look at it.
There was a problem hiding this comment.
Can't reproduce it. I get podman.errors.exceptions.APIError: 500 Server Error: Internal Server Error ({"message":"invalid reference format"}.
Could it be that podman is not installed on your system. If I remember correctly this is catched in the code as well thought. Will dig deeper.
There was a problem hiding this comment.
On a system without podman the error is: FileNotFoundError: [Errno 2] No such file or directory: 'podman'. Please provide more information when you get the output None. Please note that the correct arguments would be:
$ poetry run gl-oci pull-container --container ubuntu --tag 16.04
There was a problem hiding this comment.
Got None if a valid argument is used for pull.
There was a problem hiding this comment.
I think it's better to have no output than print "None" as it is confusing for a user.
Signed-off-by: Tobias Wolf <wolf@b1-systems.de> On-behalf-of: SAP <tobias.wolf@sap.com>
Signed-off-by: Tobias Wolf <wolf@b1-systems.de> On-behalf-of: SAP <tobias.wolf@sap.com>
NotTheEvilOne
force-pushed
the
feature/oci-podman-interaction
branch
from
0dbdc21 to
67d413d
Compare
What this PR does / why we need it:
This PR adds support to interact with a local podman installation.
Which issue(s) this PR fixes:
Closes #199
Closes #200