api_service: Implement GetLogs RPC request #2662

iaguis · 2016-05-19T09:49:42Z

Allows being able to get the logs of a pod with the API service without forking off journalctl.

Fixes #1970

jonboulle · 2016-05-19T11:02:20Z

.travis.yml

@@ -1,6 +1,8 @@
 # Configures rkt tests at Travis CI (https://travis-ci.org).

 language: go
+sudo: required


is this necessary given we were already sudoing things?

I was just following https://docs.travis-ci.com/user/trusty-ci-environment/

iaguis · 2016-05-20T10:33:45Z

Updated.

iaguis · 2016-05-23T09:55:58Z

PTAL.

alban · 2016-05-24T13:51:38Z

Godeps/Godeps.json

@@ -236,23 +236,28 @@
 		},
 		{
 			"ImportPath": "github.com/coreos/go-systemd/activation",
-			"Comment": "v5",
-			"Rev": "7b2428fec40033549c68f54e26e89e7ca9a9ce31"
+			"Comment": "v6-4-g0f21494",


We need a tagged version.

@lucab I see you tagged the last version of go-systemd. Could you tag yet another version for this PR?

Tagged: https://github.com/coreos/go-systemd/releases/tag/v7

iaguis · 2016-05-25T08:47:04Z

Updated

jellonek · 2016-05-25T10:31:11Z

Will this work on systems without systemd/journald?

iaguis · 2016-05-25T10:45:26Z

Will this work on systems without systemd/journald?

Nope

jellonek · 2016-05-25T10:48:10Z

ack.

euank · 2016-05-25T19:03:20Z

rkt/api_service.go

+		}
+		jconf.Matches = []sdjournal.Match{
+			{
+				Field: sdjournal.SD_JOURNAL_FIELD_SYSTEMD_UNIT,


Susceptible to the race mentioned in #2630? Should this use the SYSLOG_IDENTIFIER instead?

yep, forgot to update that after #2630

coreos/go-systemd#161

iaguis · 2016-05-26T13:03:11Z

Updated. Pending on a go-systemd release.

jonboulle · 2016-05-26T13:04:33Z

I assigned on to @lucab

On Thu, May 26, 2016 at 3:03 PM, Iago López Galeiras <
notifications@github.com> wrote:

Updated. Pending on a go-systemd release.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#2662 (comment)

lucab · 2016-05-26T15:20:30Z

Tagged go-systemd v8: https://github.com/coreos/go-systemd/releases/tag/v8

iaguis · 2016-05-26T15:28:14Z

Tagged go-systemd v8: https://github.com/coreos/go-systemd/releases/tag/v8

Thanks!

euank · 2016-05-27T03:22:20Z

rkt/api_service.go

+	}
+
+	s.mu.Lock()
+	jr, ok := s.journalReaders[request.PodId]


Since jconf is where all request configuration ends up, but isn't used here, would recycling a reader lose that configuration?

e.g. I call once with tail=10 and a reader is created and cached with that configuration, and then I call again with tail=1000 and still only get 10 lines because the cached journalReader has that config.

You're right. I think we should go back to one JournalReader per call... @jonboulle

alban · 2016-05-27T08:28:40Z

Can we have tests? :)
In a follow-up issue?

iaguis · 2016-05-27T08:30:28Z

Can we have tests? :)
In a follow-up issue?

Definitely

iaguis · 2016-05-27T09:31:22Z

I'll revert kinvolk@e1fc014

iaguis · 2016-05-27T09:33:04Z

Updated.

alban · 2016-05-27T12:06:51Z

api/v1alpha/client_example.go

