From ff1190987b156cb65e19fc7c687e2ff6d979b0f7 Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Tue, 13 Aug 2024 22:39:10 +0200 Subject: [PATCH 1/2] test/apiserver/graceful_termination: do not prematurely close the audit files before this PR audit logs were not fully read because the files were already closed. --- .../apiserver/graceful_termination.go | 60 ++++++++++++------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/test/extended/apiserver/graceful_termination.go b/test/extended/apiserver/graceful_termination.go index 209824b0f773..d86a4642498b 100644 --- a/test/extended/apiserver/graceful_termination.go +++ b/test/extended/apiserver/graceful_termination.go @@ -149,8 +149,8 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer][Late]", func() { o.Expect(err).NotTo(o.HaveOccurred()) // set up - // apiserverName -> reader - logReaders := map[string]io.Reader{} + // apiserverName -> filePath + auditLogs := map[string]string{} results := map[string]int{} if *controlPlaneTopology == configv1.ExternalTopologyMode { @@ -158,12 +158,13 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer][Late]", func() { pods, err := mgmtClusterOC.KubeClient().CoreV1().Pods("").List(context.TODO(), metav1.ListOptions{LabelSelector: "hypershift.openshift.io/control-plane-component=kube-apiserver"}) o.Expect(err).To(o.BeNil()) for _, pod := range pods.Items { - fileName, err := mgmtClusterOC.Run("logs", "-n", pod.Namespace, pod.Name, "-c", "audit-logs").OutputToFile(pod.Name + "-audit.log") + filePath, err := mgmtClusterOC.Run("logs", "-n", pod.Namespace, pod.Name, "-c", "audit-logs").OutputToFile(pod.Name + "-audit.log") o.Expect(err).NotTo(o.HaveOccurred()) - reader, err := os.Open(fileName) + reader, err := os.Open(filePath) o.Expect(err).NotTo(o.HaveOccurred()) - defer reader.Close() - logReaders[pod.Namespace+"-"+pod.Name] = reader + err = reader.Close() + o.Expect(err).NotTo(o.HaveOccurred()) + auditLogs[pod.Namespace+"-"+pod.Name] = filePath } } else { tempDir, err := ioutil.TempDir("", "test.oc-adm-must-gather.") @@ -200,11 +201,13 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer][Late]", func() { gzipReader, err := gzip.NewReader(file) o.Expect(err).NotTo(o.HaveOccurred()) + err = gzipReader.Close() + o.Expect(err).NotTo(o.HaveOccurred()) apiServerName := extractAPIServerNameFromAuditFile(fileName) o.Expect(apiServerName).ToNot(o.BeEmpty()) - logReaders[apiServerName] = gzipReader + auditLogs[apiServerName] = path return nil }) @@ -212,24 +215,37 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer][Late]", func() { } } - for apiServerName, reader := range logReaders { + for apiServerName, auditLog := range auditLogs { lateRequestCounter := 0 - readFile := false - scanner := bufio.NewScanner(reader) - for scanner.Scan() { - text := scanner.Text() - if !strings.HasSuffix(text, "}") { - continue // ignore truncated data + func() { + file, err := os.Open(auditLog) + o.Expect(err).NotTo(o.HaveOccurred()) + defer file.Close() + + var reader io.Reader + reader = file + if isGzipFileByExtension(file.Name()) { + gzipReader, err := gzip.NewReader(file) + o.Expect(err).NotTo(o.HaveOccurred()) + defer gzipReader.Close() + reader = gzipReader } - o.Expect(text).To(o.HavePrefix(`{"kind":"Event",`)) - if strings.Contains(text, "openshift.io/during-graceful") && strings.Contains(text, "openshift-origin-external-backend-sampler") { - lateRequestCounter++ + scanner := bufio.NewScanner(reader) + for scanner.Scan() { + text := scanner.Text() + if !strings.HasSuffix(text, "}") { + continue // ignore truncated data + } + o.Expect(text).To(o.HavePrefix(`{"kind":"Event",`)) + + if strings.Contains(text, "openshift.io/during-graceful") && strings.Contains(text, "openshift-origin-external-backend-sampler") { + lateRequestCounter++ + } } - readFile = true - } - o.Expect(readFile).To(o.BeTrue()) + o.Expect(scanner.Err()).NotTo(o.HaveOccurred()) + }() if lateRequestCounter > 0 { previousLateRequestCounter, _ := results[apiServerName] @@ -251,6 +267,10 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer][Late]", func() { }) }) +func isGzipFileByExtension(fileName string) bool { + return strings.HasSuffix(fileName, ".gz") +} + func extractAPIServerNameFromAuditFile(auditFileName string) string { pos := strings.Index(auditFileName, "-audit") if pos == -1 { From ea4355211bd00387714f640c709b7e15f6a982b8 Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Tue, 13 Aug 2024 23:04:47 +0200 Subject: [PATCH 2/2] test/apiserver/graceful_termination: process all available audit logs. before this PR only a single audit log could be read. --- .../apiserver/graceful_termination.go | 67 ++++++++++--------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/test/extended/apiserver/graceful_termination.go b/test/extended/apiserver/graceful_termination.go index d86a4642498b..0cb9c4e337b6 100644 --- a/test/extended/apiserver/graceful_termination.go +++ b/test/extended/apiserver/graceful_termination.go @@ -149,8 +149,8 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer][Late]", func() { o.Expect(err).NotTo(o.HaveOccurred()) // set up - // apiserverName -> filePath - auditLogs := map[string]string{} + // apiserverName -> filePaths + auditLogsPerServer := map[string][]string{} results := map[string]int{} if *controlPlaneTopology == configv1.ExternalTopologyMode { @@ -164,7 +164,7 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer][Late]", func() { o.Expect(err).NotTo(o.HaveOccurred()) err = reader.Close() o.Expect(err).NotTo(o.HaveOccurred()) - auditLogs[pod.Namespace+"-"+pod.Name] = filePath + auditLogsPerServer[pod.Namespace+"-"+pod.Name] = append(auditLogsPerServer[pod.Namespace+"-"+pod.Name], filePath) } } else { tempDir, err := ioutil.TempDir("", "test.oc-adm-must-gather.") @@ -207,7 +207,7 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer][Late]", func() { apiServerName := extractAPIServerNameFromAuditFile(fileName) o.Expect(apiServerName).ToNot(o.BeEmpty()) - auditLogs[apiServerName] = path + auditLogsPerServer[apiServerName] = append(auditLogsPerServer[apiServerName], path) return nil }) @@ -215,41 +215,42 @@ var _ = g.Describe("[sig-api-machinery][Feature:APIServer][Late]", func() { } } - for apiServerName, auditLog := range auditLogs { - lateRequestCounter := 0 - - func() { - file, err := os.Open(auditLog) - o.Expect(err).NotTo(o.HaveOccurred()) - defer file.Close() - - var reader io.Reader - reader = file - if isGzipFileByExtension(file.Name()) { - gzipReader, err := gzip.NewReader(file) + for apiServerName, auditLogs := range auditLogsPerServer { + for _, auditLog := range auditLogs { + lateRequestCounter := 0 + func() { + file, err := os.Open(auditLog) o.Expect(err).NotTo(o.HaveOccurred()) - defer gzipReader.Close() - reader = gzipReader - } + defer file.Close() - scanner := bufio.NewScanner(reader) - for scanner.Scan() { - text := scanner.Text() - if !strings.HasSuffix(text, "}") { - continue // ignore truncated data + var reader io.Reader + reader = file + if isGzipFileByExtension(file.Name()) { + gzipReader, err := gzip.NewReader(file) + o.Expect(err).NotTo(o.HaveOccurred()) + defer gzipReader.Close() + reader = gzipReader } - o.Expect(text).To(o.HavePrefix(`{"kind":"Event",`)) - if strings.Contains(text, "openshift.io/during-graceful") && strings.Contains(text, "openshift-origin-external-backend-sampler") { - lateRequestCounter++ + scanner := bufio.NewScanner(reader) + for scanner.Scan() { + text := scanner.Text() + if !strings.HasSuffix(text, "}") { + continue // ignore truncated data + } + o.Expect(text).To(o.HavePrefix(`{"kind":"Event",`)) + + if strings.Contains(text, "openshift.io/during-graceful") && strings.Contains(text, "openshift-origin-external-backend-sampler") { + lateRequestCounter++ + } } - } - o.Expect(scanner.Err()).NotTo(o.HaveOccurred()) - }() + o.Expect(scanner.Err()).NotTo(o.HaveOccurred()) + }() - if lateRequestCounter > 0 { - previousLateRequestCounter, _ := results[apiServerName] - results[apiServerName] = previousLateRequestCounter + lateRequestCounter + if lateRequestCounter > 0 { + previousLateRequestCounter, _ := results[apiServerName] + results[apiServerName] = previousLateRequestCounter + lateRequestCounter + } } }