From 862d256195adf3be5475b1a6935e5feb78f884a5 Mon Sep 17 00:00:00 2001 From: Andrea Nodari Date: Sat, 27 Feb 2021 17:16:46 +0100 Subject: [PATCH] Add "subresource" field to ManagedFieldEntry This field is useful to namespace the managed field entries of a subresource and differentiate them from the ones of the main resource. --- .../pkg/apiserver/customresource_handler.go | 8 +- .../apimachinery/pkg/apis/meta/v1/types.go | 9 + .../pkg/apis/meta/v1/validation/validation.go | 6 + .../meta/v1/validation/validation_test.go | 13 +- .../handlers/fieldmanager/buildmanagerinfo.go | 11 +- .../handlers/fieldmanager/capmanagers_test.go | 4 +- .../handlers/fieldmanager/fieldmanager.go | 25 +- .../fieldmanager/fieldmanager_test.go | 12 +- .../fieldmanager/internal/conflict.go | 10 +- .../fieldmanager/internal/conflict_test.go | 25 + .../fieldmanager/internal/managedfields.go | 6 +- .../internal/managedfields_test.go | 34 ++ .../fieldmanager/lastappliedupdater_test.go | 2 +- .../fieldmanager/managedfieldsupdater_test.go | 2 +- .../fieldmanager/skipnonapplied_test.go | 4 +- .../apiserver/pkg/endpoints/installer.go | 2 +- .../integration/apiserver/apply/apply_test.go | 506 ++++++++++++++++++ 17 files changed, 639 insertions(+), 40 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index b264b5ab559..9cef0d0777d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -872,7 +872,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd typeConverter, reqScope, resetFields, - false, + "", ) if err != nil { return nil, err @@ -914,7 +914,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd typeConverter, statusScope, resetFields, - true, + "status", ) if err != nil { return nil, err @@ -958,7 +958,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd return ret, nil } -func scopeWithFieldManager(typeConverter fieldmanager.TypeConverter, reqScope handlers.RequestScope, resetFields map[fieldpath.APIVersion]*fieldpath.Set, ignoreManagedFieldsFromRequestObject bool) (handlers.RequestScope, error) { +func scopeWithFieldManager(typeConverter fieldmanager.TypeConverter, reqScope handlers.RequestScope, resetFields map[fieldpath.APIVersion]*fieldpath.Set, subresource string) (handlers.RequestScope, error) { fieldManager, err := fieldmanager.NewDefaultCRDFieldManager( typeConverter, reqScope.Convertor, @@ -966,7 +966,7 @@ func scopeWithFieldManager(typeConverter fieldmanager.TypeConverter, reqScope ha reqScope.Creater, reqScope.Kind, reqScope.HubGroupVersion, - ignoreManagedFieldsFromRequestObject, + subresource, resetFields, ) if err != nil { diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go index 8b0057d8cde..fb680a44bdf 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go @@ -1181,6 +1181,15 @@ type ManagedFieldsEntry struct { // FieldsV1 holds the first JSON version format as described in the "FieldsV1" type. // +optional FieldsV1 *FieldsV1 `json:"fieldsV1,omitempty" protobuf:"bytes,7,opt,name=fieldsV1"` + + // Subresource is the name of the subresource used to update that object, or + // empty string if the object was updated through the main resource. The + // value of this field is used to distinguish between managers, even if they + // share the same name. For example, a status update will be distinct from a + // regular update using the same manager name. + // Note that the APIVersion field is not related to the Subresource field and + // it always corresponds to the version of the main resource. + Subresource string `json:"subresource,omitempty" protobuf:"bytes,8,opt,name=subresource"` } // ManagedFieldsOperationType is the type of operation which lead to a ManagedFieldsEntry being created. diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go index a5a7f144add..6706103ee18 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go @@ -171,6 +171,8 @@ func ValidateTableOptions(opts *metav1.TableOptions) field.ErrorList { return allErrs } +const MaxSubresourceNameLength = 256 + func ValidateManagedFields(fieldsList []metav1.ManagedFieldsEntry, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList for i, fields := range fieldsList { @@ -184,6 +186,10 @@ func ValidateManagedFields(fieldsList []metav1.ManagedFieldsEntry, fldPath *fiel allErrs = append(allErrs, field.Invalid(fldPath.Child("fieldsType"), fields.FieldsType, "must be `FieldsV1`")) } allErrs = append(allErrs, ValidateFieldManager(fields.Manager, fldPath.Child("manager"))...) + + if len(fields.Subresource) > MaxSubresourceNameLength { + allErrs = append(allErrs, field.TooLong(fldPath.Child("subresource"), fields.Subresource, MaxSubresourceNameLength)) + } } return allErrs } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go index 4a6f8f30250..93c3284f7cf 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go @@ -266,6 +266,12 @@ func TestValidateManagedFieldsInvalid(t *testing.T) { Manager: "field\nmanager", APIVersion: "v1", }, + { + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + APIVersion: "v1", + Subresource: "TooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLongTooLong", + }, } for _, test := range tests { @@ -291,9 +297,10 @@ func TestValidateMangedFieldsValid(t *testing.T) { APIVersion: "v1", }, { - Operation: metav1.ManagedFieldsOperationApply, - FieldsType: "FieldsV1", - APIVersion: "v1", + Operation: metav1.ManagedFieldsOperationApply, + FieldsType: "FieldsV1", + APIVersion: "v1", + Subresource: "scale", }, { Operation: metav1.ManagedFieldsOperationApply, diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/buildmanagerinfo.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/buildmanagerinfo.go index fc471e79759..58b87eb3886 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/buildmanagerinfo.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/buildmanagerinfo.go @@ -28,16 +28,18 @@ import ( type buildManagerInfoManager struct { fieldManager Manager groupVersion schema.GroupVersion + subresource string } var _ Manager = &buildManagerInfoManager{} // NewBuildManagerInfoManager creates a new Manager that converts the manager name into a unique identifier // combining operation and version for update requests, and just operation for apply requests. -func NewBuildManagerInfoManager(f Manager, gv schema.GroupVersion) Manager { +func NewBuildManagerInfoManager(f Manager, gv schema.GroupVersion, subresource string) Manager { return &buildManagerInfoManager{ fieldManager: f, groupVersion: gv, + subresource: subresource, } } @@ -61,9 +63,10 @@ func (f *buildManagerInfoManager) Apply(liveObj, appliedObj runtime.Object, mana func (f *buildManagerInfoManager) buildManagerInfo(prefix string, operation metav1.ManagedFieldsOperationType) (string, error) { managerInfo := metav1.ManagedFieldsEntry{ - Manager: prefix, - Operation: operation, - APIVersion: f.groupVersion.String(), + Manager: prefix, + Operation: operation, + APIVersion: f.groupVersion.String(), + Subresource: f.subresource, } if managerInfo.Manager == "" { managerInfo.Manager = "unknown" diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/capmanagers_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/capmanagers_test.go index 3e79450551a..221dc9eae06 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/capmanagers_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/capmanagers_test.go @@ -47,7 +47,7 @@ func (*fakeManager) Apply(_, _ runtime.Object, _ Managed, _ string, _ bool) (run func TestCapManagersManagerMergesEntries(t *testing.T) { f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), - false, + "", func(m Manager) Manager { return NewCapManagersManager(m, 3) }) @@ -113,7 +113,7 @@ func TestCapManagersManagerMergesEntries(t *testing.T) { func TestCapUpdateManagers(t *testing.T) { f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), - false, + "", func(m Manager) Manager { return NewCapManagersManager(m, 3) }) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go index c9905515a03..a4023a0e767 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go @@ -66,39 +66,39 @@ type Manager interface { // FieldManager updates the managed fields and merge applied // configurations. type FieldManager struct { - fieldManager Manager - ignoreManagedFieldsFromRequestObject bool + fieldManager Manager + subresource string } // NewFieldManager creates a new FieldManager that decodes, manages, then re-encodes managedFields // on update and apply requests. -func NewFieldManager(f Manager, ignoreManagedFieldsFromRequestObject bool) *FieldManager { - return &FieldManager{fieldManager: f, ignoreManagedFieldsFromRequestObject: ignoreManagedFieldsFromRequestObject} +func NewFieldManager(f Manager, subresource string) *FieldManager { + return &FieldManager{fieldManager: f, subresource: subresource} } // NewDefaultFieldManager creates a new FieldManager that merges apply requests // and update managed fields for other types of requests. -func NewDefaultFieldManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, ignoreManagedFieldsFromRequestObject bool, resetFields map[fieldpath.APIVersion]*fieldpath.Set) (*FieldManager, error) { +func NewDefaultFieldManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, subresource string, resetFields map[fieldpath.APIVersion]*fieldpath.Set) (*FieldManager, error) { f, err := NewStructuredMergeManager(typeConverter, objectConverter, objectDefaulter, kind.GroupVersion(), hub, resetFields) if err != nil { return nil, fmt.Errorf("failed to create field manager: %v", err) } - return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind, ignoreManagedFieldsFromRequestObject), nil + return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind, subresource), nil } // NewDefaultCRDFieldManager creates a new FieldManager specifically for // CRDs. This allows for the possibility of fields which are not defined // in models, as well as having no models defined at all. -func NewDefaultCRDFieldManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, ignoreManagedFieldsFromRequestObject bool, resetFields map[fieldpath.APIVersion]*fieldpath.Set) (_ *FieldManager, err error) { +func NewDefaultCRDFieldManager(typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectDefaulter runtime.ObjectDefaulter, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, hub schema.GroupVersion, subresource string, resetFields map[fieldpath.APIVersion]*fieldpath.Set) (_ *FieldManager, err error) { f, err := NewCRDStructuredMergeManager(typeConverter, objectConverter, objectDefaulter, kind.GroupVersion(), hub, resetFields) if err != nil { return nil, fmt.Errorf("failed to create field manager: %v", err) } - return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind, ignoreManagedFieldsFromRequestObject), nil + return newDefaultFieldManager(f, typeConverter, objectConverter, objectCreater, kind, subresource), nil } // newDefaultFieldManager is a helper function which wraps a Manager with certain default logic. -func newDefaultFieldManager(f Manager, typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, ignoreManagedFieldsFromRequestObject bool) *FieldManager { +func newDefaultFieldManager(f Manager, typeConverter TypeConverter, objectConverter runtime.ObjectConvertor, objectCreater runtime.ObjectCreater, kind schema.GroupVersionKind, subresource string) *FieldManager { return NewFieldManager( NewLastAppliedUpdater( NewLastAppliedManager( @@ -107,11 +107,11 @@ func newDefaultFieldManager(f Manager, typeConverter TypeConverter, objectConver NewBuildManagerInfoManager( NewManagedFieldsUpdater( NewStripMetaManager(f), - ), kind.GroupVersion(), + ), kind.GroupVersion(), subresource, ), DefaultMaxUpdateManagers, ), objectCreater, kind, DefaultTrackOnCreateProbability, ), typeConverter, objectConverter, kind.GroupVersion()), - ), ignoreManagedFieldsFromRequestObject, + ), subresource, ) } @@ -167,7 +167,8 @@ func emptyManagedFieldsOnErr(managed Managed, err error) (Managed, error) { func (f *FieldManager) Update(liveObj, newObj runtime.Object, manager string) (object runtime.Object, err error) { // First try to decode the managed fields provided in the update, // This is necessary to allow directly updating managed fields. - managed, err := decodeLiveOrNew(liveObj, newObj, f.ignoreManagedFieldsFromRequestObject) + isSubresource := f.subresource != "" + managed, err := decodeLiveOrNew(liveObj, newObj, isSubresource) if err != nil { return newObj, nil } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go index 7978d6221ff..a635625c5b0 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager_test.go @@ -85,14 +85,14 @@ type TestFieldManager struct { } func NewDefaultTestFieldManager(gvk schema.GroupVersionKind) TestFieldManager { - return NewTestFieldManager(gvk, false, nil) + return NewTestFieldManager(gvk, "", nil) } func NewSubresourceTestFieldManager(gvk schema.GroupVersionKind) TestFieldManager { - return NewTestFieldManager(gvk, true, nil) + return NewTestFieldManager(gvk, "scale", nil) } -func NewTestFieldManager(gvk schema.GroupVersionKind, ignoreManagedFieldsFromRequestObject bool, chainFieldManager func(Manager) Manager) TestFieldManager { +func NewTestFieldManager(gvk schema.GroupVersionKind, subresource string, chainFieldManager func(Manager) Manager) TestFieldManager { m := NewFakeOpenAPIModels() typeConverter := NewFakeTypeConverter(m) converter := newVersionConverter(typeConverter, &fakeObjectConvertor{}, gvk.GroupVersion()) @@ -118,7 +118,7 @@ func NewTestFieldManager(gvk schema.GroupVersionKind, ignoreManagedFieldsFromReq NewBuildManagerInfoManager( NewManagedFieldsUpdater( NewStripMetaManager(f), - ), gvk.GroupVersion(), + ), gvk.GroupVersion(), subresource, ), &fakeObjectCreater{gvk: gvk}, gvk, DefaultTrackOnCreateProbability, ), typeConverter, objectConverter, gvk.GroupVersion(), ), @@ -127,7 +127,7 @@ func NewTestFieldManager(gvk schema.GroupVersionKind, ignoreManagedFieldsFromReq f = chainFieldManager(f) } return TestFieldManager{ - fieldManager: NewFieldManager(f, ignoreManagedFieldsFromRequestObject), + fieldManager: NewFieldManager(f, subresource), apiVersion: gvk.GroupVersion().String(), emptyObj: live, liveObj: live.DeepCopyObject(), @@ -1266,7 +1266,7 @@ func TestUpdateViaSubresources(t *testing.T) { }, }) - // Check that managed fields cannot be changed via subresources + // Check that managed fields cannot be changed explicitly via subresources expectedManager := "fieldmanager_test_subresource" if err := f.Update(obj, expectedManager); err != nil { t.Fatalf("failed to apply object: %v", err) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/conflict.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/conflict.go index cfa19d8d9aa..8c044c91570 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/conflict.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/conflict.go @@ -75,11 +75,15 @@ func printManager(manager string) string { if err := json.Unmarshal([]byte(manager), encodedManager); err != nil { return fmt.Sprintf("%q", manager) } + managerStr := fmt.Sprintf("%q", encodedManager.Manager) + if encodedManager.Subresource != "" { + managerStr = fmt.Sprintf("%s with subresource %q", managerStr, encodedManager.Subresource) + } if encodedManager.Operation == metav1.ManagedFieldsOperationUpdate { if encodedManager.Time == nil { - return fmt.Sprintf("%q using %v", encodedManager.Manager, encodedManager.APIVersion) + return fmt.Sprintf("%s using %v", managerStr, encodedManager.APIVersion) } - return fmt.Sprintf("%q using %v at %v", encodedManager.Manager, encodedManager.APIVersion, encodedManager.Time.UTC().Format(time.RFC3339)) + return fmt.Sprintf("%s using %v at %v", managerStr, encodedManager.APIVersion, encodedManager.Time.UTC().Format(time.RFC3339)) } - return fmt.Sprintf("%q", encodedManager.Manager) + return managerStr } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/conflict_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/conflict_test.go index a523ed841dc..f9fc757c58f 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/conflict_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/conflict_test.go @@ -96,6 +96,31 @@ conflicts with "foo" using v1 at 2001-02-03T04:05:06Z: }, }, }, + { + conflict: merge.Conflicts{ + merge.Conflict{ + Manager: `{"manager":"foo","operation":"Update","subresource":"scale","apiVersion":"v1","time":"2001-02-03T04:05:06Z"}`, + Path: fieldpath.MakePathOrDie("spec", "replicas"), + }, + }, + expected: &errors.StatusError{ + ErrStatus: metav1.Status{ + Status: metav1.StatusFailure, + Code: http.StatusConflict, + Reason: metav1.StatusReasonConflict, + Details: &metav1.StatusDetails{ + Causes: []metav1.StatusCause{ + { + Type: metav1.CauseTypeFieldManagerConflict, + Message: `conflict with "foo" with subresource "scale" using v1 at 2001-02-03T04:05:06Z`, + Field: ".spec.replicas", + }, + }, + }, + Message: `Apply failed with 1 conflict: conflict with "foo" with subresource "scale" using v1 at 2001-02-03T04:05:06Z: .spec.replicas`, + }, + }, + }, } for _, tc := range testCases { actual := internal.NewConflictError(tc.conflict) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go index 40237cdc660..9b4c2032628 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields.go @@ -213,7 +213,11 @@ func sortEncodedManagedFields(encodedManagedFields []metav1.ManagedFieldsEntry) if p.Manager != q.Manager { return p.Manager < q.Manager } - return p.APIVersion < q.APIVersion + + if p.APIVersion != q.APIVersion { + return p.APIVersion < q.APIVersion + } + return p.Subresource < q.Subresource }) return encodedManagedFields, nil diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go index f6df7228c13..922f1020e04 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/managedfields_test.go @@ -258,6 +258,15 @@ func TestRoundTripManagedFields(t *testing.T) { f:storage: {} manager: foo operation: Update +`, + `- apiVersion: v1 + fieldsType: FieldsV1 + fieldsV1: + f:spec: + f:replicas: {} + manager: foo + operation: Update + subresource: scale `, } @@ -313,6 +322,18 @@ time: "2001-02-03T04:05:06Z" `, expected: "{\"manager\":\"foo\",\"operation\":\"Apply\"}", }, + { + managedFieldsEntry: ` +apiVersion: v1 +fieldsV1: + f:apiVersion: {} +manager: foo +operation: Apply +subresource: scale +time: "2001-02-03T04:05:06Z" +`, + expected: "{\"manager\":\"foo\",\"operation\":\"Apply\",\"subresource\":\"scale\"}", + }, } for _, test := range tests { @@ -462,6 +483,19 @@ func TestSortEncodedManagedFields(t *testing.T) { {Manager: "c", Operation: metav1.ManagedFieldsOperationUpdate, Time: &metav1.Time{time.Date(2000, time.January, 0, 0, 0, 0, 1, time.UTC)}}, }, }, + { + name: "entries with subresource field", + managedFields: []metav1.ManagedFieldsEntry{ + {Manager: "a", Operation: metav1.ManagedFieldsOperationApply, Subresource: "status"}, + {Manager: "a", Operation: metav1.ManagedFieldsOperationApply, Subresource: "scale"}, + {Manager: "a", Operation: metav1.ManagedFieldsOperationApply}, + }, + expected: []metav1.ManagedFieldsEntry{ + {Manager: "a", Operation: metav1.ManagedFieldsOperationApply}, + {Manager: "a", Operation: metav1.ManagedFieldsOperationApply, Subresource: "scale"}, + {Manager: "a", Operation: metav1.ManagedFieldsOperationApply, Subresource: "status"}, + }, + }, } for _, test := range tests { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater_test.go index 14e9aa167da..23d8ef0b731 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/lastappliedupdater_test.go @@ -27,7 +27,7 @@ import ( func TestLastAppliedUpdater(t *testing.T) { f := NewTestFieldManager(schema.FromAPIVersionAndKind("apps/v1", "Deployment"), - false, + "", func(m Manager) Manager { return NewLastAppliedUpdater(m) }) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater_test.go index 2e1ad3d0b11..3c1b39b8104 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/managedfieldsupdater_test.go @@ -27,7 +27,7 @@ import ( ) func TestNoManagedFieldsUpdateDoesntUpdateTime(t *testing.T) { - f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), false, nil) + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), "", nil) obj := &unstructured.Unstructured{Object: map[string]interface{}{}} if err := yaml.Unmarshal([]byte(`{ diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/skipnonapplied_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/skipnonapplied_test.go index c9e2dc02b13..a001dd0c02e 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/skipnonapplied_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/skipnonapplied_test.go @@ -42,7 +42,7 @@ func (f *fakeObjectCreater) New(_ schema.GroupVersionKind) (runtime.Object, erro } func TestNoUpdateBeforeFirstApply(t *testing.T) { - f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), false, func(m Manager) Manager { + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), "", func(m Manager) Manager { return NewSkipNonAppliedManager( m, &fakeObjectCreater{gvk: schema.GroupVersionKind{Version: "v1", Kind: "Pod"}}, @@ -82,7 +82,7 @@ func TestNoUpdateBeforeFirstApply(t *testing.T) { } func TestUpdateBeforeFirstApply(t *testing.T) { - f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), false, func(m Manager) Manager { + f := NewTestFieldManager(schema.FromAPIVersionAndKind("v1", "Pod"), "", func(m Manager) Manager { return NewSkipNonAppliedManager( m, &fakeObjectCreater{gvk: schema.GroupVersionKind{Version: "v1", Kind: "Pod"}}, diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index caaa4b2e840..eb100646538 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -607,7 +607,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag a.group.Creater, fqKindToRegister, reqScope.HubGroupVersion, - isSubresource, + subresource, resetFields, ) if err != nil { diff --git a/test/integration/apiserver/apply/apply_test.go b/test/integration/apiserver/apply/apply_test.go index f654f886b89..71622b637af 100644 --- a/test/integration/apiserver/apply/apply_test.go +++ b/test/integration/apiserver/apply/apply_test.go @@ -2920,3 +2920,509 @@ spec: t.Errorf("object has unexpected managedFields: %v", managed) } } + +func TestRenamingAppliedFieldManagers(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)() + + _, client, closeFn := setup(t) + defer closeFn() + + // Creating an object + podBytes := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "just-a-pod", + "labels": { + "a": "one" + } + }, + "spec": { + "containers": [{ + "name": "test-container-a", + "image": "test-image-one" + }] + } + }`) + _, err := client.CoreV1().RESTClient(). + Patch(types.ApplyPatchType). + Namespace("default"). + Param("fieldManager", "multi_manager_one"). + Resource("pods"). + Name("just-a-pod"). + Body(podBytes). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to apply: %v", err) + } + _, err = client.CoreV1().RESTClient(). + Patch(types.ApplyPatchType). + Namespace("default"). + Param("fieldManager", "multi_manager_two"). + Resource("pods"). + Name("just-a-pod"). + Body([]byte(`{"apiVersion":"v1","kind":"Pod","metadata":{"labels":{"b":"two"}}}`)). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to apply: %v", err) + } + + pod, err := client.CoreV1().Pods("default").Get(context.TODO(), "just-a-pod", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get object: %v", err) + } + expectedLabels := map[string]string{ + "a": "one", + "b": "two", + } + if !reflect.DeepEqual(pod.Labels, expectedLabels) { + t.Fatalf("Expected labels to be %v, but got %v", expectedLabels, pod.Labels) + } + + managedFields := pod.GetManagedFields() + for i := range managedFields { + managedFields[i].Manager = "multi_manager" + } + pod.SetManagedFields(managedFields) + + obj, err := client.CoreV1().RESTClient(). + Put(). + Namespace("default"). + Resource("pods"). + Name("just-a-pod"). + Body(pod). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to update: %v", err) + } + + accessor, err := meta.Accessor(obj) + if err != nil { + t.Fatalf("Failed to get meta accessor for object: %v", err) + } + managedFields = accessor.GetManagedFields() + if len(managedFields) != 1 { + t.Fatalf("Expected object to have 1 managed fields entry, got: %d", len(managedFields)) + } + entry := managedFields[0] + if entry.Manager != "multi_manager" || entry.Operation != "Apply" || string(entry.FieldsV1.Raw) != `{"f:metadata":{"f:labels":{"f:b":{}}}}` { + t.Fatalf(`Unexpected entry, got: %v`, entry) + } +} + +func TestRenamingUpdatedFieldManagers(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)() + + _, client, closeFn := setup(t) + defer closeFn() + + // Creating an object + podBytes := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "just-a-pod" + }, + "spec": { + "containers": [{ + "name": "test-container-a", + "image": "test-image-one" + }] + } + }`) + _, err := client.CoreV1().RESTClient(). + Patch(types.ApplyPatchType). + Namespace("default"). + Param("fieldManager", "first"). + Resource("pods"). + Name("just-a-pod"). + Body(podBytes). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to create object: %v", err) + } + + _, err = client.CoreV1().RESTClient(). + Patch(types.MergePatchType). + Namespace("default"). + Param("fieldManager", "multi_manager_one"). + Resource("pods"). + Name("just-a-pod"). + Body([]byte(`{"metadata":{"labels":{"a":"one"}}}`)). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to update: %v", err) + } + _, err = client.CoreV1().RESTClient(). + Patch(types.MergePatchType). + Namespace("default"). + Param("fieldManager", "multi_manager_two"). + Resource("pods"). + Name("just-a-pod"). + Body([]byte(`{"metadata":{"labels":{"b":"two"}}}`)). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to update: %v", err) + } + + pod, err := client.CoreV1().Pods("default").Get(context.TODO(), "just-a-pod", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get object: %v", err) + } + expectedLabels := map[string]string{ + "a": "one", + "b": "two", + } + if !reflect.DeepEqual(pod.Labels, expectedLabels) { + t.Fatalf("Expected labels to be %v, but got %v", expectedLabels, pod.Labels) + } + + managedFields := pod.GetManagedFields() + for i := range managedFields { + managedFields[i].Manager = "multi_manager" + } + pod.SetManagedFields(managedFields) + + obj, err := client.CoreV1().RESTClient(). + Put(). + Namespace("default"). + Resource("pods"). + Name("just-a-pod"). + Body(pod). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to update: %v", err) + } + + accessor, err := meta.Accessor(obj) + if err != nil { + t.Fatalf("Failed to get meta accessor for object: %v", err) + } + managedFields = accessor.GetManagedFields() + if len(managedFields) != 2 { + t.Fatalf("Expected object to have 2 managed fields entries, got: %d", len(managedFields)) + } + entry := managedFields[1] + if entry.Manager != "multi_manager" || entry.Operation != "Update" || string(entry.FieldsV1.Raw) != `{"f:metadata":{"f:labels":{"f:b":{}}}}` { + t.Fatalf(`Unexpected entry, got: %v`, entry) + } +} + +func TestDroppingSubresourceField(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)() + + _, client, closeFn := setup(t) + defer closeFn() + + // Creating an object + podBytes := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "just-a-pod" + }, + "spec": { + "containers": [{ + "name": "test-container-a", + "image": "test-image-one" + }] + } + }`) + _, err := client.CoreV1().RESTClient(). + Patch(types.ApplyPatchType). + Namespace("default"). + Param("fieldManager", "first"). + Resource("pods"). + Name("just-a-pod"). + Body(podBytes). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to create object: %v", err) + } + + _, err = client.CoreV1().RESTClient(). + Patch(types.ApplyPatchType). + Namespace("default"). + Param("fieldManager", "label_manager"). + Resource("pods"). + Name("just-a-pod"). + Body([]byte(`{"apiVersion":"v1","kind":"Pod","metadata":{"labels":{"a":"one"}}}`)). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to apply: %v", err) + } + _, err = client.CoreV1().RESTClient(). + Patch(types.ApplyPatchType). + Namespace("default"). + Param("fieldManager", "label_manager"). + Resource("pods"). + Name("just-a-pod"). + SubResource("status"). + Body([]byte(`{"apiVersion":"v1","kind":"Pod","metadata":{"labels":{"b":"two"}}}`)). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to apply: %v", err) + } + + pod, err := client.CoreV1().Pods("default").Get(context.TODO(), "just-a-pod", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get object: %v", err) + } + expectedLabels := map[string]string{ + "a": "one", + "b": "two", + } + if !reflect.DeepEqual(pod.Labels, expectedLabels) { + t.Fatalf("Expected labels to be %v, but got %v", expectedLabels, pod.Labels) + } + + managedFields := pod.GetManagedFields() + if len(managedFields) != 3 { + t.Fatalf("Expected object to have 3 managed fields entries, got: %d", len(managedFields)) + } + if managedFields[1].Manager != "label_manager" || managedFields[1].Operation != "Apply" || managedFields[1].Subresource != "" { + t.Fatalf(`Unexpected entry, got: %v`, managedFields[1]) + } + if managedFields[2].Manager != "label_manager" || managedFields[2].Operation != "Apply" || managedFields[2].Subresource != "status" { + t.Fatalf(`Unexpected entry, got: %v`, managedFields[2]) + } + + for i := range managedFields { + managedFields[i].Subresource = "" + } + pod.SetManagedFields(managedFields) + + obj, err := client.CoreV1().RESTClient(). + Put(). + Namespace("default"). + Resource("pods"). + Name("just-a-pod"). + Body(pod). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to update: %v", err) + } + + accessor, err := meta.Accessor(obj) + if err != nil { + t.Fatalf("Failed to get meta accessor for object: %v", err) + } + managedFields = accessor.GetManagedFields() + if len(managedFields) != 2 { + t.Fatalf("Expected object to have 2 managed fields entries, got: %d", len(managedFields)) + } + entry := managedFields[1] + if entry.Manager != "label_manager" || entry.Operation != "Apply" || string(entry.FieldsV1.Raw) != `{"f:metadata":{"f:labels":{"f:b":{}}}}` { + t.Fatalf(`Unexpected entry, got: %v`, entry) + } +} + +func TestDroppingSubresourceFromSpecField(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)() + + _, client, closeFn := setup(t) + defer closeFn() + + // Creating an object + podBytes := []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "just-a-pod" + }, + "spec": { + "containers": [{ + "name": "test-container-a", + "image": "test-image-one" + }] + } + }`) + _, err := client.CoreV1().RESTClient(). + Patch(types.ApplyPatchType). + Namespace("default"). + Param("fieldManager", "first"). + Resource("pods"). + Name("just-a-pod"). + Body(podBytes). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to create object: %v", err) + } + + _, err = client.CoreV1().RESTClient(). + Patch(types.MergePatchType). + Namespace("default"). + Param("fieldManager", "manager"). + Resource("pods"). + Name("just-a-pod"). + Body([]byte(`{"metadata":{"labels":{"a":"two"}}}`)). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to update: %v", err) + } + + _, err = client.CoreV1().RESTClient(). + Patch(types.MergePatchType). + Namespace("default"). + Param("fieldManager", "manager"). + Resource("pods"). + SubResource("status"). + Name("just-a-pod"). + Body([]byte(`{"status":{"phase":"Running"}}`)). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to apply: %v", err) + } + + pod, err := client.CoreV1().Pods("default").Get(context.TODO(), "just-a-pod", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get object: %v", err) + } + expectedLabels := map[string]string{"a": "two"} + if !reflect.DeepEqual(pod.Labels, expectedLabels) { + t.Fatalf("Expected labels to be %v, but got %v", expectedLabels, pod.Labels) + } + if pod.Status.Phase != v1.PodRunning { + t.Fatalf("Expected phase to be %q, but got %q", v1.PodRunning, pod.Status.Phase) + } + + managedFields := pod.GetManagedFields() + if len(managedFields) != 3 { + t.Fatalf("Expected object to have 3 managed fields entries, got: %d", len(managedFields)) + } + if managedFields[1].Manager != "manager" || managedFields[1].Operation != "Update" || managedFields[1].Subresource != "" { + t.Fatalf(`Unexpected entry, got: %v`, managedFields[1]) + } + if managedFields[2].Manager != "manager" || managedFields[2].Operation != "Update" || managedFields[2].Subresource != "status" { + t.Fatalf(`Unexpected entry, got: %v`, managedFields[2]) + } + + for i := range managedFields { + managedFields[i].Subresource = "" + } + pod.SetManagedFields(managedFields) + + obj, err := client.CoreV1().RESTClient(). + Put(). + Namespace("default"). + Resource("pods"). + Name("just-a-pod"). + Body(pod). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to update: %v", err) + } + + accessor, err := meta.Accessor(obj) + if err != nil { + t.Fatalf("Failed to get meta accessor for object: %v", err) + } + managedFields = accessor.GetManagedFields() + if len(managedFields) != 2 { + t.Fatalf("Expected object to have 2 managed fields entries, got: %d", len(managedFields)) + } + entry := managedFields[1] + if entry.Manager != "manager" || entry.Operation != "Update" || string(entry.FieldsV1.Raw) != `{"f:status":{"f:phase":{}}}` { + t.Fatalf(`Unexpected entry, got: %v`, entry) + } +} + +func TestSubresourceField(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)() + + _, client, closeFn := setup(t) + defer closeFn() + + // Creating a deployment + deploymentBytes := []byte(`{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "name": "deployment", + "labels": {"app": "nginx"} + }, + "spec": { + "replicas": 3, + "selector": { + "matchLabels": { + "app": "nginx" + } + }, + "template": { + "metadata": { + "labels": { + "app": "nginx" + } + }, + "spec": { + "containers": [{ + "name": "nginx", + "image": "nginx:latest" + }] + } + } + } + }`) + _, err := client.CoreV1().RESTClient(). + Patch(types.ApplyPatchType). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + Name("deployment"). + Param("fieldManager", "manager"). + Body(deploymentBytes).Do(context.TODO()).Get() + if err != nil { + t.Fatalf("Failed to apply object: %v", err) + } + + _, err = client.CoreV1().RESTClient(). + Patch(types.MergePatchType). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + SubResource("status"). + Name("deployment"). + Body([]byte(`{"status":{"unavailableReplicas":32}}`)). + Param("fieldManager", "manager"). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to update status: %v", err) + } + + // TODO (nodo): add test for "scale" once we start tracking managed fields (#82046) + + deployment, err := client.AppsV1().Deployments("default").Get(context.TODO(), "deployment", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get object: %v", err) + } + + managedFields := deployment.GetManagedFields() + if len(managedFields) != 2 { + t.Fatalf("Expected object to have 3 managed fields entries, got: %d", len(managedFields)) + } + if managedFields[0].Manager != "manager" || managedFields[0].Operation != "Apply" || managedFields[0].Subresource != "" { + t.Fatalf(`Unexpected entry, got: %v`, managedFields[0]) + } + if managedFields[1].Manager != "manager" || + managedFields[1].Operation != "Update" || + managedFields[1].Subresource != "status" || + string(managedFields[1].FieldsV1.Raw) != `{"f:status":{"f:unavailableReplicas":{}}}` { + t.Fatalf(`Unexpected entry, got: %v`, managedFields[1]) + } +}