+		})
+		if err != nil {
+			fmt.Println(err)
+			os.Exit(2)


Dunno, this stupid error numbering. I guess it should be 3, and then 4, and then 5. I can just get rid of those like we did with init.go some time ago

yeah, sequential numbering never works, settle on 1 unless you wanted to
try use something like http://linux.die.net/include/sysexits.h

On Fri, May 27, 2016 at 2:21 PM, Iago López Galeiras <
notifications@github.com> wrote:

In api/v1alpha/client_example.go
#2662 (comment):

@@ -51,6 +51,24 @@ func main() {

for _, p := range podResp.Pods { fmt.Printf("Pod %q is running\n", p.Id)

logsResp, err := c.GetLogs(context.Background(), &v1alpha.GetLogsRequest{

PodId: p.Id,

})

if err != nil {

fmt.Println(err)

os.Exit(2)

Dunno, this stupid error numbering. I guess it should be 3, and then 4,
and then 5. I can just get rid of those like we did with init.go some time
ago

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/coreos/rkt/pull/2662/files/c28aa39ba2c92941f58487762ed66bf9e93d0d01..cc72b0fb026b3fb54a9a9bb24e29f5ed86f8df50#r64896509,
or mute the thread
https://github.com/notifications/unsubscribe/ACewN0s-tIsx_Ge3uE7wnpSOHppyL1lMks5qFuHKgaJpZM4IiF05
.

Adds new dep on coreos/pkg

Fixes rkt#1970

To avoid the race mentioned in rkt#2630.

Sequential numbering of errors is evil.

iaguis · 2016-05-27T12:37:39Z

Updated

alban · 2016-05-27T12:40:24Z

LGTM if green.

iaguis added priority/P0 component/api server area/api technology/kubernetes required-by/rktnetes labels May 19, 2016

iaguis added this to the v1.7.0 milestone May 19, 2016

iaguis mentioned this pull request May 19, 2016

Get logs #2041

Closed

iaguis force-pushed the iaguis/get-logs-dlopen branch 5 times, most recently from 164701a to 4c8103a Compare May 19, 2016 10:26

jonboulle reviewed May 19, 2016
View reviewed changes

iaguis force-pushed the iaguis/get-logs-dlopen branch from 4c8103a to 1981c9f Compare May 20, 2016 10:32

iaguis added do not merge needs/review labels May 20, 2016

philips added priority/P1 and removed priority/P0 labels May 24, 2016

alban reviewed May 24, 2016
View reviewed changes

iaguis force-pushed the iaguis/get-logs-dlopen branch from 1981c9f to bc21b6b Compare May 25, 2016 08:46

iaguis removed the do not merge label May 25, 2016

iaguis force-pushed the iaguis/get-logs-dlopen branch from bc21b6b to 051bd6b Compare May 25, 2016 09:55

euank reviewed May 25, 2016
View reviewed changes

euank added needs/some-changes and removed needs/review labels May 25, 2016

iaguis force-pushed the iaguis/get-logs-dlopen branch from bd65c46 to f0cea2e Compare May 26, 2016 13:03

iaguis force-pushed the iaguis/get-logs-dlopen branch from f0cea2e to 7ada855 Compare May 26, 2016 15:27

euank reviewed May 27, 2016
View reviewed changes

iaguis force-pushed the iaguis/get-logs-dlopen branch from 7ada855 to 06781b1 Compare May 27, 2016 09:33

alban reviewed May 27, 2016
View reviewed changes

iaguis and others added 6 commits May 27, 2016 14:36

*: add libsystemd-journal-dev dependency

96f418f

Godeps: update go-systemd

68e1b0f

Adds new dep on coreos/pkg

api_service: Implement GetLogs RPC request

44fbaba

Fixes rkt#1970

api: add getlogs to the client example

448dd54

api_service: SYSTEMD_UNIT -> SYSLOG_IDENTIFIER

9964893

To avoid the race mentioned in rkt#2630.

api: exit with status 1 on error

d0a749a

Sequential numbering of errors is evil.

iaguis force-pushed the iaguis/get-logs-dlopen branch from 06781b1 to d0a749a Compare May 27, 2016 12:37

alban removed the needs/some-changes label May 27, 2016

jonboulle unassigned yifan-gu May 27, 2016

iaguis merged commit 1ebb525 into rkt:master May 27, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api_service: Implement GetLogs RPC request #2662

api_service: Implement GetLogs RPC request #2662

iaguis commented May 19, 2016 •

edited

jonboulle May 19, 2016

iaguis May 19, 2016

iaguis commented May 20, 2016

iaguis commented May 23, 2016

alban May 24, 2016

iaguis May 24, 2016

alban May 24, 2016

lucab May 24, 2016

iaguis commented May 25, 2016

jellonek commented May 25, 2016

iaguis commented May 25, 2016

jellonek commented May 25, 2016

euank May 25, 2016

iaguis May 26, 2016

iaguis May 26, 2016

iaguis commented May 26, 2016

jonboulle commented May 26, 2016

lucab commented May 26, 2016

iaguis commented May 26, 2016

euank May 27, 2016

iaguis May 27, 2016 •

edited by alban

alban commented May 27, 2016

iaguis commented May 27, 2016

iaguis commented May 27, 2016

iaguis commented May 27, 2016

alban May 27, 2016

iaguis May 27, 2016

jonboulle May 27, 2016

iaguis commented May 27, 2016

alban commented May 27, 2016

api_service: Implement GetLogs RPC request #2662

api_service: Implement GetLogs RPC request #2662

Conversation

iaguis commented May 19, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iaguis commented May 20, 2016

iaguis commented May 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iaguis commented May 25, 2016

jellonek commented May 25, 2016

iaguis commented May 25, 2016

jellonek commented May 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iaguis commented May 26, 2016

jonboulle commented May 26, 2016

lucab commented May 26, 2016

iaguis commented May 26, 2016

Choose a reason for hiding this comment

iaguis May 27, 2016 • edited by alban

Choose a reason for hiding this comment

alban commented May 27, 2016

iaguis commented May 27, 2016

iaguis commented May 27, 2016

iaguis commented May 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iaguis commented May 27, 2016

alban commented May 27, 2016

iaguis commented May 19, 2016 •

edited

iaguis May 27, 2016 •

edited by alban