UPSTREAM: <carry>: Extend NodeLogQuery feature

Extend the NodeLogQuery feature to support oc adm node-logs options:
- Default NodeLogQuery feature gate to true
- Add support for --since, --until, --case-sensitive, --output, options

UPSTREAM: <carry>: Extend NodeLogQuery feature

Fix handling of the "until" parameter when generating the journalctl
command. This was incorrectly being passed with the "since" value.

Author: aravindhp

pkg/features/versioned_kube_features.go

@@ -542,8 +542,8 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 	},
 
 	NodeLogQuery: {
-		{Version: version.MustParse("1.27"), Default: false, PreRelease: featuregate.Alpha},
-		{Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Beta},
+		{Version: version.MustParse("1.27"), Default: true, PreRelease: featuregate.Alpha},
+		{Version: version.MustParse("1.30"), Default: true, PreRelease: featuregate.Beta},
 	},
 
 	NodeSwap: {

pkg/kubelet/apis/config/validation/validation_test.go

@@ -389,6 +389,7 @@ func TestValidateKubeletConfiguration(t *testing.T) {
 			conf.CrashLoopBackOff = kubeletconfig.CrashLoopBackOffConfig{
 				MaxContainerRestartPeriod: &metav1.Duration{Duration: 0 * time.Second},
 			}
+
 			return conf
 		},
 		errMsg: "invalid configuration: CrashLoopBackOff.MaxContainerRestartPeriod (got: 0 seconds) must be set between 1s and 300s",

pkg/kubelet/kubelet.go

@@ -1685,16 +1685,13 @@ func (kl *Kubelet) Run(updates <-chan kubetypes.PodUpdate) {
 					http.Error(w, errs.ToAggregate().Error(), http.StatusBadRequest)
 					return
 				} else if nlq != nil {
-					if req.URL.Path != "/" && req.URL.Path != "" {
-						http.Error(w, "path not allowed in query mode", http.StatusNotAcceptable)
-						return
-					}
 					if errs := nlq.validate(); len(errs) > 0 {
 						http.Error(w, errs.ToAggregate().Error(), http.StatusNotAcceptable)
 						return
 					}
 					// Validation ensures that the request does not query services and files at the same time
-					if len(nlq.Services) > 0 {
+					// OCP: Presence of journal in the path indicates it is a query for service(s)
+					if len(nlq.Services) > 0 || req.URL.Path == "journal" || req.URL.Path == "journal/" {
 						journal.ServeHTTP(w, req)
 						return
 					}

pkg/kubelet/kubelet_server_journal.go

@@ -35,7 +35,7 @@ import (
 	"time"
 
 	securejoin "github.com/cyphar/filepath-securejoin"
-
+	"k8s.io/apimachinery/pkg/util/sets"
 	utilvalidation "k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 )
@@ -54,6 +54,7 @@ var (
 	// character cannot be used to create invalid sequences. This is intended as a broad defense against malformed
 	// input that could cause an escape.
 	reServiceNameUnsafeCharacters = regexp.MustCompile(`[^a-zA-Z\-_.:0-9@]+`)
+	reRelativeDate                = regexp.MustCompile(`^(\+|\-)?[\d]+(s|m|h|d)$`)
 )
 
 // journalServer returns text output from the OS specific service logger to view
@@ -114,6 +115,19 @@ type options struct {
 	// Pattern filters log entries by the provided regex pattern. On Linux nodes, this pattern will be read as a
 	// PCRE2 regex, on Windows nodes it will be read as a PowerShell regex. Support for this is implementation specific.
 	Pattern string
+	ocAdm
+}
+
+// ocAdm encapsulates the oc adm node-logs specific options
+type ocAdm struct {
+	// Since is an ISO timestamp or relative date from which to show logs
+	Since string
+	// Until is an ISO timestamp or relative date until which to show logs
+	Until string
+	// Format is the alternate format (short, cat, json, short-unix) to display journal logs
+	Format string
+	// CaseSensitive controls the case sensitivity of pattern searches
+	CaseSensitive bool
 }
 
 // newNodeLogQuery parses query values and converts all known options into nodeLogQuery
@@ -122,7 +136,7 @@ func newNodeLogQuery(query url.Values) (*nodeLogQuery, field.ErrorList) {
 	var nlq nodeLogQuery
 	var err error
 
-	queries, ok := query["query"]
+	queries, okQuery := query["query"]
 	if len(queries) > 0 {
 		for _, q := range queries {
 			// The presence of / or \ is a hint that the query is for a log file. If the query is for foo.log without a
@@ -134,11 +148,20 @@ func newNodeLogQuery(query url.Values) (*nodeLogQuery, field.ErrorList) {
 			}
 		}
 	}
+	units, okUnit := query["unit"]
+	if len(units) > 0 {
+		for _, u := range units {
+			// We don't check for files as the heuristics do not apply to unit
+			if strings.TrimSpace(u) != "" { // Prevent queries with just spaces
+				nlq.Services = append(nlq.Services, u)
+			}
+		}
+	}
 
 	// Prevent specifying  an empty or blank space query.
 	// Example: kubectl get --raw /api/v1/nodes/$node/proxy/logs?query="   "
-	if ok && (len(nlq.Files) == 0 && len(nlq.Services) == 0) {
-		allErrs = append(allErrs, field.Invalid(field.NewPath("query"), queries, "query cannot be empty"))
+	if (okQuery || okUnit) && (len(nlq.Files) == 0 && len(nlq.Services) == 0) {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("unit"), queries, "unit cannot be empty"))
 	}
 
 	var sinceTime time.Time
@@ -176,6 +199,9 @@ func newNodeLogQuery(query url.Values) (*nodeLogQuery, field.ErrorList) {
 
 	var tailLines int
 	tailLinesValue := query.Get("tailLines")
+	if len(tailLinesValue) == 0 {
+		tailLinesValue = query.Get("tail")
+	}
 	if len(tailLinesValue) > 0 {
 		tailLines, err = strconv.Atoi(tailLinesValue)
 		if err != nil {
@@ -186,15 +212,28 @@ func newNodeLogQuery(query url.Values) (*nodeLogQuery, field.ErrorList) {
 	}
 
 	pattern := query.Get("pattern")
+	if len(pattern) == 0 {
+		pattern = query.Get("grep")
+	}
 	if len(pattern) > 0 {
 		nlq.Pattern = pattern
+		caseSensitiveValue := query.Get("case-sensitive")
+		if len(caseSensitiveValue) > 0 {
+			caseSensitive, err := strconv.ParseBool(query.Get("case-sensitive"))
+			if err != nil {
+				allErrs = append(allErrs, field.Invalid(field.NewPath("case-sensitive"), query.Get("case-sensitive"),
+					err.Error()))
+			} else {
+				nlq.CaseSensitive = caseSensitive
+			}
+		}
 	}
 
-	if len(allErrs) > 0 {
-		return nil, allErrs
-	}
+	nlq.Since = query.Get("since")
+	nlq.Until = query.Get("until")
+	nlq.Format = query.Get("output")
 
-	if reflect.DeepEqual(nlq, nodeLogQuery{}) {
+	if len(allErrs) > 0 {
 		return nil, allErrs
 	}
 
@@ -219,14 +258,13 @@ func validateServices(services []string) field.ErrorList {
 func (n *nodeLogQuery) validate() field.ErrorList {
 	allErrs := validateServices(n.Services)
 	switch {
-	case len(n.Files) == 0 && len(n.Services) == 0:
-		allErrs = append(allErrs, field.Required(field.NewPath("query"), "cannot be empty with options"))
+	// OCP: Allow len(n.Files) == 0 && len(n.Services) == 0 as we want to be able to return all journal / WinEvent logs
 	case len(n.Files) > 0 && len(n.Services) > 0:
 		allErrs = append(allErrs, field.Invalid(field.NewPath("query"), fmt.Sprintf("%v, %v", n.Files, n.Services),
 			"cannot specify a file and service"))
 	case len(n.Files) > 1:
 		allErrs = append(allErrs, field.Invalid(field.NewPath("query"), n.Files, "cannot specify more than one file"))
-	case len(n.Files) == 1 && n.options != (options{}):
+	case len(n.Files) == 1 && !reflect.DeepEqual(n.options, options{}):
 		allErrs = append(allErrs, field.Invalid(field.NewPath("query"), n.Files, "cannot specify file with options"))
 	case len(n.Files) == 1:
 		if fullLogFilename, err := securejoin.SecureJoin(nodeLogDir, n.Files[0]); err != nil {
@@ -258,6 +296,35 @@ func (n *nodeLogQuery) validate() field.ErrorList {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("pattern"), n.Pattern, err.Error()))
 	}
 
+	// "oc adm node-logs" specific validation
+
+	if n.SinceTime != nil && (len(n.Since) > 0 || len(n.Until) > 0) {
+		allErrs = append(allErrs, field.Forbidden(field.NewPath("sinceTime"),
+			"`since or until` and `sinceTime` cannot be specified"))
+	}
+
+	if n.UntilTime != nil && (len(n.Since) > 0 || len(n.Until) > 0) {
+		allErrs = append(allErrs, field.Forbidden(field.NewPath("untilTime"),
+			"`since or until` and `untilTime` cannot be specified"))
+	}
+
+	if err := validateDate(n.Since); err != nil {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("since"), n.Since, err.Error()))
+	}
+
+	if err := validateDate(n.Until); err != nil {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("until"), n.Until, err.Error()))
+	}
+
+	allowedFormats := sets.New[string]("short-precise", "json", "short", "short-unix", "short-iso",
+		"short-iso-precise", "cat", "")
+	if len(n.Format) > 0 && runtime.GOOS == "windows" {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("output"), n.Format,
+			"output is not supported on Windows"))
+	} else if !allowedFormats.Has(n.Format) {
+		allErrs = append(allErrs, field.NotSupported(field.NewPath("output"), n.Format, allowedFormats.UnsortedList()))
+	}
+
 	return allErrs
 }
 
@@ -280,19 +347,20 @@ func (n *nodeLogQuery) copyForBoot(ctx context.Context, w io.Writer, previousBoo
 		return
 	}
 	nativeLoggers, fileLoggers := n.splitNativeVsFileLoggers(ctx)
-	if len(nativeLoggers) > 0 {
-		n.copyServiceLogs(ctx, w, nativeLoggers, previousBoot)
-	}
 
-	if len(fileLoggers) > 0 && n.options != (options{}) {
+	if len(fileLoggers) > 0 && !reflect.DeepEqual(n.options, options{}) {
 		fmt.Fprintf(w, "\noptions present and query resolved to log files for %v\ntry without specifying options\n",
 			fileLoggers)
 		return
 	}
 
 	if len(fileLoggers) > 0 {
 		copyFileLogs(ctx, w, fileLoggers)
+		return
 	}
+	// OCP: Return all logs in the case where nativeLoggers == ""
+	n.copyServiceLogs(ctx, w, nativeLoggers, previousBoot)
+
 }
 
 // splitNativeVsFileLoggers checks if each service logs to native OS logs or to a file and returns a list of services
@@ -413,3 +481,16 @@ func safeServiceName(s string) error {
 	}
 	return nil
 }
+
+func validateDate(date string) error {
+	if len(date) == 0 {
+		return nil
+	}
+	if reRelativeDate.MatchString(date) {
+		return nil
+	}
+	if _, err := time.Parse(dateLayout, date); err == nil {
+		return nil
+	}
+	return fmt.Errorf("date must be a relative time of the form '(+|-)[0-9]+(s|m|h|d)' or a date in 'YYYY-MM-DD HH:MM:SS' form")
+}

pkg/kubelet/kubelet_server_journal_linux.go

@@ -36,14 +36,20 @@ func getLoggingCmd(n *nodeLogQuery, services []string) (string, []string, error)
 	args := []string{
 		"--utc",
 		"--no-pager",
-		"--output=short-precise",
 	}
-	if n.SinceTime != nil {
+
+	if len(n.Since) > 0 {
+		args = append(args, fmt.Sprintf("--since=%s", n.Since))
+	} else if n.SinceTime != nil {
 		args = append(args, fmt.Sprintf("--since=%s", n.SinceTime.Format(dateLayout)))
 	}
-	if n.UntilTime != nil {
-		args = append(args, fmt.Sprintf("--until=%s", n.UntilTime.Format(dateLayout)))
+
+	if len(n.Until) > 0 {
+		args = append(args, fmt.Sprintf("--until=%s", n.Until))
+	} else if n.UntilTime != nil {
+		args = append(args, fmt.Sprintf("--until=%s", n.SinceTime.Format(dateLayout)))
 	}
+
 	if n.TailLines != nil {
 		args = append(args, "--pager-end", fmt.Sprintf("--lines=%d", *n.TailLines))
 	}
@@ -54,12 +60,21 @@ func getLoggingCmd(n *nodeLogQuery, services []string) (string, []string, error)
 	}
 	if len(n.Pattern) > 0 {
 		args = append(args, "--grep="+n.Pattern)
+		args = append(args, fmt.Sprintf("--case-sensitive=%t", n.CaseSensitive))
 	}
 
 	if n.Boot != nil {
 		args = append(args, "--boot", fmt.Sprintf("%d", *n.Boot))
 	}
 
+	var output string
+	if len(n.Format) > 0 {
+		output = n.Format
+	} else {
+		output = "short-precise"
+	}
+	args = append(args, fmt.Sprintf("--output=%s", output))
+
 	return "journalctl", args, nil
 }
 

pkg/kubelet/kubelet_server_journal_test.go

@@ -78,18 +78,18 @@ func Test_newNodeLogQuery(t *testing.T) {
 		want    *nodeLogQuery
 		wantErr bool
 	}{
-		{name: "empty", query: url.Values{}, want: nil},
-		{query: url.Values{"unknown": []string{"true"}}, want: nil},
+		{name: "empty", query: url.Values{}, want: &nodeLogQuery{}},
+		{query: url.Values{"unknown": []string{"true"}}, want: &nodeLogQuery{}},
 
-		{query: url.Values{"sinceTime": []string{""}}, want: nil},
+		{query: url.Values{"sinceTime": []string{""}}, want: &nodeLogQuery{}},
 		{query: url.Values{"sinceTime": []string{"2019-12-04 02:00:00"}}, wantErr: true},
 		{query: url.Values{"sinceTime": []string{"2019-12-04 02:00:00.000"}}, wantErr: true},
 		{query: url.Values{"sinceTime": []string{"2019-12-04 02"}}, wantErr: true},
 		{query: url.Values{"sinceTime": []string{"2019-12-04 02:00"}}, wantErr: true},
 		{query: url.Values{"sinceTime": []string{validTimeValue}},
 			want: &nodeLogQuery{options: options{SinceTime: &validT}}},
 
-		{query: url.Values{"untilTime": []string{""}}, want: nil},
+		{query: url.Values{"untilTime": []string{""}}, want: &nodeLogQuery{}},
 		{query: url.Values{"untilTime": []string{"2019-12-04 02:00:00"}}, wantErr: true},
 		{query: url.Values{"untilTime": []string{"2019-12-04 02:00:00.000"}}, wantErr: true},
 		{query: url.Values{"untilTime": []string{"2019-12-04 02"}}, wantErr: true},
@@ -103,7 +103,6 @@ func Test_newNodeLogQuery(t *testing.T) {
 
 		{query: url.Values{"pattern": []string{"foo"}}, want: &nodeLogQuery{options: options{Pattern: "foo"}}},
 
-		{query: url.Values{"boot": []string{""}}, want: nil},
 		{query: url.Values{"boot": []string{"0"}}, want: &nodeLogQuery{options: options{Boot: ptr.To(0)}}},
 		{query: url.Values{"boot": []string{"-23"}}, want: &nodeLogQuery{options: options{Boot: ptr.To(-23)}}},
 		{query: url.Values{"boot": []string{"foo"}}, wantErr: true},
@@ -116,6 +115,11 @@ func Test_newNodeLogQuery(t *testing.T) {
 		{query: url.Values{"query": []string{"foo", "/bar"}}, want: &nodeLogQuery{Services: []string{"foo"},
 			Files: []string{"/bar"}}},
 		{query: url.Values{"query": []string{"/foo", `\bar`}}, want: &nodeLogQuery{Files: []string{"/foo", `\bar`}}},
+		{query: url.Values{"unit": []string{""}}, wantErr: true},
+		{query: url.Values{"unit": []string{"   ", "    "}}, wantErr: true},
+		{query: url.Values{"unit": []string{"foo"}}, want: &nodeLogQuery{Services: []string{"foo"}}},
+		{query: url.Values{"unit": []string{"foo", "bar"}}, want: &nodeLogQuery{Services: []string{"foo", "bar"}}},
+		{query: url.Values{"unit": []string{"foo", "/bar"}}, want: &nodeLogQuery{Services: []string{"foo", "/bar"}}},
 	}
 	for _, tt := range tests {
 		t.Run(tt.query.Encode(), func(t *testing.T) {
@@ -180,10 +184,12 @@ func Test_nodeLogQuery_validate(t *testing.T) {
 		pattern  = "foo"
 		invalid  = "foo\\"
 	)
-	since, err := time.Parse(time.RFC3339, "2023-01-04T02:00:00Z")
+	sinceTime, err := time.Parse(time.RFC3339, "2023-01-04T02:00:00Z")
 	assert.NoError(t, err)
-	until, err := time.Parse(time.RFC3339, "2023-02-04T02:00:00Z")
+	untilTime, err := time.Parse(time.RFC3339, "2023-02-04T02:00:00Z")
 	assert.NoError(t, err)
+	since := "2019-12-04 02:00:00"
+	until := "2019-12-04 03:00:00"
 
 	tests := []struct {
 		name     string
@@ -192,23 +198,37 @@ func Test_nodeLogQuery_validate(t *testing.T) {
 		options  options
 		wantErr  bool
 	}{
-		{name: "empty", wantErr: true},
-		{name: "empty with options", options: options{SinceTime: &since}, wantErr: true},
+		{name: "empty"},
+		{name: "empty with options", options: options{SinceTime: &sinceTime}},
 		{name: "one service", Services: []string{service1}},
 		{name: "two services", Services: []string{service1, service2}},
 		{name: "one service one file", Services: []string{service1}, Files: []string{file1}, wantErr: true},
 		{name: "two files", Files: []string{file1, file2}, wantErr: true},
 		{name: "one file options", Files: []string{file1}, options: options{Pattern: pattern}, wantErr: true},
 		{name: "invalid pattern", Services: []string{service1}, options: options{Pattern: invalid}, wantErr: true},
-		{name: "since", Services: []string{service1}, options: options{SinceTime: &since}},
-		{name: "until", Services: []string{service1}, options: options{UntilTime: &until}},
-		{name: "since until", Services: []string{service1}, options: options{SinceTime: &until, UntilTime: &since},
-			wantErr: true},
-		// boot is not supported on Windows.
-		{name: "boot", Services: []string{service1}, options: options{Boot: ptr.To(-1)}, wantErr: runtime.GOOS == "windows"},
+		{name: "sinceTime", Services: []string{service1}, options: options{SinceTime: &sinceTime}},
+		{name: "untilTime", Services: []string{service1}, options: options{UntilTime: &untilTime}},
+		{name: "sinceTime untilTime", Services: []string{service1}, options: options{SinceTime: &untilTime,
+			UntilTime: &sinceTime}, wantErr: true},
+		{name: "boot", Services: []string{service1}, options: options{Boot: ptr.To(-1)}},
 		{name: "boot out of range", Services: []string{service1}, options: options{Boot: ptr.To(1)}, wantErr: true},
 		{name: "tailLines", Services: []string{service1}, options: options{TailLines: ptr.To(100)}},
 		{name: "tailLines out of range", Services: []string{service1}, options: options{TailLines: ptr.To(100000)}},
+		{name: "since", Services: []string{service1}, options: options{ocAdm: ocAdm{Since: since}}},
+		{name: "since RFC3339", Services: []string{service1}, options: options{ocAdm: ocAdm{Since: sinceTime.String()}}, wantErr: true},
+		{name: "until", Services: []string{service1}, options: options{ocAdm: ocAdm{Until: until}}},
+		{name: "until RFC3339", Services: []string{service1}, options: options{ocAdm: ocAdm{Until: untilTime.String()}}, wantErr: true},
+		{name: "since sinceTime", Services: []string{service1}, options: options{SinceTime: &sinceTime,
+			ocAdm: ocAdm{Since: since}}, wantErr: true},
+		{name: "until sinceTime", Services: []string{service1}, options: options{SinceTime: &sinceTime,
+			ocAdm: ocAdm{Until: until}}, wantErr: true},
+		{name: "since untilTime", Services: []string{service1}, options: options{UntilTime: &untilTime,
+			ocAdm: ocAdm{Since: since}}, wantErr: true},
+		{name: "until untilTime", Services: []string{service1}, options: options{UntilTime: &untilTime,
+			ocAdm: ocAdm{Until: until}}, wantErr: true},
+		{name: "format", Services: []string{service1}, options: options{ocAdm: ocAdm{Format: "cat"}}},
+		{name: "format invalid", Services: []string{service1}, options: options{ocAdm: ocAdm{Format: "foo"}},
+			wantErr: true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {