Skip to content

Commit ca453a3

Browse files
authored
#52217 - VAI: Pod Restart Shows Stale Value (#1011)
* initial implementation * added tests * moved to use a more generic approach * added conditional for multivalue converter * updated tests * improved unit tests * made no index being default subindex 0 * added tests * fixed formatter * better naming * moved to separate fields * added tests for timestamp * fixing unit tests * better error log + matchesGVK as common * updated to have compositeInt * addressing more comments * addressing more comments * removed watchers var * changed formatter to be more efficient * introducing indexed fields * removed composite int and fixed tests * changed to flat field definition + maps for optimization * addressed comments from tomleb * simplified the logic * addressed comments from @tomleb * reduced complexity * removed integer * removed COMPOSITE_INT references * changed podrestarts test to use struct * called getTypeGuidance first * removed obvious comments * removed more unused comments * fixed formatting * fixed formatting again * fixed tests and formatting * goimports * removed unused param by @tomleb * fixed format
1 parent 360a853 commit ca453a3

29 files changed

+1547
-481
lines changed

pkg/resources/common/duration_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func TestParseHumanReadableDuration(t *testing.T) {
1717
{
1818
name: "years + days",
1919
input: "2y10d",
20-
expected: (2*365+10)*24*time.Hour,
20+
expected: (2*365 + 10) * 24 * time.Hour,
2121
},
2222
{
2323
name: "years + days + hours + mins + secs",

pkg/resources/common/formatter.go

Lines changed: 108 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/rancher/apiserver/pkg/types"
1111
"github.com/rancher/steve/pkg/accesscontrol"
1212
"github.com/rancher/steve/pkg/attributes"
13+
"github.com/rancher/steve/pkg/resources/common/formatters"
1314
"github.com/rancher/steve/pkg/resources/virtual/common"
1415
"github.com/sirupsen/logrus"
1516

@@ -180,6 +181,7 @@ func formatter(summarycache common.SummaryCache, asl accesscontrol.AccessSetLook
180181

181182
if options.InSQLMode {
182183
isCRD := attributes.IsCRD(resource.Schema)
184+
convertMetadataMultiValueFields(request, gvk, unstr)
183185
convertMetadataTimestampFields(request, gvk, unstr, isCRD)
184186
}
185187
}
@@ -237,71 +239,126 @@ func excludeFields(request *types.APIRequest, unstr *unstructured.Unstructured)
237239
}
238240
}
239241

