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.

Sign up for GitHub

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

Jump to bottom

add libimage events #10219

Merged
merged 1 commit into from
May 20, 2021
Merged

add libimage events #10219

merged 1 commit into from
May 20, 2021

Conversation

vrothberg
Copy link
Member

libimage now supports events which libpod.Runtime now uses for image
events.

Signed-off-by: Valentin Rothberg [email protected]

Need to merge containers/common#524 first.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 5, 2021
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 5, 2021
@vrothberg vrothberg mentioned this pull request May 5, 2021
Merged
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2021
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2021
@vrothberg vrothberg changed the title WIP - add libimage events add libimage events May 14, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 14, 2021
@vrothberg
Copy link
Member Author

@containers/podman-maintainers PTAL

@edsantiago PTAL at the system test.

@vrothberg vrothberg force-pushed the image-events branch 2 times, most recently from 1320f67 to 48321ab Compare May 14, 2021 12:01
@vrothberg
Copy link
Member Author

@containers/podman-maintainers PTAL

edsantiago
edsantiago suggested changes May 17, 2021
View reviewed changes
Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

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

Suggestions for test; otherwise LGTM

test/system/090-events.bats Outdated Show resolved Hide resolved
test/system/090-events.bats Outdated Show resolved Hide resolved
test/system/090-events.bats Outdated Show resolved Hide resolved
@vrothberg
Copy link
Member Author

Updated. Thanks, @edsantiago, for your guidance on the tests!

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 17, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [edsantiago,vrothberg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@edsantiago
Copy link
Member

edsantiago commented May 17, 2021 •
edited
Loading

Events seem to be out of order:

# #/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
# #|     FAIL: podman events
# #| expected: '.*image push aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 dir:/tmp/podman_bats.kacprR/dir
# .*image save aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 /tmp/podman_bats.kacprR/ball.tar
# .*image loadfromarchive */tmp/podman_bats.kacprR/ball.tar
# .*image pull *docker-archive:/tmp/podman_bats.kacprR/ball.tar
# .*image tag aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 registry.com/nwowcxnklb
# .*image untag aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 registry.com/nwowcxnklb:latest
# .*image tag aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 registry.com/nwowcxnklb
# .*image remove aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 registry.com/nwowcxnklb.*'
# #|   actual: '2021-05-17 13:17:02.496094502 +0000 UTC image push aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 dir:/tmp/podman_bats.kacprR/dir'
# #|         > '2021-05-17 13:17:02.722939591 +0000 UTC image save aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 /tmp/podman_bats.kacprR/ball.tar'
# #|         > '2021-05-17 13:17:02.903253475 +0000 UTC image loadfromarchive  /tmp/podman_bats.kacprR/ball.tar'
# #|         > '2021-05-17 13:17:03.020686738 +0000 UTC image pull  docker-archive:/tmp/podman_bats.kacprR/ball.tar'
# #|         > '2021-05-17 13:17:03.19948181 +0000 UTC image untag aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 registry.com/nwowcxnklb:latest'
# #|         > '2021-05-17 13:17:03.277832913 +0000 UTC image tag aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 registry.com/nwowcxnklb'
# #|         > '2021-05-17 13:17:03.357413079 +0000 UTC image remove aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 registry.com/nwowcxnklb:latest'
# #|         > '2021-05-17 13:17:03.357449542 +0000 UTC image untag aadc32e2a62610a251826bd9a5a0337bf84f62aeee982cb555ac0fa106c30aa2 registry.com/nwowcxnklb:latest'
# #\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Explanation: expected pull-tag-untag, got pull-untag-tag. Source: ubuntu-2104 but it looks like all system tests failed.

UPDATE: CORRECTION: it's not an out-of-order bug, it's a missing-log bug. There's a missing `tag' log.

@edsantiago
Copy link
Member

The fedora-34-root failure is a missing 'remove' log. I think there's a problem with missing log entries.

@vrothberg
Copy link
Member Author

I have a suspicion: our old friend, the journal. I am forcing to use the file backend for now to see if that resolves the issue. If that does resolve the issue, I am going to open an issue so we get the journal backend fixed (similar to logs).

edsantiago
edsantiago reviewed May 17, 2021
View reviewed changes
test/system/090-events.bats Outdated
run_podman --events-backend=file tag $IMAGE $tag
run_podman --events-backend=file rmi $tag

run_podman events --stream=false --filter type=image --since $t0
Copy link
Member

Choose a reason for hiding this comment

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

I think you're going to need --events-backend=file here too

@edsantiago
Copy link
Member

Yuk. In f34 root there are two missing tag logs.

@vrothberg
Copy link
Member Author

vrothberg commented May 17, 2021 •
edited
Loading

Okay, it also fails with the "file" eventer (tag events not shown). Again it works locally.

This changes my suspicion a bit: it could be a more general issue in the events rather than a bug in a specific driver. Maybe there is a race condition when reading the events in podman events such that we eat certain events?

Edit: Unless others investigate, I will go back to the issue first thing tomorrow morning.

@edsantiago
Copy link
Member

In other logs (sorry, I don't feel like linking) I'm seeing missing remove... which leads me to wonder if your skip_if_remote is truly needed, or if that was just an early indicator of the problem we're seeing now? Anyhow, it seems indisputable that there are missing log entries no matter which log driver is used.

@vrothberg
Copy link
Member Author

It seems like the problem is not the reading but the writing of events. I debugged in a CI VM and noticed that some events are not showing up in the file.

vrothberg added a commit to vrothberg/common that referenced this pull request May 18, 2021
@vrothberg
libimage: block on events
07bae5a 
Debugging containers/podman/pull/10219 has revealed a number of issues
with respect to the consistency of events.  At the core, some events may
not be written to the backend when the runtime is shutdown.

While there are various changes required to tackle the consistency
issues, a first step is to make sure that writes to the events channel
block.

Set the timeout to 10 seconds.

Signed-off-by: Valentin Rothberg <[email protected]>
@vrothberg vrothberg mentioned this pull request May 18, 2021
Closed
@vrothberg
Copy link
Member Author

A first step is containers/common#561 but it looks like there are still consistency issue on the Podman side.

@edsantiago
Copy link
Member

@vrothberg needs a whitespace fix

@vrothberg vrothberg changed the title add libimage events WIP - add libimage events May 18, 2021
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 18, 2021
@vrothberg vrothberg force-pushed the image-events branch 4 times, most recently from 05be398 to 51ec4a8 Compare May 20, 2021 10:31
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 20, 2021
@vrothberg
add libimage events
8352e5b 
libimage now supports events which `libpod.Runtime` now uses for image
events.

Signed-off-by: Valentin Rothberg <[email protected]>
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 20, 2021
@vrothberg vrothberg changed the title WIP - add libimage events add libimage events May 20, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 20, 2021
@vrothberg
Copy link
Member Author

@containers/podman-maintainers PTAL

@rhatdan
Copy link
Member

rhatdan commented May 20, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 20, 2021
@openshift-merge-robot openshift-merge-robot merged commit 7bcec86 into containers:master May 20, 2021
@vrothberg vrothberg deleted the image-events branch May 20, 2021 13:50
This was referenced May 20, 2021
Closed
Closed
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@edsantiago edsantiago edsantiago approved these changes

Assignees

@rhatdan rhatdan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vrothberg @openshift-ci-robot @edsantiago @rhatdan @TomSweeneyRedHat @openshift-merge-robot