feat: label-based filtering added to nerdctl events#4999
Conversation
|
|
||
| eOut := EventOut{e.Timestamp, id, e.Namespace, e.Topic, TopicToStatus(e.Topic), string(out)} | ||
| if id != "" { | ||
| container, err := client.ContainerService().Get(ctx, id) |
There was a problem hiding this comment.
probably can improve like
if container, err := client.ConatinerService.Get(ctx, id); err == nil {
labels = container.Labels
}
There was a problem hiding this comment.
|
@AkihiroSuda there are so many failed checks and after surfacing little bit I strongly feels they are flaky and not related to the changes in this PR. |
|
|
||
| eOut := EventOut{e.Timestamp, id, e.Namespace, e.Topic, TopicToStatus(e.Topic), string(out)} | ||
| if id != "" { | ||
| if container, err := client.ContainerService().Get(ctx, id); err == nil { |
There was a problem hiding this comment.
the err is ignored here may be good idea to add a log
|
Please squash the commits and rebase with the current main branch |
beb85a4 to
e362fdd
Compare
|
I squashed commits and rebased on main then force pushed. |
https://github.com/containerd/nerdctl/actions/runs/28356414001/job/84140963740?pr=4999 The tests are failing with Docker v28. Please try rebasing after we merge:
|
e362fdd to
9992dfa
Compare
|
Copilot telling the reason is this:
so what I did: $ git diff this gives container enough time to live. Run test, then cleanup. @AkihiroSuda is this a right direction I am following? |
9992dfa to
2620492
Compare
Don't you need some |
2620492 to
b35453e
Compare
|
I think the test is failing because it iss running against Docker host rather than nerdctl. Would it make sense to add |
Any reason not to make it Docker-compatible? |
b35453e to
bb6a8c1
Compare
I apologize!😅 I think I managed to solve that particular test! |
There was a problem hiding this comment.
Pull request overview
Adds label-based filtering support to nerdctl events by enriching incoming events with container labels and extending the filter engine to match label= filters, along with integration coverage for the new behavior.
Changes:
- Extend event filtering to support
--filter label=key[=value]. - Resolve container labels for events (via container metadata lookup) and attach them to the internal event model used for filtering/formatting.
- Add Linux integration tests validating label filtering (key/value and key-only).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/cmd/system/events.go | Adds Labels to event output model, implements LABEL filter logic, and retrieves container labels during event processing. |
| cmd/nerdctl/system/system_events_linux_test.go | Adds integration tests covering label-based event filtering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case "LABEL": | ||
| return func(e *EventOut) bool { | ||
| if len(e.Labels) == 0 { | ||
| return false | ||
| } | ||
|
|
||
| parts := strings.SplitN(filterValue, "=", 2) | ||
| if len(parts) == 1 { | ||
| _, ok := e.Labels[parts[0]] | ||
| return ok | ||
| } | ||
|
|
||
| return e.Labels[parts[0]] == parts[1] | ||
| }, nil |
| @@ -197,8 +213,14 @@ | |||
| id = data["container_id"].(string) | |||
| } | |||
| } | |||
|
|
|||
| eOut := EventOut{e.Timestamp, id, e.Namespace, e.Topic, TopicToStatus(e.Topic), string(out)} | |||
| if id != "" { | |||
| if id != "" { | ||
| if container, err := client.ContainerService().Get(ctx, id); err != nil { | ||
| log.G(ctx).WithError(err).WithField("containerID", id).Debug("failed to retrieve container labels") | ||
| } else { | ||
| labels = container.Labels | ||
| } | ||
| } |
Signed-off-by: Mujib Ahasan <ahasanmujib8@gmail.com> test case added Signed-off-by: Mujib Ahasan <ahasanmujib8@gmail.com> events: log errors when retrieving container labels Signed-off-by: Mujib Ahasan <ahasanmujib8@gmail.com>
bb6a8c1 to
c76a63b
Compare
Description
This PR adds support for label-based filtering in nerdctl events. Users can now filter the event stream using container labels, for example:
Fixes: #4690
The implementation resolves container metadata from incoming events and matches the configured label filter before displaying the event. This helps users focus on events related to a specific workload in environments with so many running containers.