use must-gather since and since time env vars on inspect cmds (#212)

see: openshift/must-gather#400 Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>
kubevirt · Apr 24, 2024 · 54e97fd · 54e97fd
1 parent 648006b
commit 54e97fd
Show file tree

Hide file tree

Showing 10 changed files with 106 additions and 11 deletions.
diff --git a/collection-scripts/common.sh b/collection-scripts/common.sh
@@ -13,3 +13,35 @@ function check_command {
     fi
 }
 
+get_log_collection_args() {
+	# validation of MUST_GATHER_SINCE and MUST_GATHER_SINCE_TIME is done by the
+	# caller (oc adm must-gather) so it's safe to use the values as they are.
+	log_collection_args=""
+
+	if [ -n "${MUST_GATHER_SINCE:-}" ]; then
+		log_collection_args=--since="${MUST_GATHER_SINCE}"
+	fi
+	if [ -n "${MUST_GATHER_SINCE_TIME:-}" ]; then
+		log_collection_args=--since-time="${MUST_GATHER_SINCE_TIME}"
+	fi
+
+	# oc adm node-logs `--since` parameter is not the same as oc adm inspect `--since`.
+	# it takes a simplified duration in the form of '(+|-)[0-9]+(s|m|h|d)' or
+	# an ISO formatted time. since MUST_GATHER_SINCE and MUST_GATHER_SINCE_TIME
+	# are formatted differently, we re-format them so they can be used
+	# transparently by node-logs invocations.
+	node_log_collection_args=""
+
+	if [ -n "${MUST_GATHER_SINCE:-}" ]; then
+		# shellcheck disable=SC2001
+		since=$(echo "${MUST_GATHER_SINCE:-}" | sed 's/\([0-9]*[dhms]\).*/\1/')
+		node_log_collection_args=--since="-${since}"
+	fi
+	if [ -n "${MUST_GATHER_SINCE_TIME:-}" ]; then
+	  # shellcheck disable=SC2001
+		iso_time=$(echo "${MUST_GATHER_SINCE_TIME}" | sed 's/T/ /; s/Z//')
+		node_log_collection_args=--since="${iso_time}"
+	fi
+	export log_collection_args
+	export node_log_collection_args
+}
diff --git a/collection-scripts/gather_cdi b/collection-scripts/gather_cdi
@@ -3,6 +3,7 @@
 DIR_NAME=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 source "${DIR_NAME}/common.sh"
 check_command
+get_log_collection_args
 
 function gather_cdi_pod() {
   line=$1
@@ -13,7 +14,8 @@ function gather_cdi_pod() {
   pod_collection_path="${BASE_COLLECTION_PATH}/namespaces/${ns}/${pod}"
   mkdir -p "${pod_collection_path}"
 
-  oc adm inspect --dest-dir "${BASE_COLLECTION_PATH}" -n "${ns}" "pod/${pod}"
+  # shellcheck disable=SC2086
+  oc adm inspect ${log_collection_args} --dest-dir "${BASE_COLLECTION_PATH}" -n "${ns}" "pod/${pod}"
   oc logs "${pod}" -n "${ns}" > "${pod_collection_path}/${pod}.log"
 }
 
@@ -26,7 +28,8 @@ function gather_pvc() {
   pvc_collection_path="${BASE_COLLECTION_PATH}/namespaces/${ns}/core/persistentvolumeclaims"
   mkdir -p "$pvc_collection_path"
 
-  /usr/bin/oc adm inspect --dest-dir "${BASE_COLLECTION_PATH}" -n "${ns}" "pvc/${pvc}"
+  # shellcheck disable=SC2086
+  /usr/bin/oc adm inspect ${log_collection_args} --dest-dir "${BASE_COLLECTION_PATH}" -n "${ns}" "pvc/${pvc}"
   oc describe pvc "${pvc}" -n "${ns}" > "${pvc_collection_path}/${pvc}_describe.txt"
 }
 

diff --git a/collection-scripts/gather_crds b/collection-scripts/gather_crds
@@ -3,11 +3,12 @@
 DIR_NAME=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 source "${DIR_NAME}/common.sh"
 check_command
+get_log_collection_args
 
 # Resource list
 CRDS=$(/usr/bin/oc get crd | grep kubevirt.io | awk '{print $1}')
 
 # Run the collection of crds using must-gather
-echo "${CRDS[@]}" | tr ' ' '\n' | xargs -t -I{}  -P "${PROS}" --max-args=1 sh -c '/usr/bin/oc adm inspect crd --dest-dir ${BASE_COLLECTION_PATH} $1' -- {}
+echo "${CRDS[@]}" | tr ' ' '\n' | xargs -t -I{}  -P "${PROS}" --max-args=1 sh -c '/usr/bin/oc adm inspect ${log_collection_args} crd --dest-dir ${BASE_COLLECTION_PATH} $1' -- {}
 
 exit 0
diff --git a/collection-scripts/gather_instancetypes b/collection-scripts/gather_instancetypes
@@ -3,17 +3,19 @@
 DIR_NAME=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 source "${DIR_NAME}/common.sh"
 check_command
+get_log_collection_args
 
 resources=(virtualmachineinstancetype virtualmachineclusterinstancetype virtualmachinepreference virtualmachineclusterpreference)
 
-echo "${resources[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c 'echo "inspecting $1" && oc adm inspect --dest-dir ${BASE_COLLECTION_PATH} --all-namespaces $1' -- {}
+echo "${resources[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c 'echo "inspecting $1" && oc adm inspect ${log_collection_args} --dest-dir ${BASE_COLLECTION_PATH} --all-namespaces $1' -- {}
 
 function gather_controller_revision() {
   oc_project=$(echo "$1" | awk -F_ '{print $1}')
   revision_name=$(echo "$1" | awk -F_ '{print $2}')
   echo "inspecting ${revision_name} controller revision"
 
-  /usr/bin/oc adm inspect --dest-dir "${BASE_COLLECTION_PATH}" -n "${oc_project}" controllerrevision "${revision_name}"
+  # shellcheck disable=SC2086
+  /usr/bin/oc adm inspect ${log_collection_args} --dest-dir "${BASE_COLLECTION_PATH}" -n "${oc_project}" controllerrevision "${revision_name}"
 }
 
 export -f gather_controller_revision

diff --git a/collection-scripts/gather_nodes b/collection-scripts/gather_nodes
@@ -3,6 +3,7 @@
 DIR_NAME=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 source "${DIR_NAME}/common.sh"
 check_command
+get_log_collection_args
 
 MANIFEST_PATH=${MANIFEST_PATH:-"/etc"}
 
@@ -111,7 +112,7 @@ echo "${pods[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c
 
 # Collect journal logs for specified units for all nodes
 nodes=$(oc get nodes --no-headers -o custom-columns=':metadata.name')
-echo "${nodes[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c 'mkdir -p $2; oc adm node-logs $1 -u NetworkManager > $2/${1}_logs_NetworkManager' -- {} "${NODES_PATH}"/{}
+echo "${nodes[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c 'mkdir -p $2; oc adm node-logs ${node_log_collection_args} $1 -u NetworkManager > $2/${1}_logs_NetworkManager' -- {} "${NODES_PATH}"/{}
 
 oc delete -f "${DAEMONSET_MANIFEST}"
 oc delete -f "${CRD_MANIFEST}"
diff --git a/collection-scripts/gather_ns b/collection-scripts/gather_ns
@@ -3,6 +3,7 @@
 DIR_NAME=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 source "${DIR_NAME}/common.sh"
 check_command
+get_log_collection_args
 
 namespaces=()
 # KubeVirt HCO related namespaces
@@ -23,4 +24,4 @@ namespaces+=(openshift-virtualization-os-images)
 resources+=(cdi)
 
 # Run the collection of namespaces using must-gather
-echo "${namespaces[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c 'echo inspecting namespace $1 && oc adm inspect --dest-dir "${BASE_COLLECTION_PATH}" namespace $1' -- {}
+echo "${namespaces[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c 'echo inspecting namespace $1 && oc adm inspect ${log_collection_args} --dest-dir "${BASE_COLLECTION_PATH}" namespace $1' -- {}
diff --git a/collection-scripts/gather_resources b/collection-scripts/gather_resources
@@ -3,6 +3,7 @@
 DIR_NAME=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 source "${DIR_NAME}/common.sh"
 check_command
+get_log_collection_args
 
 # Resource list
 resources=()
@@ -25,4 +26,4 @@ resources+=(machines)
 "${DIR_NAME}"/version
 
 # Run the collection of resources using must-gather
-echo "${resources[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c 'echo "inspecting $1" && oc adm inspect --dest-dir ${BASE_COLLECTION_PATH} --all-namespaces $1' -- {}
+echo "${resources[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c 'echo "inspecting $1" && oc adm inspect ${log_collection_args} --dest-dir ${BASE_COLLECTION_PATH} --all-namespaces $1' -- {}
diff --git a/collection-scripts/gather_vms_details b/collection-scripts/gather_vms_details
@@ -3,6 +3,7 @@
 DIR_NAME=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 source "${DIR_NAME}/common.sh"
 check_command
+get_log_collection_args
 
 export QEMULOGPATH_1="/var/log/libvirt/qemu/"
 export QEMULOGPATH_2="/var/run/libvirt/qemu/log/"
@@ -16,8 +17,10 @@ function gather_vm_info() {
   vm_collection_path="${BASE_COLLECTION_PATH}/namespaces/${ocproject}/vms/${vmname}"
   mkdir -p "${vm_collection_path}"
 
-  /usr/bin/oc adm inspect --dest-dir "${BASE_COLLECTION_PATH}" -n "${ocproject}" pod "$ocvm"
-  /usr/bin/oc adm inspect --dest-dir "${BASE_COLLECTION_PATH}" -n "${ocproject}" virtualmachineinstances "${vmname}"
+  # shellcheck disable=SC2086
+  /usr/bin/oc adm inspect ${log_collection_args} --dest-dir "${BASE_COLLECTION_PATH}" -n "${ocproject}" pod "$ocvm"
+  # shellcheck disable=SC2086
+  /usr/bin/oc adm inspect ${log_collection_args} --dest-dir "${BASE_COLLECTION_PATH}" -n "${ocproject}" virtualmachineinstances "${vmname}"
 
   # VM : capabilities
   /usr/bin/oc exec "${ocvm}" -n "${ocproject}" -c compute -- virsh -r capabilities  > "${vm_collection_path}/${ocvm}.capabilities.xml"

diff --git a/collection-scripts/gather_vms_namespaces b/collection-scripts/gather_vms_namespaces
@@ -3,6 +3,7 @@
 DIR_NAME=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 source "${DIR_NAME}/common.sh"
 check_command
+get_log_collection_args
 
 if [[ -n $NS ]]; then
   namespaces="${NS}"
@@ -11,6 +12,6 @@ else
 fi
 
 # Run the collection of namespaces using must-gather
-echo "${namespaces[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c 'echo "inspecting namespace $1" && /usr/bin/oc adm inspect --dest-dir ${BASE_COLLECTION_PATH} namespace $1' -- {}
+echo "${namespaces[@]}" | tr ' ' '\n' | xargs -t -I{} -P "${PROS}" --max-args=1 sh -c 'echo "inspecting namespace $1" && /usr/bin/oc adm inspect ${log_collection_args} --dest-dir ${BASE_COLLECTION_PATH} namespace $1' -- {}
 
 exit 0
diff --git a/tests/validate_must_gather_test.go b/tests/validate_must_gather_test.go
@@ -1,13 +1,15 @@
 package tests_test
 
 import (
+	"bufio"
 	"context"
 	"errors"
 	"fmt"
 	"log"
 	"os"
 	"path"
 	"reflect"
+	"regexp"
 	"strings"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -207,6 +209,38 @@ var _ = Describe("validate the must-gather output", func() {
 		})
 
 		logger.Print("outputDir:", outputDir)
+
+	})
+
+	Context("[level:product]validate usage of inspection parameters", Label("level:product"), func() {
+		It("should validate inspect and node-logs parameters usage on all the relevant logged commands", func() {
+			logfile, err := getMGlogfile()
+			Expect(err).ToNot(HaveOccurred())
+
+			space := regexp.MustCompile(`\s+`)
+
+			readFile, err := os.Open(logfile)
+			Expect(err).ToNot(HaveOccurred())
+			fileScanner := bufio.NewScanner(readFile)
+			fileScanner.Split(bufio.ScanLines)
+			var inspectcmdLines []string
+			var nodelogsLines []string
+
+			for fileScanner.Scan() {
+				line := space.ReplaceAllString(fileScanner.Text(), " ")
+				if strings.Contains(line, "oc adm inspect") {
+					inspectcmdLines = append(inspectcmdLines, line)
+				}
+				if strings.Contains(line, "oc adm node-logs") {
+					nodelogsLines = append(nodelogsLines, line)
+				}
+			}
+
+			readFile.Close()
+
+			Expect(inspectcmdLines).To(HaveEach(ContainSubstring("${log_collection_args}")), "all the inspect cmd should pass log collection args")
+			Expect(nodelogsLines).To(HaveEach(ContainSubstring("${node_log_collection_args}")), "all the node-logs cmd should pass node log collection args")
+		})
 	})
 
 	Context("[level:workloads]validate workloads", Label("level:workloads"), func() {
@@ -402,3 +436,19 @@ func getDataDir() (string, error) {
 
 	return "", errors.New("can't find the cluster directory")
 }
+
+func getMGlogfile() (string, error) {
+	const logfilename = "must-gather.log"
+	outputDir, err := getDataDir()
+	if err != nil {
+		return "", err
+	}
+
+	mgLogFile := path.Join(outputDir, logfilename)
+	if _, err := os.Stat(mgLogFile); err == nil {
+		return mgLogFile, nil
+
+	}
+
+	return "", errors.New("can't find must-gather log file")
+}