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

Make push-container-manifest idempotent #3150

Closed
dustymabe opened this issue Oct 28, 2022 · 8 comments
Closed

Make push-container-manifest idempotent #3150

dustymabe opened this issue Oct 28, 2022 · 8 comments
Assignees

Comments

@dustymabe
Copy link
Member

We want to be able to run the release job multiple times (in case one run fails in an intermediate stage) and have each stage no-op if there is nothing to do (i.e. don't recreate things that already exist).

We need to update push-container-manifest to do some check here and make sure we don't push a new manifest listed container if the one that is there is correct already. We'd probably make an update somewhere in this area of the code.

I'm thinking we can do something like this:

  • grab the digest from the meta.json for each arch of the toplevel entry for this container (if it exists)
  • if all arches have an entry with a digest in meta.json (implies upload already happened)
    • check the remote container manifest list and see if it already matches each arch digest
      • if it does, do nothing, else continue push operation

When inspecting the remote container we can get the digests by doing something like:

skopeo inspect --raw docker://quay.io/fedora/fedora-coreos:stable | jq .
@jlebon
Copy link
Member

jlebon commented Oct 28, 2022

Some things I'd add to the above:

  1. Very early on, check if meta.json already has the top-level key we're going to insert. If so, no-op.
  2. Add a --force switch to override that check and also the digest check described above.

This makes it more similar to the semantics of other cosa commands.

@ravanelli ravanelli self-assigned this Oct 28, 2022
@dustymabe
Copy link
Member Author

dustymabe commented Oct 28, 2022

  1. Very early on, check if meta.json already has the top-level key we're going to insert. If so, no-op.

Oh right. None of them will have the top-level key if the operation failed at all. So yes, we can just check that the toplevel keys exist (in the arch meta.json files) and then no-op out of it. It makes the check much more simple.

@jlebon
Copy link
Member

jlebon commented Oct 28, 2022

I think we still also want what you originally described though in case the release job failed before we were actually able to re-upload the updated meta.json. We would no-op the upload, but not the meta.json update.

@cgwalters
Copy link
Member

(Makes sense but note with #2685 this issue goes away, because the container is the root, the axis around which disk images revolve, the source of truth, and not just another thing pushed out)

@dustymabe
Copy link
Member Author

We're talking about more than one container here.

@cgwalters
Copy link
Member

The extensions container? Yeah, true. Though not for FCOS. And hopefully at some point we can kill that for RHCOS too.

@dustymabe
Copy link
Member Author

We'll have a kubevirt container pretty soon too.

@cgwalters
Copy link
Member

Yeah, makes sense; we do want idempotence for that indeed.

ravanelli added a commit to ravanelli/coreos-assembler that referenced this issue Nov 1, 2022
 - Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
ravanelli added a commit to ravanelli/coreos-assembler that referenced this issue Nov 1, 2022
 - Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
ravanelli added a commit to ravanelli/coreos-assembler that referenced this issue Nov 1, 2022
 - Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
ravanelli added a commit to ravanelli/coreos-assembler that referenced this issue Nov 1, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
ravanelli added a commit to ravanelli/coreos-assembler that referenced this issue Nov 1, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
ravanelli added a commit to ravanelli/coreos-assembler that referenced this issue Nov 3, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
ravanelli added a commit to ravanelli/coreos-assembler that referenced this issue Nov 4, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
ravanelli added a commit to ravanelli/coreos-assembler that referenced this issue Nov 4, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
@jlebon jlebon closed this as completed in bfbbe41 Nov 4, 2022
jlebon pushed a commit to jlebon/coreos-assembler that referenced this issue Nov 7, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
jlebon pushed a commit to jlebon/coreos-assembler that referenced this issue Nov 8, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
dustymabe pushed a commit that referenced this issue Nov 9, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes #3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
dustymabe pushed a commit that referenced this issue Nov 9, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes #3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
jlebon pushed a commit to dustymabe/coreos-assembler that referenced this issue Nov 10, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
(cherry picked from commit 3945a7c)
jlebon pushed a commit to dustymabe/coreos-assembler that referenced this issue Nov 10, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
(cherry picked from commit 3945a7c)
jlebon pushed a commit to dustymabe/coreos-assembler that referenced this issue Nov 10, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
(cherry picked from commit 3945a7c)
dustymabe pushed a commit to dustymabe/coreos-assembler that referenced this issue Nov 10, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
(cherry picked from commit 3945a7c)
dustymabe pushed a commit to dustymabe/coreos-assembler that referenced this issue Nov 13, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
(cherry picked from commit 3945a7c)
dustymabe pushed a commit to dustymabe/coreos-assembler that referenced this issue Nov 15, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
(cherry picked from commit 3945a7c)
dustymabe pushed a commit that referenced this issue Nov 15, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes #3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
(cherry picked from commit 3945a7c)
dustymabe pushed a commit to dustymabe/coreos-assembler that referenced this issue Dec 1, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes coreos#3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
dustymabe pushed a commit that referenced this issue Dec 2, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes #3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
jlebon pushed a commit that referenced this issue Dec 2, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes #3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
jlebon pushed a commit that referenced this issue Dec 2, 2022
Check if the digests are already present in the remote
manifest and if you want to force or not a new manifest upload.

Fixes #3150

Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
(cherry picked from commit bfbbe41)

jlebon: Had to modify new code to use `runcmd` from `cosalib.utils.`.
(cherry picked from commit 9d3bdd9)
(cherry picked from commit 3945a7c)
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

No branches or pull requests

4 participants