From b12b5efe1a836be245d58ccf8b628894d89b43dc Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Wed, 11 Sep 2024 22:43:27 -0400 Subject: [PATCH 01/14] Add container IDX column Add a new index column for containers, which is the index in either the InitContainer or Containers. --- internal/dao/container.go | 11 +++++---- internal/render/container.go | 18 ++++++++++++-- internal/render/container_test.go | 41 ++++++++++++++++++++++++++++++- internal/view/container.go | 1 + internal/view/container_test.go | 2 +- 5 files changed, 64 insertions(+), 9 deletions(-) diff --git a/internal/dao/container.go b/internal/dao/container.go index 2074607f26..fd48e12af5 100644 --- a/internal/dao/container.go +++ b/internal/dao/container.go @@ -47,11 +47,11 @@ func (c *Container) List(ctx context.Context, _ string) ([]runtime.Object, error return nil, err } res := make([]runtime.Object, 0, len(po.Spec.InitContainers)+len(po.Spec.Containers)) - for _, co := range po.Spec.InitContainers { - res = append(res, makeContainerRes(co, po, cmx[co.Name], true)) + for i, co := range po.Spec.InitContainers { + res = append(res, makeContainerRes(co, po, cmx[co.Name], true, i)) } - for _, co := range po.Spec.Containers { - res = append(res, makeContainerRes(co, po, cmx[co.Name], false)) + for i, co := range po.Spec.Containers { + res = append(res, makeContainerRes(co, po, cmx[co.Name], false, i)) } return res, nil @@ -68,12 +68,13 @@ func (c *Container) TailLogs(ctx context.Context, opts *LogOptions) ([]LogChan, // ---------------------------------------------------------------------------- // Helpers... -func makeContainerRes(co v1.Container, po *v1.Pod, cmx *mv1beta1.ContainerMetrics, isInit bool) render.ContainerRes { +func makeContainerRes(co v1.Container, po *v1.Pod, cmx *mv1beta1.ContainerMetrics, isInit bool, index int) render.ContainerRes { return render.ContainerRes{ Container: &co, Status: getContainerStatus(co.Name, po.Status), MX: cmx, IsInit: isInit, + Index: index, Age: po.GetCreationTimestamp(), } } diff --git a/internal/render/container.go b/internal/render/container.go index 35f700f267..a5081fed1c 100644 --- a/internal/render/container.go +++ b/internal/render/container.go @@ -72,6 +72,7 @@ func (c Container) ColorerFunc() model1.ColorerFunc { // Header returns a header row. func (Container) Header(ns string) model1.Header { return model1.Header{ + model1.HeaderColumn{Name: "IDX", Align: tview.AlignRight}, model1.HeaderColumn{Name: "NAME"}, model1.HeaderColumn{Name: "PF"}, model1.HeaderColumn{Name: "IMAGE"}, @@ -107,8 +108,9 @@ func (c Container) Render(o interface{}, name string, r *model1.Row) error { ready, state, restarts = boolToStr(co.Status.Ready), ToContainerState(co.Status.State), strconv.Itoa(int(co.Status.RestartCount)) } - r.ID = co.Container.Name + r.ID = containerIndexLabel(co.IsInit, co.Index) r.Fields = model1.Fields{ + containerIndexLabel(co.IsInit, co.Index), co.Container.Name, "●", co.Container.Image, @@ -236,13 +238,25 @@ func probe(p *v1.Probe) string { return on } +func containerIndexLabel(isInit bool, index int) string { + var containerType string + if isInit { + containerType = "\u24D8" // encircled i (sorts first) + } else { + containerType = "\u2800" // blank (sorts last) + } + return fmt.Sprintf("%s %d", containerType, index) +} + // ContainerRes represents a container and its metrics. type ContainerRes struct { Container *v1.Container Status *v1.ContainerStatus MX *mv1beta1.ContainerMetrics IsInit bool - Age metav1.Time + // Index is the container's index in either the Containers or InitContainers. + Index int + Age metav1.Time } // GetObjectKind returns a schema object. diff --git a/internal/render/container_test.go b/internal/render/container_test.go index f790b3a541..8eba1c745b 100644 --- a/internal/render/container_test.go +++ b/internal/render/container_test.go @@ -29,8 +29,9 @@ func TestContainer(t *testing.T) { } var r model1.Row assert.Nil(t, c.Render(cres, "blee", &r)) - assert.Equal(t, "fred", r.ID) + assert.Equal(t, "⠀ 0", r.ID) assert.Equal(t, model1.Fields{ + "⠀ 0", "fred", "●", "img", @@ -54,6 +55,44 @@ func TestContainer(t *testing.T) { ) } +func TestInitContainer(t *testing.T) { + var c render.Container + + cres := render.ContainerRes{ + Container: makeContainer(), + Status: makeContainerStatus(), + MX: makeContainerMetrics(), + IsInit: true, + Age: makeAge(), + } + var r model1.Row + assert.Nil(t, c.Render(cres, "blee", &r)) + assert.Equal(t, "ⓘ 0", r.ID) + assert.Equal(t, model1.Fields{ + "ⓘ 0", + "fred", + "●", + "img", + "false", + "Running", + "true", + "0", + "off:off", + "10", + "20", + "20:20", + "100:100", + "50", + "50", + "20", + "20", + "", + "container is not ready", + }, + r.Fields[:len(r.Fields)-1], + ) +} + func BenchmarkContainerRender(b *testing.B) { var c render.Container diff --git a/internal/view/container.go b/internal/view/container.go index 01f4f1e5e5..a277d9afb3 100644 --- a/internal/view/container.go +++ b/internal/view/container.go @@ -90,6 +90,7 @@ func (c *Container) bindKeys(aa *ui.KeyActions) { ui.KeyF: ui.NewKeyAction("Show PortForward", c.showPFCmd, true), ui.KeyShiftF: ui.NewKeyAction("PortForward", c.portFwdCmd, true), ui.KeyShiftT: ui.NewKeyAction("Sort Restart", c.GetTable().SortColCmd("RESTARTS", false), false), + ui.KeyShiftI: ui.NewKeyAction("Sort Index", c.GetTable().SortColCmd("IDX", true), false), }) aa.Merge(resourceSorters(c.GetTable())) } diff --git a/internal/view/container_test.go b/internal/view/container_test.go index cc1133e88a..84787f6d1b 100644 --- a/internal/view/container_test.go +++ b/internal/view/container_test.go @@ -16,5 +16,5 @@ func TestContainerNew(t *testing.T) { assert.Nil(t, c.Init(makeCtx())) assert.Equal(t, "Containers", c.Name()) - assert.Equal(t, 18, len(c.Hints())) + assert.Equal(t, 19, len(c.Hints())) } From 1748dd26ee168b48dd7c9e40328df14836e8d63b Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Wed, 11 Sep 2024 22:44:05 -0400 Subject: [PATCH 02/14] Set container IDX as default sort column --- internal/view/container.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/view/container.go b/internal/view/container.go index a277d9afb3..868e79314f 100644 --- a/internal/view/container.go +++ b/internal/view/container.go @@ -35,6 +35,7 @@ func NewContainer(gvr client.GVR) ResourceViewer { c.GetTable().SetDecorateFn(c.decorateRows) c.AddBindKeysFn(c.bindKeys) c.GetTable().SetDecorateFn(c.portForwardIndicator) + c.GetTable().SetSortCol("IDX", true) return &c } From b13ab6b3f8fc8cbe0501aa3d9c07628089eb4912 Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Wed, 11 Sep 2024 22:44:46 -0400 Subject: [PATCH 03/14] Drop redundant INIT column --- internal/render/container.go | 2 -- internal/render/container_test.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/internal/render/container.go b/internal/render/container.go index a5081fed1c..7cdabe6aa6 100644 --- a/internal/render/container.go +++ b/internal/render/container.go @@ -78,7 +78,6 @@ func (Container) Header(ns string) model1.Header { model1.HeaderColumn{Name: "IMAGE"}, model1.HeaderColumn{Name: "READY"}, model1.HeaderColumn{Name: "STATE"}, - model1.HeaderColumn{Name: "INIT"}, model1.HeaderColumn{Name: "RESTARTS", Align: tview.AlignRight}, model1.HeaderColumn{Name: "PROBES(L:R)"}, model1.HeaderColumn{Name: "CPU", Align: tview.AlignRight, MX: true}, @@ -116,7 +115,6 @@ func (c Container) Render(o interface{}, name string, r *model1.Row) error { co.Container.Image, ready, state, - boolToStr(co.IsInit), restarts, probe(co.Container.LivenessProbe) + ":" + probe(co.Container.ReadinessProbe), toMc(cur.cpu), diff --git a/internal/render/container_test.go b/internal/render/container_test.go index 8eba1c745b..60c8d8bb65 100644 --- a/internal/render/container_test.go +++ b/internal/render/container_test.go @@ -37,7 +37,6 @@ func TestContainer(t *testing.T) { "img", "false", "Running", - "false", "0", "off:off", "10", @@ -75,7 +74,6 @@ func TestInitContainer(t *testing.T) { "img", "false", "Running", - "true", "0", "off:off", "10", From 55806e0044f39a6008408666c1ba5eabda9ee81d Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Wed, 11 Sep 2024 22:45:08 -0400 Subject: [PATCH 04/14] Use index for getContainerStatus look up to avoid name collisions --- internal/dao/container.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/internal/dao/container.go b/internal/dao/container.go index fd48e12af5..fed3100c2c 100644 --- a/internal/dao/container.go +++ b/internal/dao/container.go @@ -71,7 +71,7 @@ func (c *Container) TailLogs(ctx context.Context, opts *LogOptions) ([]LogChan, func makeContainerRes(co v1.Container, po *v1.Pod, cmx *mv1beta1.ContainerMetrics, isInit bool, index int) render.ContainerRes { return render.ContainerRes{ Container: &co, - Status: getContainerStatus(co.Name, po.Status), + Status: getContainerStatus(po.Status, isInit, index), MX: cmx, IsInit: isInit, Index: index, @@ -79,19 +79,11 @@ func makeContainerRes(co v1.Container, po *v1.Pod, cmx *mv1beta1.ContainerMetric } } -func getContainerStatus(co string, status v1.PodStatus) *v1.ContainerStatus { - for _, c := range status.ContainerStatuses { - if c.Name == co { - return &c - } +func getContainerStatus(status v1.PodStatus, isInit bool, index int) *v1.ContainerStatus { + if isInit { + return &status.InitContainerStatuses[index] } - for _, c := range status.InitContainerStatuses { - if c.Name == co { - return &c - } - } - - return nil + return &status.ContainerStatuses[index] } func (c *Container) fetchPod(fqn string) (*v1.Pod, error) { From 35580f4aa8f707b3e243e1bb84d4efb6ce56174b Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Wed, 11 Sep 2024 23:29:20 -0400 Subject: [PATCH 05/14] Add TestContainerRes_IndexLabel --- internal/render/container.go | 26 +++++++++++++----------- internal/render/container_test.go | 33 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/internal/render/container.go b/internal/render/container.go index 7cdabe6aa6..a4c89da33a 100644 --- a/internal/render/container.go +++ b/internal/render/container.go @@ -107,9 +107,9 @@ func (c Container) Render(o interface{}, name string, r *model1.Row) error { ready, state, restarts = boolToStr(co.Status.Ready), ToContainerState(co.Status.State), strconv.Itoa(int(co.Status.RestartCount)) } - r.ID = containerIndexLabel(co.IsInit, co.Index) + r.ID = co.IndexLabel() r.Fields = model1.Fields{ - containerIndexLabel(co.IsInit, co.Index), + co.IndexLabel(), co.Container.Name, "●", co.Container.Image, @@ -236,16 +236,6 @@ func probe(p *v1.Probe) string { return on } -func containerIndexLabel(isInit bool, index int) string { - var containerType string - if isInit { - containerType = "\u24D8" // encircled i (sorts first) - } else { - containerType = "\u2800" // blank (sorts last) - } - return fmt.Sprintf("%s %d", containerType, index) -} - // ContainerRes represents a container and its metrics. type ContainerRes struct { Container *v1.Container @@ -266,3 +256,15 @@ func (c ContainerRes) GetObjectKind() schema.ObjectKind { func (c ContainerRes) DeepCopyObject() runtime.Object { return c } + +// IndexLabel returns a label for the index with a prefix for the container type. +// Sorts so init containers are first. +func (c ContainerRes) IndexLabel() string { + var containerType string + if c.IsInit { + containerType = "\u24D8" // encircled i (sorts first) + } else { + containerType = "\u2800" // blank (sorts last) + } + return fmt.Sprintf("%s %d", containerType, c.Index) +} diff --git a/internal/render/container_test.go b/internal/render/container_test.go index 60c8d8bb65..275aa91017 100644 --- a/internal/render/container_test.go +++ b/internal/render/container_test.go @@ -5,6 +5,8 @@ package render_test import ( "fmt" + "github.com/fvbommel/sortorder" + "sort" "testing" "time" @@ -17,6 +19,37 @@ import ( mv1beta1 "k8s.io/metrics/pkg/apis/metrics/v1beta1" ) +func TestContainerRes_IndexLabel(t *testing.T) { + cresInit0 := render.ContainerRes{ + Index: 0, + IsInit: true, + } + cresInit1 := render.ContainerRes{ + Index: 1, + IsInit: true, + } + cresReg0 := render.ContainerRes{ + Index: 0, + IsInit: false, + } + cresReg1 := render.ContainerRes{ + Index: 1, + IsInit: false, + } + + assert.Equal(t, cresInit0.IndexLabel(), "ⓘ 0") + assert.Equal(t, cresInit1.IndexLabel(), "ⓘ 1") + assert.Equal(t, cresReg0.IndexLabel(), "⠀ 0") + assert.Equal(t, cresReg1.IndexLabel(), "⠀ 1") + + assert.True(t, sort.IsSorted(sortorder.Natural{ + cresInit0.IndexLabel(), + cresInit1.IndexLabel(), + cresReg0.IndexLabel(), + cresReg1.IndexLabel(), + })) +} + func TestContainer(t *testing.T) { var c render.Container From 2af813b89f614cd6fd79a6b5582ff4b19be0cbdd Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Thu, 12 Sep 2024 15:28:40 -0400 Subject: [PATCH 06/14] Resolve codebeat issues: - render.ContainerRes: too many instance variables - dao.makeContainerRes: Too many function arguments By changing Container and Status fields into methods that read from Pod --- internal/dao/container.go | 23 +---------- internal/render/container.go | 59 ++++++++++++++++++++++------ internal/render/container_test.go | 65 +++++++++++++++++++++---------- internal/xray/container.go | 4 +- internal/xray/container_test.go | 27 +++++++++---- internal/xray/pod.go | 12 +++--- 6 files changed, 122 insertions(+), 68 deletions(-) diff --git a/internal/dao/container.go b/internal/dao/container.go index fed3100c2c..7310d29329 100644 --- a/internal/dao/container.go +++ b/internal/dao/container.go @@ -14,7 +14,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" - mv1beta1 "k8s.io/metrics/pkg/apis/metrics/v1beta1" ) var ( @@ -48,10 +47,10 @@ func (c *Container) List(ctx context.Context, _ string) ([]runtime.Object, error } res := make([]runtime.Object, 0, len(po.Spec.InitContainers)+len(po.Spec.Containers)) for i, co := range po.Spec.InitContainers { - res = append(res, makeContainerRes(co, po, cmx[co.Name], true, i)) + res = append(res, render.MakeContainerRes(po, true, i, cmx[co.Name])) } for i, co := range po.Spec.Containers { - res = append(res, makeContainerRes(co, po, cmx[co.Name], false, i)) + res = append(res, render.MakeContainerRes(po, false, i, cmx[co.Name])) } return res, nil @@ -68,24 +67,6 @@ func (c *Container) TailLogs(ctx context.Context, opts *LogOptions) ([]LogChan, // ---------------------------------------------------------------------------- // Helpers... -func makeContainerRes(co v1.Container, po *v1.Pod, cmx *mv1beta1.ContainerMetrics, isInit bool, index int) render.ContainerRes { - return render.ContainerRes{ - Container: &co, - Status: getContainerStatus(po.Status, isInit, index), - MX: cmx, - IsInit: isInit, - Index: index, - Age: po.GetCreationTimestamp(), - } -} - -func getContainerStatus(status v1.PodStatus, isInit bool, index int) *v1.ContainerStatus { - if isInit { - return &status.InitContainerStatuses[index] - } - return &status.ContainerStatuses[index] -} - func (c *Container) fetchPod(fqn string) (*v1.Pod, error) { o, err := c.getFactory().Get("v1/pods", fqn, true, labels.Everything()) if err != nil { diff --git a/internal/render/container.go b/internal/render/container.go index a4c89da33a..b90a192cc9 100644 --- a/internal/render/container.go +++ b/internal/render/container.go @@ -101,22 +101,22 @@ func (c Container) Render(o interface{}, name string, r *model1.Row) error { return fmt.Errorf("expected ContainerRes, but got %T", o) } - cur, res := gatherMetrics(co.Container, co.MX) + cur, res := gatherMetrics(co.Container(), co.MX) ready, state, restarts := "false", MissingValue, "0" - if co.Status != nil { - ready, state, restarts = boolToStr(co.Status.Ready), ToContainerState(co.Status.State), strconv.Itoa(int(co.Status.RestartCount)) + if co.Status() != nil { + ready, state, restarts = boolToStr(co.Status().Ready), ToContainerState(co.Status().State), strconv.Itoa(int(co.Status().RestartCount)) } r.ID = co.IndexLabel() r.Fields = model1.Fields{ co.IndexLabel(), - co.Container.Name, + co.Container().Name, "●", - co.Container.Image, + co.Container().Image, ready, state, restarts, - probe(co.Container.LivenessProbe) + ":" + probe(co.Container.ReadinessProbe), + probe(co.Container().LivenessProbe) + ":" + probe(co.Container().ReadinessProbe), toMc(cur.cpu), toMi(cur.mem), toMc(res.cpu) + ":" + toMc(res.lcpu), @@ -125,7 +125,7 @@ func (c Container) Render(o interface{}, name string, r *model1.Row) error { client.ToPercentageStr(cur.cpu, res.lcpu), client.ToPercentageStr(cur.mem, res.mem), client.ToPercentageStr(cur.mem, res.lmem), - ToContainerPorts(co.Container.Ports), + ToContainerPorts(co.Container().Ports), AsStatus(c.diagnose(state, ready)), ToAge(co.Age), } @@ -236,14 +236,23 @@ func probe(p *v1.Probe) string { return on } +func MakeContainerRes(po *v1.Pod, isInit bool, index int, cmx *mv1beta1.ContainerMetrics) ContainerRes { + return ContainerRes{ + pod: po, + IsInit: isInit, + Index: index, + MX: cmx, + Age: po.GetCreationTimestamp(), + } +} + // ContainerRes represents a container and its metrics. type ContainerRes struct { - Container *v1.Container - Status *v1.ContainerStatus - MX *mv1beta1.ContainerMetrics - IsInit bool + pod *v1.Pod + IsInit bool // Index is the container's index in either the Containers or InitContainers. Index int + MX *mv1beta1.ContainerMetrics Age metav1.Time } @@ -257,6 +266,34 @@ func (c ContainerRes) DeepCopyObject() runtime.Object { return c } +// Container returns the underlying container or init container +func (c ContainerRes) Container() *v1.Container { + if c.IsInit { + if c.Index >= 0 && c.Index < len(c.pod.Spec.InitContainers) { + return &c.pod.Spec.InitContainers[c.Index] + } + return nil + } + if c.Index >= 0 && c.Index < len(c.pod.Spec.Containers) { + return &c.pod.Spec.Containers[c.Index] + } + return nil +} + +// Status returns the underlying container or init container status +func (c ContainerRes) Status() *v1.ContainerStatus { + if c.IsInit { + if c.Index >= 0 && c.Index < len(c.pod.Status.InitContainerStatuses) { + return &c.pod.Status.InitContainerStatuses[c.Index] + } + return nil + } + if c.Index >= 0 && c.Index < len(c.pod.Status.ContainerStatuses) { + return &c.pod.Status.ContainerStatuses[c.Index] + } + return nil +} + // IndexLabel returns a label for the index with a prefix for the container type. // Sorts so init containers are first. func (c ContainerRes) IndexLabel() string { diff --git a/internal/render/container_test.go b/internal/render/container_test.go index 275aa91017..8881c670b6 100644 --- a/internal/render/container_test.go +++ b/internal/render/container_test.go @@ -53,13 +53,13 @@ func TestContainerRes_IndexLabel(t *testing.T) { func TestContainer(t *testing.T) { var c render.Container - cres := render.ContainerRes{ - Container: makeContainer(), - Status: makeContainerStatus(), - MX: makeContainerMetrics(), - IsInit: false, - Age: makeAge(), - } + cres := makeContainerRes( + makeContainer(), + makeContainerStatus(), + makeContainerMetrics(), + false, + makeAge(), + ) var r model1.Row assert.Nil(t, c.Render(cres, "blee", &r)) assert.Equal(t, "⠀ 0", r.ID) @@ -90,13 +90,13 @@ func TestContainer(t *testing.T) { func TestInitContainer(t *testing.T) { var c render.Container - cres := render.ContainerRes{ - Container: makeContainer(), - Status: makeContainerStatus(), - MX: makeContainerMetrics(), - IsInit: true, - Age: makeAge(), - } + cres := makeContainerRes( + makeContainer(), + makeContainerStatus(), + makeContainerMetrics(), + true, + makeAge(), + ) var r model1.Row assert.Nil(t, c.Render(cres, "blee", &r)) assert.Equal(t, "ⓘ 0", r.ID) @@ -127,13 +127,13 @@ func TestInitContainer(t *testing.T) { func BenchmarkContainerRender(b *testing.B) { var c render.Container - cres := render.ContainerRes{ - Container: makeContainer(), - Status: makeContainerStatus(), - MX: makeContainerMetrics(), - IsInit: false, - Age: makeAge(), - } + cres := makeContainerRes( + makeContainer(), + makeContainerStatus(), + makeContainerMetrics(), + false, + makeAge(), + ) var r model1.Row b.ReportAllocs() @@ -146,6 +146,29 @@ func BenchmarkContainerRender(b *testing.B) { // ---------------------------------------------------------------------------- // Helpers... +func makeContainerRes(container *v1.Container, status *v1.ContainerStatus, cmx *mv1beta1.ContainerMetrics, isInit bool, age metav1.Time) render.ContainerRes { + po := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + CreationTimestamp: age, + }, + } + + if isInit { + po.Spec.InitContainers = []v1.Container{*container} + po.Status.InitContainerStatuses = []v1.ContainerStatus{*status} + } else { + po.Spec.Containers = []v1.Container{*container} + po.Status.ContainerStatuses = []v1.ContainerStatus{*status} + } + + return render.MakeContainerRes( + po, + isInit, + 0, + cmx, + ) +} + func toQty(s string) resource.Quantity { q, _ := resource.ParseQuantity(s) return q diff --git a/internal/xray/container.go b/internal/xray/container.go index 50a608607c..384e18bb7d 100644 --- a/internal/xray/container.go +++ b/internal/xray/container.go @@ -31,13 +31,13 @@ func (c *Container) Render(ctx context.Context, ns string, o interface{}) error return fmt.Errorf("no factory found in context") } - root := NewTreeNode("containers", client.FQN(ns, co.Container.Name)) + root := NewTreeNode("containers", client.FQN(ns, co.Container().Name)) parent, ok := ctx.Value(KeyParent).(*TreeNode) if !ok { return fmt.Errorf("Expecting a TreeNode but got %T", ctx.Value(KeyParent)) } pns, _ := client.Namespaced(parent.ID) - c.envRefs(f, root, pns, co.Container) + c.envRefs(f, root, pns, co.Container()) parent.Add(root) return nil diff --git a/internal/xray/container_test.go b/internal/xray/container_test.go index 09d51b7345..030059ee5e 100644 --- a/internal/xray/container_test.go +++ b/internal/xray/container_test.go @@ -36,7 +36,7 @@ func TestCOConfigMapRefs(t *testing.T) { ctx := context.WithValue(context.Background(), xray.KeyParent, root) ctx = context.WithValue(ctx, internal.KeyFactory, makeFactory()) - assert.Nil(t, re.Render(ctx, "", render.ContainerRes{Container: makeCMContainer("c1", false)})) + assert.Nil(t, re.Render(ctx, "", makeContainerRes(makeCMContainer("c1", false)))) assert.Equal(t, xray.MissingRefStatus, root.Children[0].Children[0].Extras[xray.StatusKey]) } @@ -47,37 +47,37 @@ func TestCORefs(t *testing.T) { e string }{ "cm_required": { - co: render.ContainerRes{Container: makeCMContainer("c1", false)}, + co: makeContainerRes(makeCMContainer("c1", false)), level1: 1, level2: 1, e: xray.MissingRefStatus, }, "cm_optional": { - co: render.ContainerRes{Container: makeCMContainer("c1", true)}, + co: makeContainerRes(makeCMContainer("c1", true)), level1: 1, level2: 1, e: xray.OkStatus, }, "cm_doubleRef": { - co: render.ContainerRes{Container: makeDoubleCMKeysContainer("c1", false)}, + co: makeContainerRes(makeDoubleCMKeysContainer("c1", false)), level1: 1, level2: 1, e: xray.MissingRefStatus, }, "sec_required": { - co: render.ContainerRes{Container: makeSecContainer("c1", false)}, + co: makeContainerRes(makeSecContainer("c1", false)), level1: 1, level2: 1, e: xray.MissingRefStatus, }, "sec_optional": { - co: render.ContainerRes{Container: makeSecContainer("c1", true)}, + co: makeContainerRes(makeSecContainer("c1", true)), level1: 1, level2: 1, e: xray.OkStatus, }, "envFrom_optional": { - co: render.ContainerRes{Container: makeCMEnvFromContainer("c1", false)}, + co: makeContainerRes(makeCMEnvFromContainer("c1", false)), level1: 1, level2: 2, e: xray.MissingRefStatus, @@ -250,3 +250,16 @@ func load(t *testing.T, n string) *unstructured.Unstructured { return &o } + +// ---------------------------------------------------------------------------- +// Helpers... + +func makeContainerRes(container *v1.Container) render.ContainerRes { + po := &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{*container}, + }, + } + + return render.MakeContainerRes(po, false, 0, nil) +} diff --git a/internal/xray/pod.go b/internal/xray/pod.go index dbcbf92008..b34dcca7d6 100644 --- a/internal/xray/pod.go +++ b/internal/xray/pod.go @@ -44,7 +44,7 @@ func (p *Pod) Render(ctx context.Context, ns string, o interface{}) error { return fmt.Errorf("Expecting a TreeNode but got %T", ctx.Value(KeyParent)) } - if err := p.containerRefs(ctx, node, po.Namespace, po.Spec); err != nil { + if err := p.containerRefs(ctx, node, po.Namespace, &po); err != nil { return err } p.podVolumeRefs(f, node, po.Namespace, po.Spec.Volumes) @@ -82,16 +82,16 @@ func (p *Pod) validate(node *TreeNode, po v1.Pod) error { return nil } -func (*Pod) containerRefs(ctx context.Context, parent *TreeNode, ns string, spec v1.PodSpec) error { +func (*Pod) containerRefs(ctx context.Context, parent *TreeNode, ns string, po *v1.Pod) error { ctx = context.WithValue(ctx, KeyParent, parent) var cre Container - for i := 0; i < len(spec.InitContainers); i++ { - if err := cre.Render(ctx, ns, render.ContainerRes{Container: &spec.InitContainers[i]}); err != nil { + for i := 0; i < len(po.Spec.InitContainers); i++ { + if err := cre.Render(ctx, ns, render.MakeContainerRes(po, true, i, nil)); err != nil { return err } } - for i := 0; i < len(spec.Containers); i++ { - if err := cre.Render(ctx, ns, render.ContainerRes{Container: &spec.Containers[i]}); err != nil { + for i := 0; i < len(po.Spec.Containers); i++ { + if err := cre.Render(ctx, ns, render.MakeContainerRes(po, false, i, nil)); err != nil { return err } } From 0e70b11427e26b05d59f903b56736899c1833e42 Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Thu, 12 Sep 2024 15:54:00 -0400 Subject: [PATCH 07/14] codebeat: remove optional args from MakeContainerRes --- internal/dao/container.go | 8 ++++++-- internal/render/container.go | 3 +-- internal/render/container_test.go | 5 +++-- internal/xray/container_test.go | 2 +- internal/xray/pod.go | 4 ++-- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/internal/dao/container.go b/internal/dao/container.go index 7310d29329..5b8f6c0cc7 100644 --- a/internal/dao/container.go +++ b/internal/dao/container.go @@ -47,10 +47,14 @@ func (c *Container) List(ctx context.Context, _ string) ([]runtime.Object, error } res := make([]runtime.Object, 0, len(po.Spec.InitContainers)+len(po.Spec.Containers)) for i, co := range po.Spec.InitContainers { - res = append(res, render.MakeContainerRes(po, true, i, cmx[co.Name])) + cr := render.MakeContainerRes(po, true, i) + cr.MX = cmx[co.Name] + res = append(res, cr) } for i, co := range po.Spec.Containers { - res = append(res, render.MakeContainerRes(po, false, i, cmx[co.Name])) + cr := render.MakeContainerRes(po, false, i) + cr.MX = cmx[co.Name] + res = append(res, cr) } return res, nil diff --git a/internal/render/container.go b/internal/render/container.go index b90a192cc9..107f12e94c 100644 --- a/internal/render/container.go +++ b/internal/render/container.go @@ -236,12 +236,11 @@ func probe(p *v1.Probe) string { return on } -func MakeContainerRes(po *v1.Pod, isInit bool, index int, cmx *mv1beta1.ContainerMetrics) ContainerRes { +func MakeContainerRes(po *v1.Pod, isInit bool, index int) ContainerRes { return ContainerRes{ pod: po, IsInit: isInit, Index: index, - MX: cmx, Age: po.GetCreationTimestamp(), } } diff --git a/internal/render/container_test.go b/internal/render/container_test.go index 8881c670b6..9b48bfbe18 100644 --- a/internal/render/container_test.go +++ b/internal/render/container_test.go @@ -161,12 +161,13 @@ func makeContainerRes(container *v1.Container, status *v1.ContainerStatus, cmx * po.Status.ContainerStatuses = []v1.ContainerStatus{*status} } - return render.MakeContainerRes( + cr := render.MakeContainerRes( po, isInit, 0, - cmx, ) + cr.MX = cmx + return cr } func toQty(s string) resource.Quantity { diff --git a/internal/xray/container_test.go b/internal/xray/container_test.go index 030059ee5e..6154909720 100644 --- a/internal/xray/container_test.go +++ b/internal/xray/container_test.go @@ -261,5 +261,5 @@ func makeContainerRes(container *v1.Container) render.ContainerRes { }, } - return render.MakeContainerRes(po, false, 0, nil) + return render.MakeContainerRes(po, false, 0) } diff --git a/internal/xray/pod.go b/internal/xray/pod.go index b34dcca7d6..910330fa35 100644 --- a/internal/xray/pod.go +++ b/internal/xray/pod.go @@ -86,12 +86,12 @@ func (*Pod) containerRefs(ctx context.Context, parent *TreeNode, ns string, po * ctx = context.WithValue(ctx, KeyParent, parent) var cre Container for i := 0; i < len(po.Spec.InitContainers); i++ { - if err := cre.Render(ctx, ns, render.MakeContainerRes(po, true, i, nil)); err != nil { + if err := cre.Render(ctx, ns, render.MakeContainerRes(po, true, i)); err != nil { return err } } for i := 0; i < len(po.Spec.Containers); i++ { - if err := cre.Render(ctx, ns, render.MakeContainerRes(po, false, i, nil)); err != nil { + if err := cre.Render(ctx, ns, render.MakeContainerRes(po, false, i)); err != nil { return err } } From 3b95b126a0de19ed53f498f039e42bcc930d8342 Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Mon, 16 Sep 2024 09:49:55 -0400 Subject: [PATCH 08/14] Revert "codebeat: remove optional args from MakeContainerRes" This reverts commit 0e70b11427e26b05d59f903b56736899c1833e42. --- internal/dao/container.go | 8 ++------ internal/render/container.go | 3 ++- internal/render/container_test.go | 5 ++--- internal/xray/container_test.go | 2 +- internal/xray/pod.go | 4 ++-- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/internal/dao/container.go b/internal/dao/container.go index 5b8f6c0cc7..7310d29329 100644 --- a/internal/dao/container.go +++ b/internal/dao/container.go @@ -47,14 +47,10 @@ func (c *Container) List(ctx context.Context, _ string) ([]runtime.Object, error } res := make([]runtime.Object, 0, len(po.Spec.InitContainers)+len(po.Spec.Containers)) for i, co := range po.Spec.InitContainers { - cr := render.MakeContainerRes(po, true, i) - cr.MX = cmx[co.Name] - res = append(res, cr) + res = append(res, render.MakeContainerRes(po, true, i, cmx[co.Name])) } for i, co := range po.Spec.Containers { - cr := render.MakeContainerRes(po, false, i) - cr.MX = cmx[co.Name] - res = append(res, cr) + res = append(res, render.MakeContainerRes(po, false, i, cmx[co.Name])) } return res, nil diff --git a/internal/render/container.go b/internal/render/container.go index 107f12e94c..b90a192cc9 100644 --- a/internal/render/container.go +++ b/internal/render/container.go @@ -236,11 +236,12 @@ func probe(p *v1.Probe) string { return on } -func MakeContainerRes(po *v1.Pod, isInit bool, index int) ContainerRes { +func MakeContainerRes(po *v1.Pod, isInit bool, index int, cmx *mv1beta1.ContainerMetrics) ContainerRes { return ContainerRes{ pod: po, IsInit: isInit, Index: index, + MX: cmx, Age: po.GetCreationTimestamp(), } } diff --git a/internal/render/container_test.go b/internal/render/container_test.go index 9b48bfbe18..8881c670b6 100644 --- a/internal/render/container_test.go +++ b/internal/render/container_test.go @@ -161,13 +161,12 @@ func makeContainerRes(container *v1.Container, status *v1.ContainerStatus, cmx * po.Status.ContainerStatuses = []v1.ContainerStatus{*status} } - cr := render.MakeContainerRes( + return render.MakeContainerRes( po, isInit, 0, + cmx, ) - cr.MX = cmx - return cr } func toQty(s string) resource.Quantity { diff --git a/internal/xray/container_test.go b/internal/xray/container_test.go index 6154909720..030059ee5e 100644 --- a/internal/xray/container_test.go +++ b/internal/xray/container_test.go @@ -261,5 +261,5 @@ func makeContainerRes(container *v1.Container) render.ContainerRes { }, } - return render.MakeContainerRes(po, false, 0) + return render.MakeContainerRes(po, false, 0, nil) } diff --git a/internal/xray/pod.go b/internal/xray/pod.go index 910330fa35..b34dcca7d6 100644 --- a/internal/xray/pod.go +++ b/internal/xray/pod.go @@ -86,12 +86,12 @@ func (*Pod) containerRefs(ctx context.Context, parent *TreeNode, ns string, po * ctx = context.WithValue(ctx, KeyParent, parent) var cre Container for i := 0; i < len(po.Spec.InitContainers); i++ { - if err := cre.Render(ctx, ns, render.MakeContainerRes(po, true, i)); err != nil { + if err := cre.Render(ctx, ns, render.MakeContainerRes(po, true, i, nil)); err != nil { return err } } for i := 0; i < len(po.Spec.Containers); i++ { - if err := cre.Render(ctx, ns, render.MakeContainerRes(po, false, i)); err != nil { + if err := cre.Render(ctx, ns, render.MakeContainerRes(po, false, i, nil)); err != nil { return err } } From 7d52a91f2631b453d93dea245f971790df6d1dc9 Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Mon, 16 Sep 2024 09:50:00 -0400 Subject: [PATCH 09/14] Revert "Resolve codebeat issues:" This reverts commit 2af813b89f614cd6fd79a6b5582ff4b19be0cbdd. --- internal/dao/container.go | 23 ++++++++++- internal/render/container.go | 59 ++++++---------------------- internal/render/container_test.go | 65 ++++++++++--------------------- internal/xray/container.go | 4 +- internal/xray/container_test.go | 27 ++++--------- internal/xray/pod.go | 12 +++--- 6 files changed, 68 insertions(+), 122 deletions(-) diff --git a/internal/dao/container.go b/internal/dao/container.go index 7310d29329..fed3100c2c 100644 --- a/internal/dao/container.go +++ b/internal/dao/container.go @@ -14,6 +14,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + mv1beta1 "k8s.io/metrics/pkg/apis/metrics/v1beta1" ) var ( @@ -47,10 +48,10 @@ func (c *Container) List(ctx context.Context, _ string) ([]runtime.Object, error } res := make([]runtime.Object, 0, len(po.Spec.InitContainers)+len(po.Spec.Containers)) for i, co := range po.Spec.InitContainers { - res = append(res, render.MakeContainerRes(po, true, i, cmx[co.Name])) + res = append(res, makeContainerRes(co, po, cmx[co.Name], true, i)) } for i, co := range po.Spec.Containers { - res = append(res, render.MakeContainerRes(po, false, i, cmx[co.Name])) + res = append(res, makeContainerRes(co, po, cmx[co.Name], false, i)) } return res, nil @@ -67,6 +68,24 @@ func (c *Container) TailLogs(ctx context.Context, opts *LogOptions) ([]LogChan, // ---------------------------------------------------------------------------- // Helpers... +func makeContainerRes(co v1.Container, po *v1.Pod, cmx *mv1beta1.ContainerMetrics, isInit bool, index int) render.ContainerRes { + return render.ContainerRes{ + Container: &co, + Status: getContainerStatus(po.Status, isInit, index), + MX: cmx, + IsInit: isInit, + Index: index, + Age: po.GetCreationTimestamp(), + } +} + +func getContainerStatus(status v1.PodStatus, isInit bool, index int) *v1.ContainerStatus { + if isInit { + return &status.InitContainerStatuses[index] + } + return &status.ContainerStatuses[index] +} + func (c *Container) fetchPod(fqn string) (*v1.Pod, error) { o, err := c.getFactory().Get("v1/pods", fqn, true, labels.Everything()) if err != nil { diff --git a/internal/render/container.go b/internal/render/container.go index b90a192cc9..a4c89da33a 100644 --- a/internal/render/container.go +++ b/internal/render/container.go @@ -101,22 +101,22 @@ func (c Container) Render(o interface{}, name string, r *model1.Row) error { return fmt.Errorf("expected ContainerRes, but got %T", o) } - cur, res := gatherMetrics(co.Container(), co.MX) + cur, res := gatherMetrics(co.Container, co.MX) ready, state, restarts := "false", MissingValue, "0" - if co.Status() != nil { - ready, state, restarts = boolToStr(co.Status().Ready), ToContainerState(co.Status().State), strconv.Itoa(int(co.Status().RestartCount)) + if co.Status != nil { + ready, state, restarts = boolToStr(co.Status.Ready), ToContainerState(co.Status.State), strconv.Itoa(int(co.Status.RestartCount)) } r.ID = co.IndexLabel() r.Fields = model1.Fields{ co.IndexLabel(), - co.Container().Name, + co.Container.Name, "●", - co.Container().Image, + co.Container.Image, ready, state, restarts, - probe(co.Container().LivenessProbe) + ":" + probe(co.Container().ReadinessProbe), + probe(co.Container.LivenessProbe) + ":" + probe(co.Container.ReadinessProbe), toMc(cur.cpu), toMi(cur.mem), toMc(res.cpu) + ":" + toMc(res.lcpu), @@ -125,7 +125,7 @@ func (c Container) Render(o interface{}, name string, r *model1.Row) error { client.ToPercentageStr(cur.cpu, res.lcpu), client.ToPercentageStr(cur.mem, res.mem), client.ToPercentageStr(cur.mem, res.lmem), - ToContainerPorts(co.Container().Ports), + ToContainerPorts(co.Container.Ports), AsStatus(c.diagnose(state, ready)), ToAge(co.Age), } @@ -236,23 +236,14 @@ func probe(p *v1.Probe) string { return on } -func MakeContainerRes(po *v1.Pod, isInit bool, index int, cmx *mv1beta1.ContainerMetrics) ContainerRes { - return ContainerRes{ - pod: po, - IsInit: isInit, - Index: index, - MX: cmx, - Age: po.GetCreationTimestamp(), - } -} - // ContainerRes represents a container and its metrics. type ContainerRes struct { - pod *v1.Pod - IsInit bool + Container *v1.Container + Status *v1.ContainerStatus + MX *mv1beta1.ContainerMetrics + IsInit bool // Index is the container's index in either the Containers or InitContainers. Index int - MX *mv1beta1.ContainerMetrics Age metav1.Time } @@ -266,34 +257,6 @@ func (c ContainerRes) DeepCopyObject() runtime.Object { return c } -// Container returns the underlying container or init container -func (c ContainerRes) Container() *v1.Container { - if c.IsInit { - if c.Index >= 0 && c.Index < len(c.pod.Spec.InitContainers) { - return &c.pod.Spec.InitContainers[c.Index] - } - return nil - } - if c.Index >= 0 && c.Index < len(c.pod.Spec.Containers) { - return &c.pod.Spec.Containers[c.Index] - } - return nil -} - -// Status returns the underlying container or init container status -func (c ContainerRes) Status() *v1.ContainerStatus { - if c.IsInit { - if c.Index >= 0 && c.Index < len(c.pod.Status.InitContainerStatuses) { - return &c.pod.Status.InitContainerStatuses[c.Index] - } - return nil - } - if c.Index >= 0 && c.Index < len(c.pod.Status.ContainerStatuses) { - return &c.pod.Status.ContainerStatuses[c.Index] - } - return nil -} - // IndexLabel returns a label for the index with a prefix for the container type. // Sorts so init containers are first. func (c ContainerRes) IndexLabel() string { diff --git a/internal/render/container_test.go b/internal/render/container_test.go index 8881c670b6..275aa91017 100644 --- a/internal/render/container_test.go +++ b/internal/render/container_test.go @@ -53,13 +53,13 @@ func TestContainerRes_IndexLabel(t *testing.T) { func TestContainer(t *testing.T) { var c render.Container - cres := makeContainerRes( - makeContainer(), - makeContainerStatus(), - makeContainerMetrics(), - false, - makeAge(), - ) + cres := render.ContainerRes{ + Container: makeContainer(), + Status: makeContainerStatus(), + MX: makeContainerMetrics(), + IsInit: false, + Age: makeAge(), + } var r model1.Row assert.Nil(t, c.Render(cres, "blee", &r)) assert.Equal(t, "⠀ 0", r.ID) @@ -90,13 +90,13 @@ func TestContainer(t *testing.T) { func TestInitContainer(t *testing.T) { var c render.Container - cres := makeContainerRes( - makeContainer(), - makeContainerStatus(), - makeContainerMetrics(), - true, - makeAge(), - ) + cres := render.ContainerRes{ + Container: makeContainer(), + Status: makeContainerStatus(), + MX: makeContainerMetrics(), + IsInit: true, + Age: makeAge(), + } var r model1.Row assert.Nil(t, c.Render(cres, "blee", &r)) assert.Equal(t, "ⓘ 0", r.ID) @@ -127,13 +127,13 @@ func TestInitContainer(t *testing.T) { func BenchmarkContainerRender(b *testing.B) { var c render.Container - cres := makeContainerRes( - makeContainer(), - makeContainerStatus(), - makeContainerMetrics(), - false, - makeAge(), - ) + cres := render.ContainerRes{ + Container: makeContainer(), + Status: makeContainerStatus(), + MX: makeContainerMetrics(), + IsInit: false, + Age: makeAge(), + } var r model1.Row b.ReportAllocs() @@ -146,29 +146,6 @@ func BenchmarkContainerRender(b *testing.B) { // ---------------------------------------------------------------------------- // Helpers... -func makeContainerRes(container *v1.Container, status *v1.ContainerStatus, cmx *mv1beta1.ContainerMetrics, isInit bool, age metav1.Time) render.ContainerRes { - po := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - CreationTimestamp: age, - }, - } - - if isInit { - po.Spec.InitContainers = []v1.Container{*container} - po.Status.InitContainerStatuses = []v1.ContainerStatus{*status} - } else { - po.Spec.Containers = []v1.Container{*container} - po.Status.ContainerStatuses = []v1.ContainerStatus{*status} - } - - return render.MakeContainerRes( - po, - isInit, - 0, - cmx, - ) -} - func toQty(s string) resource.Quantity { q, _ := resource.ParseQuantity(s) return q diff --git a/internal/xray/container.go b/internal/xray/container.go index 384e18bb7d..50a608607c 100644 --- a/internal/xray/container.go +++ b/internal/xray/container.go @@ -31,13 +31,13 @@ func (c *Container) Render(ctx context.Context, ns string, o interface{}) error return fmt.Errorf("no factory found in context") } - root := NewTreeNode("containers", client.FQN(ns, co.Container().Name)) + root := NewTreeNode("containers", client.FQN(ns, co.Container.Name)) parent, ok := ctx.Value(KeyParent).(*TreeNode) if !ok { return fmt.Errorf("Expecting a TreeNode but got %T", ctx.Value(KeyParent)) } pns, _ := client.Namespaced(parent.ID) - c.envRefs(f, root, pns, co.Container()) + c.envRefs(f, root, pns, co.Container) parent.Add(root) return nil diff --git a/internal/xray/container_test.go b/internal/xray/container_test.go index 030059ee5e..09d51b7345 100644 --- a/internal/xray/container_test.go +++ b/internal/xray/container_test.go @@ -36,7 +36,7 @@ func TestCOConfigMapRefs(t *testing.T) { ctx := context.WithValue(context.Background(), xray.KeyParent, root) ctx = context.WithValue(ctx, internal.KeyFactory, makeFactory()) - assert.Nil(t, re.Render(ctx, "", makeContainerRes(makeCMContainer("c1", false)))) + assert.Nil(t, re.Render(ctx, "", render.ContainerRes{Container: makeCMContainer("c1", false)})) assert.Equal(t, xray.MissingRefStatus, root.Children[0].Children[0].Extras[xray.StatusKey]) } @@ -47,37 +47,37 @@ func TestCORefs(t *testing.T) { e string }{ "cm_required": { - co: makeContainerRes(makeCMContainer("c1", false)), + co: render.ContainerRes{Container: makeCMContainer("c1", false)}, level1: 1, level2: 1, e: xray.MissingRefStatus, }, "cm_optional": { - co: makeContainerRes(makeCMContainer("c1", true)), + co: render.ContainerRes{Container: makeCMContainer("c1", true)}, level1: 1, level2: 1, e: xray.OkStatus, }, "cm_doubleRef": { - co: makeContainerRes(makeDoubleCMKeysContainer("c1", false)), + co: render.ContainerRes{Container: makeDoubleCMKeysContainer("c1", false)}, level1: 1, level2: 1, e: xray.MissingRefStatus, }, "sec_required": { - co: makeContainerRes(makeSecContainer("c1", false)), + co: render.ContainerRes{Container: makeSecContainer("c1", false)}, level1: 1, level2: 1, e: xray.MissingRefStatus, }, "sec_optional": { - co: makeContainerRes(makeSecContainer("c1", true)), + co: render.ContainerRes{Container: makeSecContainer("c1", true)}, level1: 1, level2: 1, e: xray.OkStatus, }, "envFrom_optional": { - co: makeContainerRes(makeCMEnvFromContainer("c1", false)), + co: render.ContainerRes{Container: makeCMEnvFromContainer("c1", false)}, level1: 1, level2: 2, e: xray.MissingRefStatus, @@ -250,16 +250,3 @@ func load(t *testing.T, n string) *unstructured.Unstructured { return &o } - -// ---------------------------------------------------------------------------- -// Helpers... - -func makeContainerRes(container *v1.Container) render.ContainerRes { - po := &v1.Pod{ - Spec: v1.PodSpec{ - Containers: []v1.Container{*container}, - }, - } - - return render.MakeContainerRes(po, false, 0, nil) -} diff --git a/internal/xray/pod.go b/internal/xray/pod.go index b34dcca7d6..dbcbf92008 100644 --- a/internal/xray/pod.go +++ b/internal/xray/pod.go @@ -44,7 +44,7 @@ func (p *Pod) Render(ctx context.Context, ns string, o interface{}) error { return fmt.Errorf("Expecting a TreeNode but got %T", ctx.Value(KeyParent)) } - if err := p.containerRefs(ctx, node, po.Namespace, &po); err != nil { + if err := p.containerRefs(ctx, node, po.Namespace, po.Spec); err != nil { return err } p.podVolumeRefs(f, node, po.Namespace, po.Spec.Volumes) @@ -82,16 +82,16 @@ func (p *Pod) validate(node *TreeNode, po v1.Pod) error { return nil } -func (*Pod) containerRefs(ctx context.Context, parent *TreeNode, ns string, po *v1.Pod) error { +func (*Pod) containerRefs(ctx context.Context, parent *TreeNode, ns string, spec v1.PodSpec) error { ctx = context.WithValue(ctx, KeyParent, parent) var cre Container - for i := 0; i < len(po.Spec.InitContainers); i++ { - if err := cre.Render(ctx, ns, render.MakeContainerRes(po, true, i, nil)); err != nil { + for i := 0; i < len(spec.InitContainers); i++ { + if err := cre.Render(ctx, ns, render.ContainerRes{Container: &spec.InitContainers[i]}); err != nil { return err } } - for i := 0; i < len(po.Spec.Containers); i++ { - if err := cre.Render(ctx, ns, render.MakeContainerRes(po, false, i, nil)); err != nil { + for i := 0; i < len(spec.Containers); i++ { + if err := cre.Render(ctx, ns, render.ContainerRes{Container: &spec.Containers[i]}); err != nil { return err } } From 3c96b3d95c39c436905c59ff81ddb25e6866511e Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Mon, 16 Sep 2024 11:07:14 -0400 Subject: [PATCH 10/14] Pre-compute container index --- internal/dao/container.go | 49 ++++++++++++------ internal/render/container.go | 22 ++------ internal/render/container_test.go | 83 +++++-------------------------- 3 files changed, 51 insertions(+), 103 deletions(-) diff --git a/internal/dao/container.go b/internal/dao/container.go index fed3100c2c..04a932f48f 100644 --- a/internal/dao/container.go +++ b/internal/dao/container.go @@ -6,7 +6,6 @@ package dao import ( "context" "fmt" - "github.com/derailed/k9s/internal" "github.com/derailed/k9s/internal/client" "github.com/derailed/k9s/internal/render" @@ -46,14 +45,18 @@ func (c *Container) List(ctx context.Context, _ string) ([]runtime.Object, error if err != nil { return nil, err } - res := make([]runtime.Object, 0, len(po.Spec.InitContainers)+len(po.Spec.Containers)) - for i, co := range po.Spec.InitContainers { - res = append(res, makeContainerRes(co, po, cmx[co.Name], true, i)) + + res := make([]runtime.Object, 0, len(po.Spec.InitContainers)+len(po.Spec.EphemeralContainers)+len(po.Spec.Containers)) + containerTypes := map[string][]v1.Container{ + "I": po.Spec.InitContainers, + "E": convertEphemeralContainersToContainers(po.Spec.EphemeralContainers), + "M": po.Spec.Containers, } - for i, co := range po.Spec.Containers { - res = append(res, makeContainerRes(co, po, cmx[co.Name], false, i)) + for prefix, containers := range containerTypes { + for i, co := range containers { + res = append(res, makeContainerRes(fmt.Sprintf("%s%d", prefix, i+1), co, po, cmx[co.Name])) + } } - return res, nil } @@ -68,22 +71,29 @@ func (c *Container) TailLogs(ctx context.Context, opts *LogOptions) ([]LogChan, // ---------------------------------------------------------------------------- // Helpers... -func makeContainerRes(co v1.Container, po *v1.Pod, cmx *mv1beta1.ContainerMetrics, isInit bool, index int) render.ContainerRes { +func makeContainerRes(index string, co v1.Container, po *v1.Pod, cmx *mv1beta1.ContainerMetrics) render.ContainerRes { return render.ContainerRes{ + Index: index, Container: &co, - Status: getContainerStatus(po.Status, isInit, index), + Status: getContainerStatus(co.Name, po.Status), MX: cmx, - IsInit: isInit, - Index: index, Age: po.GetCreationTimestamp(), } } -func getContainerStatus(status v1.PodStatus, isInit bool, index int) *v1.ContainerStatus { - if isInit { - return &status.InitContainerStatuses[index] +func getContainerStatus(co string, status v1.PodStatus) *v1.ContainerStatus { + for _, c := range status.ContainerStatuses { + if c.Name == co { + return &c + } } - return &status.ContainerStatuses[index] + for _, c := range status.InitContainerStatuses { + if c.Name == co { + return &c + } + } + + return nil } func (c *Container) fetchPod(fqn string) (*v1.Pod, error) { @@ -95,3 +105,12 @@ func (c *Container) fetchPod(fqn string) (*v1.Pod, error) { err = runtime.DefaultUnstructuredConverter.FromUnstructured(o.(*unstructured.Unstructured).Object, &po) return &po, err } + +// convertEphemeralContainersToContainers reduces EphemeralContainers to common fields with Containers +func convertEphemeralContainersToContainers(ecos []v1.EphemeralContainer) []v1.Container { + cos := make([]v1.Container, len(ecos)) + for i, eco := range ecos { + cos[i] = v1.Container(eco.EphemeralContainerCommon) + } + return cos +} diff --git a/internal/render/container.go b/internal/render/container.go index a4c89da33a..c33cb26d21 100644 --- a/internal/render/container.go +++ b/internal/render/container.go @@ -107,9 +107,9 @@ func (c Container) Render(o interface{}, name string, r *model1.Row) error { ready, state, restarts = boolToStr(co.Status.Ready), ToContainerState(co.Status.State), strconv.Itoa(int(co.Status.RestartCount)) } - r.ID = co.IndexLabel() + r.ID = co.Index r.Fields = model1.Fields{ - co.IndexLabel(), + co.Index, co.Container.Name, "●", co.Container.Image, @@ -238,13 +238,11 @@ func probe(p *v1.Probe) string { // ContainerRes represents a container and its metrics. type ContainerRes struct { + Index string Container *v1.Container Status *v1.ContainerStatus MX *mv1beta1.ContainerMetrics - IsInit bool - // Index is the container's index in either the Containers or InitContainers. - Index int - Age metav1.Time + Age metav1.Time } // GetObjectKind returns a schema object. @@ -256,15 +254,3 @@ func (c ContainerRes) GetObjectKind() schema.ObjectKind { func (c ContainerRes) DeepCopyObject() runtime.Object { return c } - -// IndexLabel returns a label for the index with a prefix for the container type. -// Sorts so init containers are first. -func (c ContainerRes) IndexLabel() string { - var containerType string - if c.IsInit { - containerType = "\u24D8" // encircled i (sorts first) - } else { - containerType = "\u2800" // blank (sorts last) - } - return fmt.Sprintf("%s %d", containerType, c.Index) -} diff --git a/internal/render/container_test.go b/internal/render/container_test.go index 275aa91017..fc94a904ee 100644 --- a/internal/render/container_test.go +++ b/internal/render/container_test.go @@ -5,7 +5,6 @@ package render_test import ( "fmt" - "github.com/fvbommel/sortorder" "sort" "testing" "time" @@ -19,89 +18,33 @@ import ( mv1beta1 "k8s.io/metrics/pkg/apis/metrics/v1beta1" ) -func TestContainerRes_IndexLabel(t *testing.T) { - cresInit0 := render.ContainerRes{ - Index: 0, - IsInit: true, - } - cresInit1 := render.ContainerRes{ - Index: 1, - IsInit: true, - } - cresReg0 := render.ContainerRes{ - Index: 0, - IsInit: false, - } - cresReg1 := render.ContainerRes{ - Index: 1, - IsInit: false, - } +func TestContainerRes_IndexSorting(t *testing.T) { + ephm1 := model1.Row{Fields: []string{"E1"}} + ephm10 := model1.Row{Fields: []string{"E10"}} + init1 := model1.Row{Fields: []string{"I1"}} + init2 := model1.Row{Fields: []string{"I2"}} + main1 := model1.Row{Fields: []string{"M1"}} - assert.Equal(t, cresInit0.IndexLabel(), "ⓘ 0") - assert.Equal(t, cresInit1.IndexLabel(), "ⓘ 1") - assert.Equal(t, cresReg0.IndexLabel(), "⠀ 0") - assert.Equal(t, cresReg1.IndexLabel(), "⠀ 1") - - assert.True(t, sort.IsSorted(sortorder.Natural{ - cresInit0.IndexLabel(), - cresInit1.IndexLabel(), - cresReg0.IndexLabel(), - cresReg1.IndexLabel(), - })) + rows := model1.Rows{ephm1, ephm10, init1, init2, main1} + sorter := model1.RowSorter{Rows: rows, Index: 0, Asc: true} + assert.True(t, sort.IsSorted(sorter)) } func TestContainer(t *testing.T) { var c render.Container cres := render.ContainerRes{ + Index: "M1", Container: makeContainer(), Status: makeContainerStatus(), MX: makeContainerMetrics(), - IsInit: false, - Age: makeAge(), - } - var r model1.Row - assert.Nil(t, c.Render(cres, "blee", &r)) - assert.Equal(t, "⠀ 0", r.ID) - assert.Equal(t, model1.Fields{ - "⠀ 0", - "fred", - "●", - "img", - "false", - "Running", - "0", - "off:off", - "10", - "20", - "20:20", - "100:100", - "50", - "50", - "20", - "20", - "", - "container is not ready", - }, - r.Fields[:len(r.Fields)-1], - ) -} - -func TestInitContainer(t *testing.T) { - var c render.Container - - cres := render.ContainerRes{ - Container: makeContainer(), - Status: makeContainerStatus(), - MX: makeContainerMetrics(), - IsInit: true, Age: makeAge(), } var r model1.Row assert.Nil(t, c.Render(cres, "blee", &r)) - assert.Equal(t, "ⓘ 0", r.ID) + assert.Equal(t, "M1", r.ID) assert.Equal(t, model1.Fields{ - "ⓘ 0", + "M1", "fred", "●", "img", @@ -128,10 +71,10 @@ func BenchmarkContainerRender(b *testing.B) { var c render.Container cres := render.ContainerRes{ + Index: "M1", Container: makeContainer(), Status: makeContainerStatus(), MX: makeContainerMetrics(), - IsInit: false, Age: makeAge(), } var r model1.Row From 4e3856a306a8f77e6b15cb05bbdbbef858aeaace Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Mon, 16 Sep 2024 11:24:36 -0400 Subject: [PATCH 11/14] Pre-extract container status --- internal/dao/container.go | 62 +++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/internal/dao/container.go b/internal/dao/container.go index 04a932f48f..c992418ffb 100644 --- a/internal/dao/container.go +++ b/internal/dao/container.go @@ -10,6 +10,7 @@ import ( "github.com/derailed/k9s/internal/client" "github.com/derailed/k9s/internal/render" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -46,15 +47,39 @@ func (c *Container) List(ctx context.Context, _ string) ([]runtime.Object, error return nil, err } - res := make([]runtime.Object, 0, len(po.Spec.InitContainers)+len(po.Spec.EphemeralContainers)+len(po.Spec.Containers)) - containerTypes := map[string][]v1.Container{ - "I": po.Spec.InitContainers, - "E": convertEphemeralContainersToContainers(po.Spec.EphemeralContainers), - "M": po.Spec.Containers, + type containerGroup struct { + indexPrefix string + containers []v1.Container + statuses []v1.ContainerStatus + } + containerGroups := []containerGroup{ + { + indexPrefix: "E", + containers: convertEphemeralContainersToContainers(po.Spec.EphemeralContainers), + statuses: po.Status.EphemeralContainerStatuses, + }, + { + indexPrefix: "I", + containers: po.Spec.InitContainers, + statuses: po.Status.InitContainerStatuses, + }, + { + indexPrefix: "M", + containers: po.Spec.Containers, + statuses: po.Status.ContainerStatuses, + }, } - for prefix, containers := range containerTypes { - for i, co := range containers { - res = append(res, makeContainerRes(fmt.Sprintf("%s%d", prefix, i+1), co, po, cmx[co.Name])) + + res := make([]runtime.Object, 0, len(po.Spec.InitContainers)+len(po.Spec.EphemeralContainers)+len(po.Spec.Containers)) + for _, group := range containerGroups { + for i := range group.containers { + res = append(res, makeContainerRes( + fmt.Sprintf("%s%d", group.indexPrefix, i+1), + group.containers[i], + group.statuses[i], + cmx[group.containers[i].Name], + po.GetCreationTimestamp(), + )) } } return res, nil @@ -71,31 +96,16 @@ func (c *Container) TailLogs(ctx context.Context, opts *LogOptions) ([]LogChan, // ---------------------------------------------------------------------------- // Helpers... -func makeContainerRes(index string, co v1.Container, po *v1.Pod, cmx *mv1beta1.ContainerMetrics) render.ContainerRes { +func makeContainerRes(index string, co v1.Container, status v1.ContainerStatus, cmx *mv1beta1.ContainerMetrics, age metav1.Time) render.ContainerRes { return render.ContainerRes{ Index: index, Container: &co, - Status: getContainerStatus(co.Name, po.Status), + Status: &status, MX: cmx, - Age: po.GetCreationTimestamp(), + Age: age, } } -func getContainerStatus(co string, status v1.PodStatus) *v1.ContainerStatus { - for _, c := range status.ContainerStatuses { - if c.Name == co { - return &c - } - } - for _, c := range status.InitContainerStatuses { - if c.Name == co { - return &c - } - } - - return nil -} - func (c *Container) fetchPod(fqn string) (*v1.Pod, error) { o, err := c.getFactory().Get("v1/pods", fqn, true, labels.Everything()) if err != nil { From a20ab06396e33ac67a30d591436d0134e8c2ffa5 Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Mon, 16 Sep 2024 11:35:58 -0400 Subject: [PATCH 12/14] Flesh out TestContainerList --- internal/dao/container_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/dao/container_test.go b/internal/dao/container_test.go index f53c82959d..43b698dfb5 100644 --- a/internal/dao/container_test.go +++ b/internal/dao/container_test.go @@ -5,6 +5,7 @@ package dao_test import ( "context" + "github.com/derailed/k9s/internal/render" "testing" "github.com/derailed/k9s/internal" @@ -34,6 +35,12 @@ func TestContainerList(t *testing.T) { oo, err := c.List(ctx, "") assert.Nil(t, err) assert.Equal(t, 1, len(oo)) + assert.IsType(t, render.ContainerRes{}, oo[0]) + cres := oo[0].(render.ContainerRes) + assert.Equal(t, "M1", cres.Index) + assert.Equal(t, "fred", cres.Container.Name) + assert.Equal(t, "fred", cres.Status.Name) + assert.NotEmpty(t, cres.Age) } // ---------------------------------------------------------------------------- From a44811897806193f44488729baa112566a228454 Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Tue, 17 Sep 2024 10:18:05 -0400 Subject: [PATCH 13/14] Handle container list when ContainerStatus is not set yet --- internal/dao/container.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/internal/dao/container.go b/internal/dao/container.go index c992418ffb..20728e0077 100644 --- a/internal/dao/container.go +++ b/internal/dao/container.go @@ -72,12 +72,17 @@ func (c *Container) List(ctx context.Context, _ string) ([]runtime.Object, error res := make([]runtime.Object, 0, len(po.Spec.InitContainers)+len(po.Spec.EphemeralContainers)+len(po.Spec.Containers)) for _, group := range containerGroups { - for i := range group.containers { + for i, co := range group.containers { + var status *v1.ContainerStatus + if len(group.statuses) > i { + status = &group.statuses[i] + } + res = append(res, makeContainerRes( fmt.Sprintf("%s%d", group.indexPrefix, i+1), - group.containers[i], - group.statuses[i], - cmx[group.containers[i].Name], + &co, + status, + cmx[co.Name], po.GetCreationTimestamp(), )) } @@ -96,11 +101,11 @@ func (c *Container) TailLogs(ctx context.Context, opts *LogOptions) ([]LogChan, // ---------------------------------------------------------------------------- // Helpers... -func makeContainerRes(index string, co v1.Container, status v1.ContainerStatus, cmx *mv1beta1.ContainerMetrics, age metav1.Time) render.ContainerRes { +func makeContainerRes(index string, co *v1.Container, status *v1.ContainerStatus, cmx *mv1beta1.ContainerMetrics, age metav1.Time) render.ContainerRes { return render.ContainerRes{ Index: index, - Container: &co, - Status: &status, + Container: co, + Status: status, MX: cmx, Age: age, } From 2a8e7c4bc7cdef9364a1aa77591d5c6b7ebdd408 Mon Sep 17 00:00:00 2001 From: Ryan Brainard <966764+ryanbrainard@users.noreply.github.com> Date: Tue, 17 Sep 2024 10:38:51 -0400 Subject: [PATCH 14/14] Revert change to make container id the index, as it breaks container logs --- internal/render/container.go | 2 +- internal/render/container_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/render/container.go b/internal/render/container.go index c33cb26d21..cddb95e80b 100644 --- a/internal/render/container.go +++ b/internal/render/container.go @@ -107,7 +107,7 @@ func (c Container) Render(o interface{}, name string, r *model1.Row) error { ready, state, restarts = boolToStr(co.Status.Ready), ToContainerState(co.Status.State), strconv.Itoa(int(co.Status.RestartCount)) } - r.ID = co.Index + r.ID = co.Container.Name r.Fields = model1.Fields{ co.Index, co.Container.Name, diff --git a/internal/render/container_test.go b/internal/render/container_test.go index fc94a904ee..374873d7fd 100644 --- a/internal/render/container_test.go +++ b/internal/render/container_test.go @@ -42,7 +42,7 @@ func TestContainer(t *testing.T) { } var r model1.Row assert.Nil(t, c.Render(cres, "blee", &r)) - assert.Equal(t, "M1", r.ID) + assert.Equal(t, "fred", r.ID) assert.Equal(t, model1.Fields{ "M1", "fred",