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

Implemented Until Query Parameter for Containers/logs #10868

Merged
merged 1 commit into from
Jul 10, 2021
Merged

Implemented Until Query Parameter for Containers/logs #10868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions libpod/container_log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (c *Container) readFromLogFile(ctx context.Context, options *logs.LogOption
for _, nll := range tailLog {
nll.CID = c.ID()
nll.CName = c.Name()
if nll.Since(options.Since) {
if nll.Since(options.Since) && nll.Until(options.Until) {
logChannel <- nll
}
}
Expand Down Expand Up @@ -88,7 +88,7 @@ func (c *Container) readFromLogFile(ctx context.Context, options *logs.LogOption
}
nll.CID = c.ID()
nll.CName = c.Name()
if nll.Since(options.Since) {
if nll.Since(options.Since) && nll.Until(options.Until) {
logChannel <- nll
}
}
Expand Down
8 changes: 8 additions & 0 deletions libpod/container_log_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption
}
}()

beforeTimeStamp := true
afterTimeStamp := false // needed for options.Since
tailQueue := []*logs.LogLine{} // needed for options.Tail
doTail := options.Tail > 0
Expand Down Expand Up @@ -156,6 +157,13 @@ func (c *Container) readFromJournal(ctx context.Context, options *logs.LogOption
}
afterTimeStamp = true
}
if beforeTimeStamp {
entryTime := time.Unix(0, int64(entry.RealtimeTimestamp)*int64(time.Microsecond))
if entryTime.Before(options.Until) || !options.Until.IsZero() {
continue
}
beforeTimeStamp = false
}

// If we're reading an event and the container exited/died,
// then we're done and can return.
Expand Down
8 changes: 7 additions & 1 deletion libpod/logs/log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type LogOptions struct {
Details bool
Follow bool
Since time.Time
Until time.Time
Tail int64
Timestamps bool
Multi bool
Expand Down Expand Up @@ -184,7 +185,12 @@ func (l *LogLine) String(options *LogOptions) string {

// Since returns a bool as to whether a log line occurred after a given time
func (l *LogLine) Since(since time.Time) bool {
return l.Time.After(since)
return l.Time.After(since) || since.IsZero()
}

// Until returns a bool as to whether a log line occurred before a given time
func (l *LogLine) Until(until time.Time) bool {
return l.Time.Before(until) || until.IsZero()
}

// NewLogLine creates a logLine struct from a container log string
Expand Down
14 changes: 8 additions & 6 deletions pkg/api/handlers/compat/containers_logs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,20 @@ func LogsFromContainer(w http.ResponseWriter, r *http.Request) {

var until time.Time
if _, found := r.URL.Query()["until"]; found {
// FIXME: until != since but the logs backend does not yet support until.
since, err = util.ParseInputTime(query.Until)
if err != nil {
utils.BadRequest(w, "until", query.Until, err)
return
if query.Until != "0" {
until, err = util.ParseInputTime(query.Until)
if err != nil {
utils.BadRequest(w, "until", query.Until, err)
return
}
}
}

options := &logs.LogOptions{
Details: true,
Follow: query.Follow,
Since: since,
Until: until,
Tail: tail,
Timestamps: query.Timestamps,
}
Expand Down Expand Up @@ -119,7 +121,7 @@ func LogsFromContainer(w http.ResponseWriter, r *http.Request) {

for line := range logChannel {
if _, found := r.URL.Query()["until"]; found {
if line.Time.After(until) {
if line.Time.After(until) && !until.IsZero() {
break
}
}
Expand Down
13 changes: 13 additions & 0 deletions test/apiv2/python/rest_api/test_v2_0_0_container.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import random
import unittest
import json

import requests
from dateutil.parser import parse
Expand Down Expand Up @@ -97,6 +98,18 @@ def test_attach(self):
def test_logs(self):
r = requests.get(self.uri(self.resolve_container("/containers/{}/logs?stdout=true")))
self.assertEqual(r.status_code, 200, r.text)
r = requests.post(
self.podman_url + "/v1.40/containers/create?name=topcontainer",
json={"Cmd": ["top", "ls"], "Image": "alpine:latest"},
)
self.assertEqual(r.status_code, 201, r.text)
payload = r.json()
container_id = payload["Id"]
self.assertIsNotNone(container_id)
r = requests.get(self.podman_url + f"/v1.40/containers/{payload['Id']}/logs?follow=false&stdout=true&until=0")
Copy link
Member

Choose a reason for hiding this comment

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

Can you set this up to actually test a log file with entries before and after the until?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think what I just changed gets the job done. I call logs twice once with until=0 and another time with until=1 (arbitrary small value) and compare the length of the logs asserting that 0 should be longer showing that it is working. I am unsure of how to capture and compare the log output more than just the length.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually after messing around with it today I do not think we can parse the logs in these tests since they aren't in the response.

self.assertEqual(r.status_code, 200, r.text)
r = requests.get(self.podman_url + f"/v1.40/containers/{payload['Id']}/logs?follow=false&stdout=true&until=1")
self.assertEqual(r.status_code, 200, r.text)

def test_commit(self):
r = requests.post(self.uri(self.resolve_container("/commit?container={}")))
Expand Down