fix: trim the history annotation when the size is too big (#418)

* fix

* add comment and format

* add testcase

Author: YamasouA

simulator/scheduler/storereflector/storereflector.go

@@ -9,6 +9,7 @@ import (
 	"golang.org/x/xerrors"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	validation "k8s.io/apimachinery/pkg/api/validation"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
@@ -157,11 +158,19 @@ func updateResultHistory(p *corev1.Pod, m map[string]string) error {
 
 	results = append(results, m)
 
-	r, err := json.Marshal(results)
-	if err != nil {
-		return xerrors.Errorf("encode all results: %w", err)
+	// If we exceed that annotation size limitation, we should drop entries from the oldest side
+	// of the history slice until the payload fits, since the newer histories are likely more important.
+	for ; len(results) != 0; results = results[1:] {
+		r, err := json.Marshal(results)
+		if err != nil {
+			return xerrors.Errorf("encode all results: %w", err)
+		}
+		if len(r) <= validation.TotalAnnotationSizeLimitB {
+			metav1.SetMetaDataAnnotation(&p.ObjectMeta, ResultsHistoryAnnotation, string(r))
+			return nil
+		}
 	}
-	metav1.SetMetaDataAnnotation(&p.ObjectMeta, ResultsHistoryAnnotation, string(r))
 
-	return nil
+	// Basically shouldn't happen unless a single history entry is bigger than TotalAnnotationSizeLimitB, which is very unlikely.
+	return xerrors.Errorf("result history still exceeds annotation limit even after removing several histories")
 }

simulator/scheduler/storereflector/storereflector_test.go

@@ -3,6 +3,7 @@ package storereflector
 import (
 	"context"
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -129,6 +130,27 @@ func Test_updateResultHistory(t *testing.T) {
 			},
 			wantErr: assert.NoError,
 		},
+		{
+			name: "success: trim oldest result history when exceeds annotation size limitation",
+			p: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						ResultsHistoryAnnotation: fmt.Sprintf(`[{"result":"%s"}]`, strings.Repeat("a", 200000)),
+					},
+				},
+			},
+			m: map[string]string{
+				"result": strings.Repeat("b", 200000),
+			},
+			wantPod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						ResultsHistoryAnnotation: fmt.Sprintf(`[{"result":"%s"}]`, strings.Repeat("b", 200000)),
+					},
+				},
+			},
+			wantErr: assert.NoError,
+		},
 		{
 			name: "fail: Pod has broken value on annotation",
 			p: &corev1.Pod{
@@ -144,6 +166,27 @@ func Test_updateResultHistory(t *testing.T) {
 			},
 			wantErr: assert.Error,
 		},
+		{
+			name: "fail: single result history exceeds annotation size limitation",
+			p: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						ResultsHistoryAnnotation: "[]",
+					},
+				},
+			},
+			m: map[string]string{
+				"result": strings.Repeat("a", 270000),
+			},
+			wantPod: &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						ResultsHistoryAnnotation: "[]",
+					},
+				},
+			},
+			wantErr: assert.Error,
+		},
 	}
 	for _, tt := range tests {
 		tt := tt