From dedc769a39f6c05e91fafe0a67cd189a948b2dcf Mon Sep 17 00:00:00 2001 From: Shawna Monero Date: Thu, 22 Jun 2023 12:54:13 -0700 Subject: [PATCH 01/10] fix field selector generation --- backend/service/k8s/event.go | 34 ++++++++++++++++--------------- backend/service/k8s/event_test.go | 18 ++++++++-------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/backend/service/k8s/event.go b/backend/service/k8s/event.go index d1088eb1b2..0b81343569 100644 --- a/backend/service/k8s/event.go +++ b/backend/service/k8s/event.go @@ -58,27 +58,24 @@ func ProtoForEvent(cluster string, k8sEvent *corev1.Event) *k8sapiv1.Event { // Note in the case of a blank string being returned, that means there is no field selector, // and all events will be returned. -func convertTypesToFieldSelector(types []k8sapiv1.EventType) string { - fs := "" +// Note that chaining field selectors only results in AND not OR, thus to have multiples +// we must have multiple field selectors. +// See https://github.com/kubernetes/kubernetes/issues/32946 for more info +func convertTypesToFieldSelectors(types []k8sapiv1.EventType) []string { + fs := []string{} setOfTypes := make(map[k8sapiv1.EventType]bool) for _, t := range types { setOfTypes[t] = true } if setOfTypes[k8sapiv1.EventType_NORMAL] { - fs += "type=Normal" + fs = append(fs, "type=Normal") } if setOfTypes[k8sapiv1.EventType_WARNING] { - if fs != "" { - fs += "," - } - fs += "type=Warning" + fs = append(fs, "type=Warning") } if setOfTypes[k8sapiv1.EventType_ERROR] { - if fs != "" { - fs += "," - } - fs += "type=Error" + fs = append(fs, "type=Error") } return fs @@ -91,15 +88,20 @@ func (s *svc) ListNamespaceEvents(ctx context.Context, clientset, cluster, names } // Note if field selector is blank it will return everything - eventList, err := cs.CoreV1().Events(cs.Namespace()).List(ctx, metav1.ListOptions{FieldSelector: convertTypesToFieldSelector(types)}) - if err != nil { - return nil, err + totalEventList := []corev1.Event{} + fs := convertTypesToFieldSelectors(types) + for _, f := range fs { + eventList, err := cs.CoreV1().Events(cs.Namespace()).List(ctx, metav1.ListOptions{FieldSelector: f}) + if err != nil { + return nil, err + } + totalEventList = append(totalEventList, eventList.Items...) } // in the future, could also potentially return full event object rather than the subset in ProtoForEvent var events []*k8sapiv1.Event - for i := range eventList.Items { - events = append(events, ProtoForEvent(cs.Cluster(), &eventList.Items[i])) + for _, ev := range totalEventList { + events = append(events, ProtoForEvent(cs.Cluster(), &ev)) } return events, nil diff --git a/backend/service/k8s/event_test.go b/backend/service/k8s/event_test.go index 3efdf2968e..09168c8927 100644 --- a/backend/service/k8s/event_test.go +++ b/backend/service/k8s/event_test.go @@ -82,43 +82,43 @@ func TestListEvents(t *testing.T) { } } -func TestConvertTypesToFieldSelector(t *testing.T) { +func TestConvertTypesToFieldSelectors(t *testing.T) { testCases := []struct { name string types []k8sapiv1.EventType - expected string + expected []string }{ { name: "empty", types: []k8sapiv1.EventType{}, - expected: "", + expected: []string{}, }, { name: "single", types: []k8sapiv1.EventType{k8sapiv1.EventType_WARNING}, - expected: "type=Warning", + expected: []string{"type=Warning"}, }, { name: "multiple", types: []k8sapiv1.EventType{k8sapiv1.EventType_WARNING, k8sapiv1.EventType_NORMAL}, - expected: "type=Normal,type=Warning", + expected: []string{"type=Normal", "type=Warning"}, }, { name: "duplicate", types: []k8sapiv1.EventType{k8sapiv1.EventType_WARNING, k8sapiv1.EventType_WARNING}, - expected: "type=Warning", + expected: []string{"type=Warning"}, }, { name: "all", types: []k8sapiv1.EventType{k8sapiv1.EventType_WARNING, k8sapiv1.EventType_NORMAL, k8sapiv1.EventType_ERROR}, - expected: "type=Normal,type=Warning,type=Error", + expected: []string{"type=Error", "type=Normal", "type=Warning"}, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - actual := convertTypesToFieldSelector(tc.types) - assert.Equal(t, tc.expected, actual) + actual := convertTypesToFieldSelectors(tc.types) + assert.ElementsMatch(t, tc.expected, actual) }) } } From fb630f0760bedcefd1588f518869e89974990325 Mon Sep 17 00:00:00 2001 From: Shawna Monero Date: Thu, 22 Jun 2023 13:05:06 -0700 Subject: [PATCH 02/10] fix field selector generation --- backend/service/k8s/event.go | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/service/k8s/event.go b/backend/service/k8s/event.go index 0b81343569..a5e35f5d41 100644 --- a/backend/service/k8s/event.go +++ b/backend/service/k8s/event.go @@ -101,6 +101,7 @@ func (s *svc) ListNamespaceEvents(ctx context.Context, clientset, cluster, names // in the future, could also potentially return full event object rather than the subset in ProtoForEvent var events []*k8sapiv1.Event for _, ev := range totalEventList { + ev := ev events = append(events, ProtoForEvent(cs.Cluster(), &ev)) } From 480fb286cc0263f7f2ed604ea057153bb024cee9 Mon Sep 17 00:00:00 2001 From: Shawna Monero Date: Fri, 23 Jun 2023 17:28:30 -0700 Subject: [PATCH 03/10] comments --- backend/service/k8s/event.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/backend/service/k8s/event.go b/backend/service/k8s/event.go index a5e35f5d41..824943259f 100644 --- a/backend/service/k8s/event.go +++ b/backend/service/k8s/event.go @@ -87,9 +87,16 @@ func (s *svc) ListNamespaceEvents(ctx context.Context, clientset, cluster, names return nil, err } - // Note if field selector is blank it will return everything totalEventList := []corev1.Event{} fs := convertTypesToFieldSelectors(types) + // Note if field selector is blank it will return everything + if len(fs) == 0 { + eventList, err := cs.CoreV1().Events(cs.Namespace()).List(ctx, metav1.ListOptions{}) + if err != nil { + return nil, err + } + totalEventList = append(totalEventList, eventList.Items...) + } for _, f := range fs { eventList, err := cs.CoreV1().Events(cs.Namespace()).List(ctx, metav1.ListOptions{FieldSelector: f}) if err != nil { From 4338d70fea98daec7aab2831ae3ab674ff6fae20 Mon Sep 17 00:00:00 2001 From: Shawna Monero Date: Fri, 23 Jun 2023 20:05:40 -0700 Subject: [PATCH 04/10] sort --- backend/service/k8s/event.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/backend/service/k8s/event.go b/backend/service/k8s/event.go index 824943259f..7e824ef967 100644 --- a/backend/service/k8s/event.go +++ b/backend/service/k8s/event.go @@ -2,6 +2,7 @@ package k8s import ( "context" + "sort" "strings" "github.com/iancoleman/strcase" @@ -34,6 +35,25 @@ func (s *svc) ListEvents(ctx context.Context, clientset, cluster, namespace, obj return events, nil } +func sortEventsByTime(events []*k8sapiv1.Event) { + sort.Slice(events, func(i, j int) bool { + return events[i].CreationTimeMillis > events[j].CreationTimeMillis + }) +} + +func convertTypeStringToEnum(str string) k8sapiv1.EventType { + switch str { + case "Normal": + return k8sapiv1.EventType_NORMAL + case "Warning": + return k8sapiv1.EventType_WARNING + case "Error": + return k8sapiv1.EventType_ERROR + default: + return k8sapiv1.EventType_TYPE_UNSPECIFIED + } +} + func ProtoForEvent(cluster string, k8sEvent *corev1.Event) *k8sapiv1.Event { clusterName := k8sEvent.ClusterName if clusterName == "" { @@ -53,6 +73,7 @@ func ProtoForEvent(cluster string, k8sEvent *corev1.Event) *k8sapiv1.Event { InvolvedObjectName: k8sEvent.InvolvedObject.Name, Kind: protoForObjectKind(k8sEvent.InvolvedObject.Kind), CreationTimeMillis: k8sEvent.GetObjectMeta().GetCreationTimestamp().UnixMilli(), + Type: convertTypeStringToEnum(k8sEvent.Type), } } @@ -111,6 +132,8 @@ func (s *svc) ListNamespaceEvents(ctx context.Context, clientset, cluster, names ev := ev events = append(events, ProtoForEvent(cs.Cluster(), &ev)) } + // Sort by CreationTimeMillis + sortEventsByTime(events) return events, nil } From 45c90407c9febeaa642a98a57e544b1c077f06c3 Mon Sep 17 00:00:00 2001 From: Shawna Monero Date: Fri, 23 Jun 2023 20:06:46 -0700 Subject: [PATCH 05/10] sort --- backend/service/k8s/event.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/service/k8s/event.go b/backend/service/k8s/event.go index 7e824ef967..6f0b8efc03 100644 --- a/backend/service/k8s/event.go +++ b/backend/service/k8s/event.go @@ -35,6 +35,7 @@ func (s *svc) ListEvents(ctx context.Context, clientset, cluster, namespace, obj return events, nil } +// For ease of use, we can sort the events in reverse chronological order func sortEventsByTime(events []*k8sapiv1.Event) { sort.Slice(events, func(i, j int) bool { return events[i].CreationTimeMillis > events[j].CreationTimeMillis @@ -132,7 +133,7 @@ func (s *svc) ListNamespaceEvents(ctx context.Context, clientset, cluster, names ev := ev events = append(events, ProtoForEvent(cs.Cluster(), &ev)) } - // Sort by CreationTimeMillis + // Sort by CreationTimeMillis, this is needed because we might have multiple types of events sortEventsByTime(events) return events, nil From d410cf9b80852e0d243a59ef66d73816d181a0eb Mon Sep 17 00:00:00 2001 From: Shawna Monero Date: Mon, 26 Jun 2023 13:38:37 -0700 Subject: [PATCH 06/10] comment --- backend/service/k8s/event.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/backend/service/k8s/event.go b/backend/service/k8s/event.go index 87eead8749..2ea1b13286 100644 --- a/backend/service/k8s/event.go +++ b/backend/service/k8s/event.go @@ -56,7 +56,7 @@ func convertTypeStringToEnum(str string) k8sapiv1.EventType { } func ProtoForEvent(cluster string, k8sEvent *corev1.Event) *k8sapiv1.Event { - clusterName := GetKubeClusterName(k8sEvent) + clusterName := k8sEvent.ClusterName if clusterName == "" { clusterName = cluster } @@ -133,7 +133,8 @@ func (s *svc) ListNamespaceEvents(ctx context.Context, clientset, cluster, names ev := ev events = append(events, ProtoForEvent(cs.Cluster(), &ev)) } - // Sort by CreationTimeMillis, this is needed because we might have multiple types of events + // Sort in reverse chronological order (by CreationTimeMillis), + // this is needed because we might have multiple types of events sortEventsByTime(events) return events, nil From fbb7403fe81529acfb0c402785b5b144b6abe44e Mon Sep 17 00:00:00 2001 From: Shawna Monero Date: Mon, 26 Jun 2023 13:47:23 -0700 Subject: [PATCH 07/10] comment --- backend/service/k8s/event.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/service/k8s/event.go b/backend/service/k8s/event.go index 2ea1b13286..f04e8e278f 100644 --- a/backend/service/k8s/event.go +++ b/backend/service/k8s/event.go @@ -127,7 +127,8 @@ func (s *svc) ListNamespaceEvents(ctx context.Context, clientset, cluster, names totalEventList = append(totalEventList, eventList.Items...) } - // in the future, could also potentially return full event object rather than the subset in ProtoForEvent + // in the future, could also potentially return full event object rather than the subset in + // ProtoForEvent() var events []*k8sapiv1.Event for _, ev := range totalEventList { ev := ev From 13ab7ea0270977540baba389984221bb7c8d010f Mon Sep 17 00:00:00 2001 From: Shawna Monero Date: Mon, 26 Jun 2023 13:50:54 -0700 Subject: [PATCH 08/10] oops --- backend/service/k8s/event.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/backend/service/k8s/event.go b/backend/service/k8s/event.go index f04e8e278f..ffc25f04cf 100644 --- a/backend/service/k8s/event.go +++ b/backend/service/k8s/event.go @@ -56,17 +56,13 @@ func convertTypeStringToEnum(str string) k8sapiv1.EventType { } func ProtoForEvent(cluster string, k8sEvent *corev1.Event) *k8sapiv1.Event { - clusterName := k8sEvent.ClusterName - if clusterName == "" { - clusterName = cluster - } // Note for timestamps - in k8s 1.25 LastTimestamp is deprecated in favor of // EventTime. However, some objects currently use EventTime, while others use LastTimestamp. Using the // CreationTime from the Metadata is an option as it refers to the creation of the object by the server. // See https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#ObjectMeta // See also https://github.com/kubernetes/kubernetes/issues/90482 for a discussion on different timestamps. return &k8sapiv1.Event{ - Cluster: clusterName, + Cluster: cluster, Namespace: k8sEvent.Namespace, Name: k8sEvent.Name, Reason: k8sEvent.Reason, From 9cff7b3155e4811a13dbaf76daacbca05721fc77 Mon Sep 17 00:00:00 2001 From: Shawna Monero Date: Mon, 26 Jun 2023 14:32:59 -0700 Subject: [PATCH 09/10] oops --- backend/service/k8s/event.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/backend/service/k8s/event.go b/backend/service/k8s/event.go index ffc25f04cf..32bc9a84cb 100644 --- a/backend/service/k8s/event.go +++ b/backend/service/k8s/event.go @@ -56,6 +56,10 @@ func convertTypeStringToEnum(str string) k8sapiv1.EventType { } func ProtoForEvent(cluster string, k8sEvent *corev1.Event) *k8sapiv1.Event { + clusterName := GetKubeClusterName(k8sEvent) + if clusterName == "" { + clusterName = cluster + } // Note for timestamps - in k8s 1.25 LastTimestamp is deprecated in favor of // EventTime. However, some objects currently use EventTime, while others use LastTimestamp. Using the // CreationTime from the Metadata is an option as it refers to the creation of the object by the server. From c0f15b28c1bb28d3692e93e8e615571566b8fae2 Mon Sep 17 00:00:00 2001 From: Shawna Monero Date: Mon, 26 Jun 2023 14:33:11 -0700 Subject: [PATCH 10/10] oops --- backend/service/k8s/event.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/service/k8s/event.go b/backend/service/k8s/event.go index 32bc9a84cb..d1db50fd04 100644 --- a/backend/service/k8s/event.go +++ b/backend/service/k8s/event.go @@ -66,7 +66,7 @@ func ProtoForEvent(cluster string, k8sEvent *corev1.Event) *k8sapiv1.Event { // See https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#ObjectMeta // See also https://github.com/kubernetes/kubernetes/issues/90482 for a discussion on different timestamps. return &k8sapiv1.Event{ - Cluster: cluster, + Cluster: clusterName, Namespace: k8sEvent.Namespace, Name: k8sEvent.Name, Reason: k8sEvent.Reason,