Separate Linux and Windows node log collection by sebsoto · Pull Request #165 · openshift/must-gather

sebsoto · 2020-07-30T17:17:55Z

The '-u' flag for 'oc adm node-logs' cannot be used against Windows
nodes. This commit ensures that the '-u' method of collection is only
used for services running on Linux nodes, and the '--path' option is
used for collecting Windows node log files.

sferich888

@davemulford can you review this too.
I think we could simplify this (reduce the number of scripts needed here), if we leverage the -l flag better.

sferich888 · 2020-07-30T17:30:32Z

    for service in ${NODE_SERVICES[@]}; do
        echo "INFO: Collecting host service logs for $service"
-        /usr/bin/oc adm node-logs --role=$1 -u $service > ${DIR_PATH}/${service}_service.log &
+        /usr/bin/oc adm node-logs --role=$1 -l kubernetes.io/os=linux -u $service > ${DIR_PATH}/${service}_service.log &


Can we use one or more labels here?

Yes a comma separated list can be used

sferich888 · 2020-07-30T17:31:00Z

@@ -0,0 +1,22 @@
+#!/bin/bash


Instead of this; would it be better to just merge this into one bash script and select the things we want using labels?

Most of the Linux log collection relies on the fact that the service logs being collected are from journald. This is not the case for Windows, and why I felt it made sense to seperate them

We are missing container runtime, logs (docker), as well as kublet how do we get those added?

I saw that we were only collecting Kubelet logs for master nodes for Linux, and as Windows nodes can only be workers, I removed the kubelet logs from the collected log list.
I can add it back if that would be desirable.

As for Docker logs, that actually is logged to a directory that isn't reachable by oc adm node-logs and changing that logging directory isn't currently within scope of the Windows Machine Config Operator.

We want to collect everything required to debug what is happening on the node (service logs wise); as it pertains to OpenShift. So I think we want to collect at the very least the container runtime and the kublet process logs (thinks that are installed outside of OpenShift that are pre-requisites for OpenShift to run or be connected to the cluster). .

@sferich888 collecting kubelet logs is doable. But as @sebsoto mentioned, collecting the Docker logs will be difficult. Please open an issue on our board so that we can track this.

I've added the kubelet log.

The issue with the Docker logs is that the logging location for that is not under our control. Our product asks that our users bring their own Windows VM image, with docker pre-installed. We'd either have to increase the customer requirements, or investigate reconfiguring Docker.

sferich888 · 2020-07-30T17:31:37Z

  PIDS+=($!)

-  oc adm node-logs --role="worker" -u crio | \
+  oc adm node-logs --role="worker" -l kubernetes.io/os=linux -u crio | \


I don't think we want this here, as we want to get logs in this situation from any place where OVN is installed, not just on linux.

For OVN onn Windows, there is a Windows service named hybrid-overlay.exe instead. the -u option does not work on windows as well, as that is specific to journald, which windows does not have

Shouldn't oc adm node-logs account for this? Is this a bug / issue we should address in that command?

I would not consider it a bug, as there really isn't a way to get it to work on Windows. The closest parallel to journald would be the Windows Event Viewer, but none of the Windows Kubernetes components are logging to that.

sdodson · 2020-07-30T19:13:41Z

/lgtm
This looks like a step in the right direction and given the constraints we're working under with respect to Docker installation and configuration this may be the best we can do right now.

@sferich888 @davemulford I'd appreciate if you approved this.

sferich888 · 2020-07-30T20:24:24Z

@@ -0,0 +1,22 @@
+#!/bin/bash
+BASE_COLLECTION_PATH="/must-gather"
+WINDOWS_NODE_LOGS=$BASE_COLLECTION_PATH/windows


Do we want to update https://github.com/openshift/must-gather/blob/master/collection-scripts/gather_service_logs#L13 to say ${BASE_COLLECTION_PATH}/host_service_logs/linux, and then the referenced line can read ${BASE_COLLECTION_PATH}/host_service_logs/windows Thus helping with the find-ability and placement consistency of the logs.

Yes, having uniformly named directories would benefit both customers and support. We should definitely have this change.

I have made this change

I think given we have both tests and users expecting to find linux logs in one location today we should leave that unchanged but write out windows logs, which are net new, to a new location. So lets revert the relocation of linux logs.

sferich888 · 2020-07-31T01:35:38Z

/lgtm
This looks like a step in the right direction and given the constraints we're working under with respect to Docker installation and configuration this may be the best we can do right now.

@sferich888 @davemulford I'd appreciate if you approved this.

So after looking into this more, I don't think this PR will work? But I could be wrong.

The way we get logs from nodes is done with a
carry patch on the kubelet.

As I understand it, the window nodes will use an upstream kubelet, and as such the method or approach to getting node logs will not be available on windows.

Does anyone know, how / what API oc adm node-logs is connecting to on a window node?

sebsoto · 2020-07-31T14:44:41Z

@sferich888 I have confirmed that it does work, to answer your question for why it works I am looking into that

sebsoto · 2020-07-31T14:53:57Z

@sferich888 That carry patch seems to be specific to the -u node-logs option, we are using --path.

aravindhp · 2020-07-31T14:57:11Z

@sferich888 the carry patch is for Provide an administrator a streaming view of journal logs without them having to implement a client side reader. Only available to cluster admins.. There is no concept of a journal log in Windows. We are using oc adm node-logs --path to work around it.

sferich888 · 2020-07-31T15:45:43Z

@aravindhp yes, but I think oc adm node-logs --path still uses the shim (carry patch) to get at the hosts logs (correct)? @smarterclayton

aravindhp · 2020-07-31T16:14:18Z

@sferich888 oc adm node-logs --path is something we can confirm is working with an upstream kubelet running on a Windows node. And for the product due to another carry patch openshift/kubernetes#301 and release requirements we will be switching to a downstream kubelet for Windows also.

davemulford · 2020-07-31T18:03:17Z

Thanks for the quick changes @sebsoto.

This looks fine to me, @sferich888 what do you think?

/LGTM

openshift-bot · 2020-07-31T21:18:55Z

/retest