242+
// convertMetadataMultiValueFields converts multi-value JSON arrays back to display strings
243+
func convertMetadataMultiValueFields(request *types.APIRequest, gvk schema2.GroupVersionKind, unstr *unstructured.Unstructured) {
244+
if request.Schema == nil {
245+
return
246+
}
247+
248+
if gvk != PodGVK {
249+
return
250+
}
251+
252+
curValue, got, err := unstructured.NestedSlice(unstr.Object, "metadata", "fields")
253+
if err != nil || !got {
254+
return
255+
}
256+
257+
changedFields := false
258+
cols := GetColumnDefinitions(request.Schema)
259+
for _, col := range cols {
260+
index := GetIndexValueFromString(col.Field)
261+
if index == -1 {
262+
continue
263+
}
264+
265+
if index >= len(curValue) {
266+
continue
267+
}
268+
269+
arr, ok := curValue[index].([]interface{})
270+
if !ok || len(arr) < 2 {
271+
continue
272+
}
273+
274+
if col.Name == "Restarts" && gvk == PodGVK {
275+
curValue[index] = formatters.FormatRestarts(arr)
276+
changedFields = true
277+
break
278+
}
279+
280+
// Future: Add other multi-value field formatters here
281+
}
282+
283+
if changedFields {
284+
if err := unstructured.SetNestedSlice(unstr.Object, curValue, "metadata", "fields"); err != nil {
285+
logrus.Errorf("failed to set multi-value display value: %s", err.Error())
286+
}
287+
}
288+
}
289+
240290
// convertMetadataTimestampFields updates metadata timestamp fields to ensure they remain fresh and human-readable when sent back
241291
// to the client. Internally, fields are stored as Unix timestamps; on each request, we calculate the elapsed time since
242292
// those timestamps by subtracting them from time.Now(), then format the resulting duration into a human-friendly string.
243293
// This prevents cached durations (e.g. “2d” - 2 days) from becoming stale over time.
244294
func convertMetadataTimestampFields(request *types.APIRequest, gvk schema2.GroupVersionKind, unstr *unstructured.Unstructured, isCRD bool) {
245-
if request.Schema != nil {
246-
cols := GetColumnDefinitions(request.Schema)
247-
for _, col := range cols {
248-
gvkDateFields, gvkFound := DateFieldsByGVK[gvk]
249-
250-
hasCRDDateField := isCRD && col.Type == "date"
251-
hasGVKDateFieldMapping := gvkFound && slices.Contains(gvkDateFields, col.Name)
252-
if hasCRDDateField || hasGVKDateFieldMapping {
253-
index := GetIndexValueFromString(col.Field)
254-
if index == -1 {
255-
logrus.Errorf("field index not found at column.Field struct variable: %s", col.Field)
256-
return
257-
}
295+
if request.Schema == nil {
296+
return
297+
}
258298

259-
curValue, got, err := unstructured.NestedSlice(unstr.Object, "metadata", "fields")
260-
if err != nil {
261-
logrus.Warnf("failed to get metadata.fields slice from unstr.Object: %s", err.Error())
262-
return
263-
}
299+
gvkDateFields, gvkFound := DateFieldsByGVK[gvk]
300+
if !isCRD && !gvkFound {
301+
return
302+
}
264303

265-
if !got {
266-
logrus.Warnf("couldn't find metadata.fields at unstr.Object")
267-
return
268-
}
304+
curValue, got, err := unstructured.NestedSlice(unstr.Object, "metadata", "fields")
305+
if err != nil || !got {
306+
return
307+
}
269308

270-
timeValue, ok := curValue[index].(string)
271-
if !ok {
272-
logrus.Warnf("time field isn't a string")
273-
return
274-
}
309+
changedFields := false
310+
cols := GetColumnDefinitions(request.Schema)
311+
for _, col := range cols {
312+
index := GetIndexValueFromString(col.Field)
313+
if index == -1 {
314+
continue
315+
}
275316

276-
if _, err := time.Parse(time.RFC3339, timeValue); err == nil {
277-
// it's already a timestamp, so we don't need to do anything
278-
return
279-
}
317+
if index >= len(curValue) {
318+
continue
319+
}
280320

281-
dur, ok := isDuration(timeValue)
282-
if !ok {
283-
millis, err := strconv.ParseInt(timeValue, 10, 64)
284-
if err != nil {
285-
logrus.Warnf("convert timestamp value: %s failed with error: %s", timeValue, err.Error())
286-
return
287-
}
321+
hasCRDDateField := isCRD && col.Type == "date"
322+
hasGVKDateFieldMapping := gvkFound && slices.Contains(gvkDateFields, col.Name)
323+
if !hasCRDDateField && !hasGVKDateFieldMapping {
324+
continue
325+
}
288326

289-
timestamp := time.Unix(0, millis*int64(time.Millisecond))
290-
dur = time.Since(timestamp)
291-
}
327+
timeValue, ok := curValue[index].(string)
328+
if !ok {
329+
logrus.Warnf("time field isn't a string")
330+
continue
331+
}
292332

293-
humanDuration := duration.HumanDuration(dur)
294-
if humanDuration == "<invalid>" {
295-
logrus.Warnf("couldn't convert value %d into human duration for column %s", int64(dur), col.Name)
296-
return
297-
}
333+
if _, err := time.Parse(time.RFC3339, timeValue); err == nil {
334+
continue
335+
}
298336

299-
curValue[index] = humanDuration
300-
if err := unstructured.SetNestedSlice(unstr.Object, curValue, "metadata", "fields"); err != nil {
301-
logrus.Errorf("failed to set value back to metadata.fields slice: %s", err.Error())
302-
return
303-
}
337+
dur, ok := isDuration(timeValue)
338+
if !ok {
339+
millis, err := strconv.ParseInt(timeValue, 10, 64)
340+
if err != nil {
341+
logrus.Warnf("convert timestamp value: %s failed with error: %s", timeValue, err.Error())
342+
continue
304343
}
344+
345+
timestamp := time.Unix(0, millis*int64(time.Millisecond))
346+
dur = time.Since(timestamp)
347+
}
348+
349+
humanDuration := duration.HumanDuration(dur)
350+
if humanDuration == "<invalid>" {
351+
logrus.Warnf("couldn't convert value %d into human duration for column %s", int64(dur), col.Name)
352+
continue
353+
}
354+
355+
curValue[index] = humanDuration
356+
changedFields = true
357+
}
358+
359+
if changedFields {
360+
if err := unstructured.SetNestedSlice(unstr.Object, curValue, "metadata", "fields"); err != nil {
361+
logrus.Errorf("failed to set value back to metadata.fields slice: %s", err.Error())
305362
}
306363
}
307364
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package formatters
2+
3+
import (
4+
"fmt"
5+
"time"
6+
7+
"k8s.io/apimachinery/pkg/util/duration"
8+
)
9+
10+
// FormatRestarts formats a restart count array [count, timestamp_ms] as a display string.
11+
// Examples: "0", "4 (3h38m ago)"
12+
func FormatRestarts(values []interface{}) string {
13+
primary := toInt64(values, 0)
14+
secondary := toInt64(values, 1)
15+
16+
if secondary == 0 {
17+
return fmt.Sprintf("%d", primary)
18+
}
19+
20+
// Calculate fresh duration from timestamp
21+
timestamp := time.UnixMilli(secondary)
22+
dur := time.Since(timestamp)
23+
humanDur := duration.HumanDuration(dur)
24+
25+
return fmt.Sprintf("%d (%s ago)", primary, humanDur)
26+
}
27+
28+
// toInt64 safely extracts an int64 from a slice at the given index
29+
func toInt64(arr []interface{}, idx int) int64 {
30+
if idx >= len(arr) || arr[idx] == nil {
31+
return 0
32+
}
33+
switch v := arr[idx].(type) {
34+
case int64:
35+
return v
36+
case int:
37+
return int64(v)
38+
case float64:
39+
return int64(v)
40+
default:
41+
return 0
42+
}
43+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package formatters
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestFormatRestarts(t *testing.T) {
11+
// Use current time and calculate expected results
12+
now := time.Now()
13+
14+
tests := []struct {
15+
name string
16+
input []interface{}
17+
wantContains string // partial match for dynamic time
18+
}{
19+
{
20+
name: "count only",
21+
input: []interface{}{0, nil},
22+
wantContains: "0",
23+
},
24+
{
25+
name: "count with recent timestamp (3 hours ago)",
26+
input: []interface{}{4, float64(now.Add(-3 * time.Hour).UnixMilli())},
27+
wantContains: "4 (3h",
28+
},
29+
{
30+
name: "count with old timestamp (48 hours ago)",
31+
input: []interface{}{10, float64(now.Add(-48 * time.Hour).UnixMilli())},
32+
wantContains: "10 (2d",
33+
},
34+
{
35+
name: "count with very recent timestamp (5 minutes ago)",
36+
input: []interface{}{2, float64(now.Add(-5 * time.Minute).UnixMilli())},
37+
wantContains: "2 (5m",
38+
},
39+
}
40+
41+
for _, tt := range tests {
42+
t.Run(tt.name, func(t *testing.T) {
43+
result := FormatRestarts(tt.input)
44+
assert.Contains(t, result, tt.wantContains)
45+
})
46+
}
47+
}

pkg/resources/common/util.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/rancher/apiserver/pkg/types"
88
"github.com/rancher/steve/pkg/attributes"
9+
"k8s.io/apimachinery/pkg/runtime/schema"
910
)
1011

1112
// GetIndexValueFromString looks for values between [ ].
@@ -39,3 +40,11 @@ func GetColumnDefinitions(schema *types.APISchema) []ColumnDefinition {
3940
}
4041
return colDefs
4142
}
43+
44+
// Common GroupVersionKind definitions used throughout the codebase
45+
var (
46+
PodGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}
47+
EventGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Event"}
48+
MgmtClusterGVK = schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "Cluster"}
49+
ProvisioningClusterGVK = schema.GroupVersionKind{Group: "provisioning.cattle.io", Version: "v1", Kind: "Cluster"}
50+
)
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package multivalue
2+
3+
import (
4+
rescommon "github.com/rancher/steve/pkg/resources/common"
5+
"github.com/sirupsen/logrus"
6+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
7+
)
8+
9+
// ParseFunc transforms a string value into a multi-value array
10+
type ParseFunc func(string) ([]interface{}, error)
11+
12+
// FieldConfig defines a multi-value field transformation
13+
type FieldConfig struct {
14+
ColumnName string
15+
ParseFunc ParseFunc
16+
}
17+
18+
// Converter transforms multi-value fields into JSON arrays for storage
19+
type Converter struct {
20+
Columns []rescommon.ColumnDefinition
21+
Fields []FieldConfig
22+
}
23+
24+
// Transform processes metadata.fields and converts registered multi-value fields
25+
func (c *Converter) Transform(obj *unstructured.Unstructured) (*unstructured.Unstructured, error) {
26+
fields, got, err := unstructured.NestedSlice(obj.Object, "metadata", "fields")
27+
if err != nil || !got {
28+
return obj, err
29+
}
30+
31+
updated := false
32+
33+
for _, fieldConfig := range c.Fields {
34+
index := findColumnIndex(c.Columns, fieldConfig.ColumnName)
35+
if index == -1 || index >= len(fields) {
36+
continue
37+
}
38+
39+
if fields[index] == nil {
40+
continue
41+
}
42+
43+
value, ok := fields[index].(string)
44+
if !ok {
45+
continue
46+
}
47+
48+
arrayValue, err := fieldConfig.ParseFunc(value)
49+
if err != nil {
50+
logrus.Debugf("failed to parse %s value %q: %v", fieldConfig.ColumnName, value, err)
51+
continue
52+
}
53+
54+
fields[index] = arrayValue
55+
updated = true
56+
}
57+
58+
if updated {
59+
if err := unstructured.SetNestedSlice(obj.Object, fields, "metadata", "fields"); err != nil {
60+
return obj, err
61+
}
62+
}
63+
64+
return obj, nil
65+
}
66+
67+
// findColumnIndex finds the index of a column by name
68+
func findColumnIndex(columns []rescommon.ColumnDefinition, columnName string) int {
69+
for _, col := range columns {
70+
if col.Name == columnName {
71+
return rescommon.GetIndexValueFromString(col.Field)
72+
}
73+
}
74+
return -1
75+
}

0 commit comments

Comments
 (0)