tarmak cluster logs #575

JoshVanL · 2018-10-12T16:06:58Z

What this PR does / why we need it:
This adds a new cluster sub-command logs. The following arguments will be taken as instance pools. This sub-command will go and fetch the logs from every service in each instance of the instance pool for every instance pool given. The logs will then be bundled together into a tar ball for convenient file sharing.

fixes #574

Adds `tarmak cluster logs` for convenient fetching of logs of any instance or target group

/assign
TODO:

fix cmd hang
more efficiency

JoshVanL · 2018-10-12T16:31:36Z

/cc @MattiasGees @alljames @simonswine
Any ideas on how to keep the ssh connection open to improve the speed of fetching logs?
Thoughts on current implementation?

MattiasGees

In general I like this idea that you try to fetch all services that way we never miss anything. But I feel this let us transfer a lot of logs that we don't need. A predefined list will lower the amount of logs, but it also adds the burden that we have to maintain that list if things change.

A few things I like to see changed.

Make sure the extension of the log files end with .log
Now we fetch all the logs over one separate ssh connection. I propose to do the following steps. Disadvantage will be that we need to use local space on the instances.
1. Select services from which we get the logs
2. Save all logs to local disk in one temp folder
3. Get all logs from local disk by downloading that folder
Add extra arguments to get logs from/between certain dates. journalctl has support for --since and --until. I think this can be valuable. I think by default we should only get the last 24 hours of logs. Otherwise they can become big.

Also I get an error when I try to get logs from the worker instance group which has multiple instances.

tarmak cluster logs --path ~/Downloads/logs.tar.gz worker

INFO[0001] fetching service logs from instance 'worker'  app=tarmak
ssh: Could not resolve hostname worker: nodename nor servname provided, or not known
ERRO[0001] failed to gather unit service list from instance 'worker', skipping...  app=tarmak
INFO[0001] fetching service logs from instance 'worker'  app=tarmak
ssh: Could not resolve hostname worker: nodename nor servname provided, or not known
ERRO[0001] failed to gather unit service list from instance 'worker', skipping...  app=tarmak
INFO[0001] fetching service logs from instance 'worker'  app=tarmak
ssh: Could not resolve hostname worker: nodename nor servname provided, or not known
ERRO[0001] failed to gather unit service list from instance 'worker', skipping...  app=tarmak
INFO[0001] logs bundle written to '/Users/mattias/Downloads/logs.tar.gz'  app=tarmak
INFO[0001] Tarmak performed all tasks successfully.

MattiasGees · 2018-10-16T13:05:35Z

cmd/tarmak/cmd/cluster_logs.go

+
+// clusterLogsCmd handles `tarmak clusters logs`
+var clusterLogsCmd = &cobra.Command{
+	Use:   "logs",


Change to logs [INSTANCE POOL] to say we need to define an instance pool.

MattiasGees · 2018-10-16T13:15:12Z

cmd/tarmak/cmd/cluster_logs.go

+			t.Log().Fatal("expecting at least a one instance pool name")
+		}
+
+		t.CancellationContext().WaitOrCancel(t.NewCmdTerraform(args).Logs)


Why NewCmdTerraform? I don't feel this belongs here as the logs have nothing to do with Terraform.

I agree however neither is SSH(). I like using this struct for consistency and using the context etc. Perhaps we should rename it to CmdTarmak however this doesn't make much sense being in terraform.go
Thoughts?

JoshVanL · 2018-10-17T15:39:52Z

Sane defaults
- Retrieve logs for 24h
- Control plan + Etcds + Bastion + Vault default targets
Look into the rotation time of journald
Allow specifc instance or instances (similar name like in cluster ssh instance)
Use a single journalctl -o json stream per instance
Filter then per service / kernel / target on the tarmak host

JoshVanL · 2018-10-18T16:12:03Z

/unassign
/assign @simonswine @MattiasGees

Logs are now json streamed and piped straight to file to be bundled together.
All hosts in a group are gathered concurrently.
Sane defaults for since + until.

simonswine

Thanks for that feature @JoshVanL

Some minor comments, but in general great work!

/assign @JoshVanL
/unassign

simonswine · 2018-10-23T10:42:18Z

pkg/tarmak/logs/logs.go

+	until   string
+	targets []string
+
+	mu       sync.Mutex


Make clearer what that lock is protecting

in its name also a comment

simonswine · 2018-10-23T10:42:52Z

