fix(pvc): fix compatible issue if SC is changed (#6512)

Author: liubog2008

pkg/client/client.go

@@ -50,8 +50,8 @@ const (
 
 type Client interface {
 	client.WithWatch
-	Apply(ctx context.Context, obj client.Object) error
-	ApplyWithResult(ctx context.Context, obj client.Object) (ApplyResult, error)
+	Apply(ctx context.Context, obj client.Object, opts ...ApplyOption) error
+	ApplyWithResult(ctx context.Context, obj client.Object, opts ...ApplyOption) (ApplyResult, error)
 }
 
 type ApplyResult int
@@ -80,9 +80,9 @@ type applier struct {
 	parser GVKParser
 }
 
-func (p *applier) Apply(ctx context.Context, obj client.Object) error {
+func (p *applier) Apply(ctx context.Context, obj client.Object, opts ...ApplyOption) error {
 	logger := logr.FromContextOrDiscard(ctx)
-	res, err := p.ApplyWithResult(ctx, obj)
+	res, err := p.ApplyWithResult(ctx, obj, opts...)
 	if err == nil {
 		logger.Info("apply success",
 			"kind", reflect.TypeOf(obj),
@@ -94,7 +94,12 @@ func (p *applier) Apply(ctx context.Context, obj client.Object) error {
 	return err
 }
 
-func (p *applier) ApplyWithResult(ctx context.Context, obj client.Object) (ApplyResult, error) {
+func (p *applier) ApplyWithResult(ctx context.Context, obj client.Object, opts ...ApplyOption) (ApplyResult, error) {
+	o := &ApplyOptions{}
+	for _, opt := range opts {
+		opt.With(o)
+	}
+
 	gvks, _, err := scheme.Scheme.ObjectKinds(obj)
 	if err != nil {
 		return ApplyResultUnchanged, fmt.Errorf("cannot get gvks of the obj %T: %w", obj, err)
@@ -122,6 +127,19 @@ func (p *applier) ApplyWithResult(ctx context.Context, obj client.Object) (Apply
 		if err != nil {
 			return ApplyResultUnchanged, err
 		}
+		for _, fields := range o.Immutable {
+			val, exists, err := unstructured.NestedFieldNoCopy(current.Object, fields...)
+			if err != nil {
+				return ApplyResultUnchanged, fmt.Errorf("cannot get immutable fields %v: %w", fields, err)
+			}
+			if !exists {
+				unstructured.RemoveNestedField(expected.Object, fields...)
+			} else {
+				if err := unstructured.SetNestedField(expected.Object, val, fields...); err != nil {
+					return ApplyResultUnchanged, fmt.Errorf("cannot set immutable fields %v: %w", fields, err)
+				}
+			}
+		}
 		lastApplied, err := p.Extract(current, DefaultFieldManager, gvks[0], "")
 		if err != nil {
 			return ApplyResultUnchanged, fmt.Errorf("cannot extract last applied patch: %w", err)

pkg/client/client_test.go

@@ -28,12 +28,13 @@ import (
 
 func TestApply(t *testing.T) {
 	cases := []struct {
-		desc     string
-		objs     []client.Object
-		obj      client.Object
-		expected client.Object
-		res      ApplyResult
-		hasErr   bool
+		desc      string
+		objs      []client.Object
+		obj       client.Object
+		expected  client.Object
+		res       ApplyResult
+		immutable []string
+		hasErr    bool
 	}{
 		{
 			desc: "apply a new obj",
@@ -62,6 +63,26 @@ func TestApply(t *testing.T) {
 			expected: fake.FakeObj("aa", fake.GVK[corev1.Pod](corev1.SchemeGroupVersion), fake.Label[corev1.Pod]("test", "test")),
 			res:      ApplyResultUnchanged,
 		},
+		{
+			desc: "apply for an existing obj with immutable fields",
+			objs: []client.Object{
+				fake.FakeObj("aa", fake.Label[corev1.Pod]("test", "test"), func(obj *corev1.Pod) *corev1.Pod {
+					obj.Spec.NodeName = "xxx"
+					return obj
+				}),
+			},
+			obj: fake.FakeObj("aa", fake.Label[corev1.Pod]("test", "test"), func(obj *corev1.Pod) *corev1.Pod {
+				// nodeName is immutable
+				obj.Spec.NodeName = "yyy"
+				return obj
+			}),
+			expected: fake.FakeObj("aa", fake.GVK[corev1.Pod](corev1.SchemeGroupVersion), fake.Label[corev1.Pod]("test", "test"), func(obj *corev1.Pod) *corev1.Pod {
+				obj.Spec.NodeName = "xxx"
+				return obj
+			}),
+			immutable: []string{"spec", "nodeName"},
+			res:       ApplyResultUnchanged,
+		},
 	}
 
 	for i := range cases {
@@ -71,10 +92,10 @@ func TestApply(t *testing.T) {
 
 			p := NewFakeClient()
 			for _, obj := range c.objs {
-				err := p.Apply(context.TODO(), obj)
+				err := p.Apply(context.TODO(), obj, Immutable(c.immutable...))
 				require.NoError(tt, err)
 			}
-			res, err := p.ApplyWithResult(context.TODO(), c.obj)
+			res, err := p.ApplyWithResult(context.TODO(), c.obj, Immutable(c.immutable...))
 			if c.hasErr {
 				assert.Error(tt, err)
 			} else {

pkg/client/opt.go

@@ -0,0 +1,46 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package client
+
+type ApplyOptions struct {
+	// Immutable defines fields which is immutable
+	// It's only for some fields which cannot be changed but actually maybe changed.
+	// For example,
+	// Storage class ref can be changed in instance CR to modify volumes
+	// if feature VolumeAttributesClass is not enabled(just like v1).
+	// However, it's immutable in PVC. When VolumeAttributesClass is enabled,
+	// storage class should not be applied to PVC.
+	// NOTE; now slice/array is not supported
+	Immutable [][]string
+}
+
+type ApplyOption interface {
+	With(opts *ApplyOptions)
+}
+
+type ApplyOptionFunc func(opts *ApplyOptions)
+
+func (f ApplyOptionFunc) With(opts *ApplyOptions) {
+	f(opts)
+}
+
+func Immutable(fields ...string) ApplyOption {
+	return ApplyOptionFunc(func(opts *ApplyOptions) {
+		if len(fields) == 0 {
+			return
+		}
+		opts.Immutable = append(opts.Immutable, fields)
+	})
+}

pkg/volumes/sync.go

@@ -105,7 +105,15 @@ func SyncPVCs(ctx context.Context, c client.Client, pvcs []*corev1.PersistentVol
 }
 
 func syncPVC(ctx context.Context, c client.Client, pvc *corev1.PersistentVolumeClaim) error {
-	if err := c.Apply(ctx, pvc); err != nil {
+	if err := c.Apply(
+		ctx,
+		pvc,
+		// If VAC is not enabled, we use params in SC to modify volume.
+		// So we allow SC ref being changed.
+		// If VAC is enabled, we should also not apply changed SC to PVC.
+		// Reset to SC of current PVC.
+		client.Immutable("spec", "storageClassName"),
+	); err != nil {
 		return err
 	}
 	msgs := isPVCSynced(pvc)