feature: kubectl logs options

[pires]

Unit logs are made available to journald, and that's great but that's an implementation detail. For any Kubernetes user, kubectl logs [--follow] <pod name> is what's expected to work.

At the time of this writing, there are hints that we agree this feature makes sense:

systemk/systemd/pod.go

Lines 265 to 274 in 745f958

	func (p *P) GetContainerLogs(ctx context.Context, namespace, podName, containerName string, opts api.ContainerLogOpts) (io.ReadCloser, error) {
	log.Printf("GetContainerLogs called")
	unitname := UnitPrefix(namespace, podName) + separator + containerName
	args := []string{"-u", unitname}
	cmd := exec.Command("journalctl", args...)
	// returns the buffers? What about following, use pipes here or something?
	buf, err := cmd.CombinedOutput()
	return ioutil.NopCloser(bytes.NewReader(buf)), err
	}

The way I see it, the implementation could do with a bit more love. And as I was looking into what could be done on that front, I recalled moby (dockerd) does provide a journald logging driver, which relies on https://github.com/coreos/go-systemd/blob/master/sdjournal/journal.go, but it is quite verbose. So I'm starting this issue to brainstorm if it makes sense to adopt such code, if there are any known alternatives or, ultimately, if this becomes a non-goal entirely.

[pires]

One more option is --since. This makes me think we must consider all logging options.

[pires]

Also, something I realized during my tests is that if a unit with the same name ran in the past, there's the chance logs are still around which doesn't seem a good thing.

A quick search led me to learn journalctl allows for vacuum the logs but only archived ones, so something like this could work:

journalctl --rotate
journalctl --vacuum-time=1s

[pires]

Also, something I realized during my tests is that if a unit with the same name ran in the past, there's the chance logs are still around which doesn't seem a good thing.

A quick search led me to learn journalctl allows for vacuum the logs but only archived ones, so something like this could work:

Recreation of pods where the name sticks is usually a thing only when dealing with naked Pods (not under the control of any envelope, such as Deployment) or StatefulSets, eg my-pod-0. But this is enough to justify, I think, some sort of logic here that avoids fetching logs from previous instances of a homonym Pod.

An alternative to what I proposed earlier, is to have the provider enforce journald entries that have been created since after the current Pod instance started. Since the provider has access to informers cache (the current code doesn't have access to the Pod informer but we can change that), we can easily retrieve the starttime attribute and compute based on that.

WDYT @miekg?

[miekg]

Also, something I realized during my tests is that if a unit with the same name ran in the past, there's the chance logs are still around which doesn't seem a good thing.

I don't think this is bad, this could actually be helpful. And anyway if journald works like this we should not jump through hoops to make it work differently.

[pires]

It's not much about systemd but about the fact that you'll get logs from Pod instances that no longer exist and/or never belonged to you. What if user A runs Pod xyz, prints some data, deletes Pod, and then user B runs a Pod with same name: is it ok for user B to get logs from a workload instance they didn't own? In the containerized Kubernetes world, even when journald is the log sink, it is guaranteed the logs pertain only to the instance of the Pod you're retrieving logs for.

[pires]

Another alternative, and way more easy, is to append Pod UID to the the systemd unit name, hence guaranteeing uniqueness.

[miekg]

[ Quoting <notifications@github.com> in "Re: [virtual-kubelet/systemk] featu..." ]

Another alternative, and way more easy, is to append Pod UID to the the systemd unit name, hence guaranteeing uniqueness.

that might work better, but leads to very long and ugly unit names. If an identically named pod lands on the same machine, it in the same namespace anyway, so I don't really why that would be an issue.

[pires]

I think I'm too tied to how the kubelet does things and to keep UX parity when possible.

Anyway, I think it's fine to leave it for now and if it becomes a problem later it will be easy to address.

[miekg]

[ Quoting <notifications@github.com> in "Re: [virtual-kubelet/systemk] featu..." ]

I think I'm too tied to how the kubelet does things and to keep UX parity when possible. Anyway, I think it's fine to leave it for now and if it becomes a problem later it will be easy to address.

seems most sensible way forward. +1