-
Notifications
You must be signed in to change notification settings - Fork 168
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
cosalib/container_manifest.py: cleanup manifest before create/push #3952
cosalib/container_manifest.py: cleanup manifest before create/push #3952
Conversation
This was partially exposed because of coreos/fedora-coreos-pipeline#1047 where we're now using the persistent builders for this task. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -19,6 +19,22 @@ def create_local_container_manifest(repo, tag, images) -> dict: | |||
capture_output=True).stdout | |||
return json.loads(manifest_info) | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need an extra line here? CI test fails as two blank lines are required before every function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
if cp.stderr: | ||
print(f" STDERR: {cp.stderr.decode()}") | ||
raise Exception("Error encountered when checking if manifest exists") | ||
return cp.returncode == 0 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be an extra line here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
If a manifest previously exists then let's try to clean it up before continuing. We saw this in the pipeline where one run failed in the middle and then a later run failed because the manifest existed: ``` 2024-11-19 14:29:26,184 INFO - Running command: ['podman', 'manifest', 'create', 'quay.io/coreos-assembler/coreos-assembler:main'] Error: creating manifest: image name "quay.io/coreos-assembler/coreos-assembler:main" is already associated with image "ffbb463bfb0e3bb0fb4c42856d2dca0ff985d1dd5d7b34f1b3ae0277287ad83d": that name is already in use ```
7a53b49
to
9639525
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
If a manifest previously exists then let's try to clean it up before continuing. We saw this in the pipeline where one run failed in the middle and then a later run failed because the manifest existed: