K8s: Increment generation in generic strategy (#97946)

---------

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>

pkg/apiserver/registry/generic/strategy.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/grafana/grafana/pkg/apimachinery/utils"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
@@ -41,15 +42,29 @@ func (g *genericStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.S
 	return fields
 }
 
-func (g *genericStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {}
-
-func (g *genericStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
-	oldMeta, err := utils.MetaAccessor(old)
+func (g *genericStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
+	meta, err := utils.MetaAccessor(obj)
 	if err != nil {
 		return
 	}
+	meta.SetGeneration(1)
+	objCopy := obj.DeepCopyObject()
+	err = runtime.SetZeroValue(objCopy)
+	if err != nil {
+		return
+	}
+	metaCopy, err := utils.MetaAccessor(objCopy)
+	if err != nil {
+		return
+	}
+	status, err := metaCopy.GetStatus()
+	if err == nil {
+		_ = meta.SetStatus(status)
+	}
+}
 
-	status, err := oldMeta.GetStatus()
+func (g *genericStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
+	oldMeta, err := utils.MetaAccessor(old)
 	if err != nil {
 		return
 	}
@@ -59,7 +74,27 @@ func (g *genericStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime
 		return
 	}
 
-	_ = newMeta.SetStatus(status)
+	// update shouldn't change the status
+	status, err := oldMeta.GetStatus()
+	if err != nil {
+		_ = newMeta.SetStatus(nil)
+	} else {
+		_ = newMeta.SetStatus(status)
+	}
+
+	spec, err := newMeta.GetSpec()
+	if err != nil {
+		return
+	}
+
+	oldSpec, err := oldMeta.GetSpec()
+	if err != nil {
+		return
+	}
+
+	if !apiequality.Semantic.DeepEqual(spec, oldSpec) {
+		newMeta.SetGeneration(oldMeta.GetGeneration() + 1)
+	}
 }
 
 func (g *genericStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {

pkg/apiserver/registry/generic/strategy_test.go

@@ -0,0 +1,134 @@
+package generic_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/grafana/grafana/pkg/apiserver/registry/generic"
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apiserver/pkg/apis/example"
+)
+
+func TestPrepareForUpdate(t *testing.T) {
+	ctx := context.TODO()
+	gv := schema.GroupVersion{Group: "test", Version: "v1"}
+	strategy := generic.NewStrategy(runtime.NewScheme(), gv)
+
+	oldObj := &example.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "test",
+			Namespace:  "default",
+			Generation: 1,
+		},
+		Spec: example.PodSpec{
+			NodeSelector: map[string]string{"foo": "bar"},
+		},
+		Status: example.PodStatus{
+			Phase: example.PodPhase("Running"),
+		},
+	}
+
+	testCases := []struct {
+		name        string
+		newObj      *example.Pod
+		oldObj      *example.Pod
+		expectedGen int64
+		expectedObj *example.Pod
+	}{
+		{
+			name: "ignore status updates",
+			newObj: &example.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "test",
+					Namespace:  "default",
+					Generation: 1,
+				},
+				Spec: example.PodSpec{
+					NodeSelector: map[string]string{"foo": "bar"},
+				},
+				Status: example.PodStatus{
+					Phase: example.PodPhase("Stopped"),
+				},
+			},
+			oldObj:      oldObj.DeepCopy(),
+			expectedGen: 2,
+			expectedObj: &example.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "test",
+					Namespace:  "default",
+					Generation: 1,
+				},
+				Spec: example.PodSpec{
+					NodeSelector: map[string]string{"foo": "bar"},
+				},
+				Status: example.PodStatus{
+					Phase: example.PodPhase("Running"),
+				},
+			},
+		},
+		{
+			name: "increment generation if spec changes",
+			newObj: &example.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "test",
+					Namespace:  "default",
+					Generation: 1,
+				},
+				Spec: example.PodSpec{
+					NodeSelector: map[string]string{"foo": "baz"},
+				},
+				Status: example.PodStatus{
+					Phase: example.PodPhase("Running"),
+				},
+			},
+			oldObj:      oldObj.DeepCopy(),
+			expectedGen: 2,
+			expectedObj: &example.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "test",
+					Namespace:  "default",
+					Generation: 2,
+				},
+				Spec: example.PodSpec{
+					NodeSelector: map[string]string{"foo": "baz"},
+				},
+				Status: example.PodStatus{
+					Phase: example.PodPhase("Running"),
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			strategy.PrepareForUpdate(ctx, tc.newObj, tc.oldObj)
+			require.Equal(t, tc.expectedObj, tc.newObj)
+		})
+	}
+}
+
+func TestPrepareForCreate(t *testing.T) {
+	ctx := context.TODO()
+	gv := schema.GroupVersion{Group: "test", Version: "v1"}
+	strategy := generic.NewStrategy(runtime.NewScheme(), gv)
+
+	obj := &example.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "default",
+		},
+		Spec: example.PodSpec{
+			NodeSelector: map[string]string{"foo": "bar"},
+		},
+		Status: example.PodStatus{
+			Phase: example.PodPhase("Running"),
+		},
+	}
+
+	strategy.PrepareForCreate(ctx, obj)
+	require.Equal(t, int64(1), obj.Generation)
+	require.Equal(t, example.PodStatus{}, obj.Status)
+}

pkg/tests/apis/helper.go

@@ -216,6 +216,9 @@ func (c *K8sResourceClient) sanitizeObject(v *unstructured.Unstructured, replace
 	meta, ok := copy["metadata"].(map[string]any)
 	require.True(c.t, ok)
 
+	// remove generation
+	delete(meta, "generation")
+
 	replaceMeta = append(replaceMeta, "creationTimestamp", "resourceVersion", "uid")
 	for _, key := range replaceMeta {
 		old, ok := meta[key]
@@ -403,12 +406,18 @@ func DoRequest[T any](c *K8sTestHelper, params RequestParams, result *T) K8sResp
 // Read local JSON or YAML file into a resource
 func (c *K8sTestHelper) LoadYAMLOrJSONFile(fpath string) *unstructured.Unstructured {
 	c.t.Helper()
+	return c.LoadYAMLOrJSON(string(c.LoadFile(fpath)))
+}
+
+// Read local file into a byte slice. Does not need to be a resource.
+func (c *K8sTestHelper) LoadFile(fpath string) []byte {
+	c.t.Helper()
 
 	//nolint:gosec
 	raw, err := os.ReadFile(fpath)
 	require.NoError(c.t, err)
 	require.NotEmpty(c.t, raw)
-	return c.LoadYAMLOrJSON(string(raw))
+	return raw
 }
 
 // Read local JSON or YAML file into a resource