pkg/tarmak/logs/logs.go

+	wg       sync.WaitGroup
+	hosts    []interfaces.Host
+	tmpDir   string
+	tmpFiles map[string]*os.File


I think you should make sure that this has it's own lock protecting read/writes to the map

pkg/tarmak/logs/logs.go

simonswine · 2018-10-23T10:52:52Z

pkg/tarmak/logs/logs.go

+		}
+
+		entry := new(SystemdEntry)
+		if err := json.Unmarshal(r, entry); err != nil {


use json.NewDecoder and a decode loop instead of a scanner + individual decodes

simonswine · 2018-10-23T10:54:16Z

pkg/tarmak/logs/logs.go

+			l.mu.Lock()
+			err = fmt.Errorf("failed to unmarshal entry [%s]: %s", r, err)
+			result = multierror.Append(result, err)
+			l.mu.Unlock()


Keep the are with the lock separate from other code (new lines before and after). also keep it minimal (i.e.) without the Errorf

simonswine · 2018-10-23T10:56:33Z

cmd/tarmak/cmd/cluster.go

+		"path",
+		utils.DefaultLogsPathPlaceholder,
+		"target tar ball path",
+	)


Maybe we default to the current working directory, config files for tarmak should not be polluted

yeah I also think current working directory is a better solution.

simonswine · 2018-10-23T10:57:24Z

cmd/tarmak/cmd/cluster_logs.go

+	),
+	Short: "Gather logs from a list of instances or target groups",
+	PreRunE: func(cmd *cobra.Command, args []string) error {
+		if len(args) == 0 {


Some better validation of targets would be great.

I've done more validation further down. I don't want to call ListHosts() here again because it's so expensive

simonswine · 2018-10-23T10:58:05Z

pkg/tarmak/logs/logs.go

+
+	switch group {
+	case "control-plane":
+		l.targets = []string{"master", "etcd"}


Maybe we define vault as being part of the control-plane?

alljames · 2018-10-23T14:38:34Z

pkg/tarmak/interfaces/interfaces.go

+}
+
+type Logs interface {
+	Gather(group string, flags tarmakv1alpha1.ClusterLogsFlags) error


or Aggregate ?

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

JoshVanL · 2019-02-01T11:10:50Z

/test verify quick
/test verify docs

JoshVanL · 2019-02-01T11:15:23Z

/test verify quick

JoshVanL · 2019-02-01T11:18:11Z

/test verify quick

simonswine · 2019-02-04T14:16:27Z

I think almost there, i find this behaviour odd:

./_output/tarmak cluster logs --path test.tar.gz master vault                                                                                                                                                                                     1 ↵
INFO[0000] fetching logs from target 'master'            app=tarmak
INFO[0001] fetching journald logs from instance 'master-3'  app=tarmak
INFO[0001] fetching journald logs from instance 'master-1'  app=tarmak
INFO[0001] fetching journald logs from instance 'master-2'  app=tarmak
INFO[0001] new connection to bastion host successful     app=tarmak
INFO[0004] logs bundle written to '/home/simon/.golang/packages/src/github.com/jetstack/tarmak/test.tar.gz'  app=tarmak
INFO[0004] fetching logs from target 'vault'             app=tarmak
INFO[0004] fetching journald logs from instance 'vault-3'  app=tarmak
INFO[0004] fetching journald logs from instance 'vault-2'  app=tarmak
INFO[0004] fetching journald logs from instance 'vault-1'  app=tarmak
INFO[0005] logs bundle written to '/home/simon/.golang/packages/src/github.com/jetstack/tarmak/test.tar.gz'  app=tarmak
INFO[0005] tarmak performed all tasks successfully

They should not overwrite each other, i guess

control-plane and master etcd vault should do the exact same

simonswine · 2019-02-04T14:16:37Z

/assign @JoshVanL

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

JoshVanL · 2019-02-04T18:20:35Z

/unassign

simonswine · 2019-02-06T12:06:49Z

Thanks @JoshVanL, looking good now

/approve
/lgtm

jetstack-bot · 2019-02-06T12:07:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoshVanL, MattiasGees, simonswine

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~~ [JoshVanL,simonswine]

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

jetstack-bot assigned JoshVanL Oct 12, 2018

jetstack-bot requested a review from kragniz October 12, 2018 16:07

jetstack-bot requested review from alljames, MattiasGees and simonswine October 12, 2018 16:31

MattiasGees suggested changes Oct 16, 2018

View reviewed changes

JoshVanL force-pushed the 574-tarmak-cluster-logs branch from af51d76 to 76fde99 Compare October 17, 2018 15:40

jetstack-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 18, 2018

jetstack-bot assigned MattiasGees and simonswine and unassigned JoshVanL Oct 18, 2018

JoshVanL changed the title ~~WIP: tarmak cluster logs~~ tarmak cluster logs Oct 18, 2018

jetstack-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 18, 2018

MattiasGees mentioned this pull request Oct 23, 2018

Clean-up when destroying environment #527

Merged

simonswine reviewed Oct 23, 2018

View reviewed changes

jetstack-bot assigned JoshVanL and unassigned simonswine Oct 23, 2018

simonswine unassigned MattiasGees Oct 23, 2018

jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2018

alljames reviewed Oct 23, 2018

View reviewed changes

JoshVanL force-pushed the 574-tarmak-cluster-logs branch 2 times, most recently from f38a295 to 14c3df8 Compare October 23, 2018 16:45

jetstack-bot assigned simonswine Feb 1, 2019

JoshVanL added 15 commits February 1, 2019 11:07

Adds tarmak cluster logs

c9a83d2

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Fix cmd from hanging

109a5e4

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Gather all instance logs from single json stream

541913e

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Output logs matches journalctl format

e1c6449

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Gather logs concurrently and handle signals

63c12cb

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Fetch non service logs

03b3fb2

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Add time scale to logs - default last 24h

e174b25

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Adds target groups to choose

5d048cf

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Pipe logs straight to file to support large sizes

5d6f382

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Update reference docs

3be5cf4

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Adds review changes

cf04ba1

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Updates generated cmd and api docs

5c794bb

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Removes ErrorOrNil() to remove confusion

75ffe84

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Improve ssh Execute signals

b899d01

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

Better aggregate logs of the same service

2800a68

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

jetstack-bot assigned JoshVanL Feb 4, 2019

Decrease log output noise and properly group logs

9b792e7

Signed-off-by: JoshVanL <vleeuwenjoshua@gmail.com>

JoshVanL force-pushed the 574-tarmak-cluster-logs branch from 0b9a084 to 9b792e7 Compare February 4, 2019 18:19

jetstack-bot unassigned JoshVanL Feb 4, 2019

jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 6, 2019

jetstack-bot merged commit 463b70c into jetstack:master Feb 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tarmak cluster logs #575

tarmak cluster logs #575

JoshVanL commented Oct 12, 2018 •

edited

Loading

JoshVanL commented Oct 12, 2018

MattiasGees left a comment

MattiasGees Oct 16, 2018

MattiasGees Oct 16, 2018

JoshVanL Oct 17, 2018

JoshVanL commented Oct 17, 2018

JoshVanL commented Oct 18, 2018

simonswine left a comment

simonswine Oct 23, 2018

simonswine Oct 23, 2018

simonswine Oct 23, 2018

simonswine Oct 23, 2018

simonswine Oct 23, 2018

simonswine Oct 23, 2018

MattiasGees Oct 23, 2018

alljames Oct 23, 2018

simonswine Oct 23, 2018

JoshVanL Oct 23, 2018

simonswine Oct 23, 2018

alljames Oct 23, 2018

JoshVanL commented Feb 1, 2019

JoshVanL commented Feb 1, 2019

JoshVanL commented Feb 1, 2019

simonswine commented Feb 4, 2019

simonswine commented Feb 4, 2019

JoshVanL commented Feb 4, 2019

simonswine commented Feb 6, 2019

jetstack-bot commented Feb 6, 2019

tarmak cluster logs #575

tarmak cluster logs #575

Conversation

JoshVanL commented Oct 12, 2018 • edited Loading

JoshVanL commented Oct 12, 2018

MattiasGees left a comment

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

JoshVanL commented Oct 17, 2018

JoshVanL commented Oct 18, 2018

simonswine left a comment

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

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

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

Choose a reason for hiding this comment

JoshVanL commented Feb 1, 2019

JoshVanL commented Feb 1, 2019

JoshVanL commented Feb 1, 2019

simonswine commented Feb 4, 2019

simonswine commented Feb 4, 2019

JoshVanL commented Feb 4, 2019

simonswine commented Feb 6, 2019

jetstack-bot commented Feb 6, 2019

JoshVanL commented Oct 12, 2018 •

edited

Loading