From d4b7411f65fd9dcfc6b0f6578f371bb98345a7fa Mon Sep 17 00:00:00 2001 From: jennybuckley Date: Wed, 17 Oct 2018 11:17:58 -0700 Subject: [PATCH] convert lastapplied to ownerclaims --- .../k8s.io/apimachinery/pkg/api/meta/meta.go | 2 +- .../apimachinery/pkg/apis/meta/v1/meta.go | 16 +- .../apimachinery/pkg/apis/meta/v1/types.go | 162 +++++++++++++++++- .../apis/meta/v1/unstructured/unstructured.go | 14 +- .../apiserver/pkg/endpoints/handlers/apply.go | 21 ++- .../pkg/endpoints/patchhandler_test.go | 20 ++- .../apiserver/pkg/registry/rest/create.go | 4 +- .../apiserver/pkg/registry/rest/update.go | 6 +- 8 files changed, 202 insertions(+), 43 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/meta.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/meta.go index 9e3f7c9c196..dd01d435a9f 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/meta.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/meta.go @@ -138,7 +138,7 @@ func AsPartialObjectMetadata(m metav1.Object) *metav1beta1.PartialObjectMetadata Finalizers: m.GetFinalizers(), ClusterName: m.GetClusterName(), Initializers: m.GetInitializers(), - LastApplied: m.GetLastApplied(), + ManagedFields: m.GetManagedFields(), }, } } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/meta.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/meta.go index 5673edaf6de..40c610b9065 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/meta.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/meta.go @@ -63,8 +63,8 @@ type Object interface { SetOwnerReferences([]OwnerReference) GetClusterName() string SetClusterName(clusterName string) - GetLastApplied() map[string]string - SetLastApplied(lastApplied map[string]string) + GetManagedFields() map[string]VersionedFieldSet + SetManagedFields(lastApplied map[string]VersionedFieldSet) } // ListMetaAccessor retrieves the list interface from an object @@ -171,13 +171,13 @@ func (meta *ObjectMeta) SetOwnerReferences(references []OwnerReference) { func (meta *ObjectMeta) GetClusterName() string { return meta.ClusterName } func (meta *ObjectMeta) SetClusterName(clusterName string) { meta.ClusterName = clusterName } -func (meta *ObjectMeta) GetLastApplied() map[string]string { - return meta.LastApplied +func (meta *ObjectMeta) GetManagedFields() map[string]VersionedFieldSet { + return meta.ManagedFields } -func (meta *ObjectMeta) SetLastApplied(lastApplied map[string]string) { - meta.LastApplied = make(map[string]string, len(lastApplied)) - for key, value := range lastApplied { - meta.LastApplied[key] = value +func (meta *ObjectMeta) SetManagedFields(ManagedFields map[string]VersionedFieldSet) { + meta.ManagedFields = make(map[string]VersionedFieldSet, len(ManagedFields)) + for key, value := range ManagedFields { + meta.ManagedFields[key] = value } } 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 333887eb02a..a3989e28898 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 @@ -250,14 +250,79 @@ type ObjectMeta struct { // +optional ClusterName string `json:"clusterName,omitempty" protobuf:"bytes,15,opt,name=clusterName"` - // LastApplied is a map of workflow-id to last applied - // configuration. A workflow can be the user's name, a + // ManagedFields is a map of workflow-id to the set of fields + // that are managed by that workflow. This is mostly for internal + // housekeeping, and users typically shouldn't need to set or + // understand this field. A workflow can be the user's name, a // controller's name, or the name of a specific apply path like - // "ci-cd". It keeps track of the ownership of fields. The last - // applied configuration is always in the same version as the - // parent object. It's used to keep track of the intent of the - // workflow-id that submitted that configuration. - LastApplied map[string]string `json:"lastApplied,omitempty" protobuf:"bytes,17,rep,name=lastApplied"` + // "ci-cd". The set of fields is always in the version that the + // workflow used when modifying the object. + // + // An example value of this field for a deployment with a + // single container managed by a user: + // { + // "user-workflow-id": { + // "apiVersion": "extensions/v1beta", + // "fields": { + // "children": [{ + // "pathElement": { + // "fieldName": "spec" + // }, + // "set": { + // "children": [{ + // "pathElement": { + // "fieldName": "template" + // }, + // "set": { + // "children": [{ + // "pathElement": { + // "fieldName": "spec" + // }, + // "set": { + // "children": [{ + // "pathElement": { + // "fieldName": "containers" + // }, + // "set": { + // "members": [{ + // "key": [{ + // "name": "name", + // "value": { + // "stringValue": "my-container" + // } + // }] + // }], + // "children": [{ + // "pathElement": { + // "key": [{ + // "name": "name", + // "value": { + // "stringValue": "my-container" + // } + // }] + // }, + // "set": { + // "members": [{ + // "fieldName": "name" + // }, + // { + // "fieldName": "image" + // }] + // } + // }] + // } + // }] + // } + // }] + // } + // }] + // } + // }] + // } + // } + // } + // +optional + ManagedFields map[string]VersionedFieldSet `json:"managedFields,omitempty" protobuf:"bytes,17,rep,name=managedFields"` } // Initializers tracks the progress of initialization. @@ -1005,3 +1070,86 @@ const ( LabelSelectorOpExists LabelSelectorOperator = "Exists" LabelSelectorOpDoesNotExist LabelSelectorOperator = "DoesNotExist" ) + +// VersionedFieldSet is a pair of a FieldSet and the group version of the resource +// that the fieldset applies to. +type VersionedFieldSet struct { + // APIVersion defines the version of this resource that this field set + // applies to. The format is "group/version" just like the top-level + // APIVersion field. It is necessary to track the version of a field + // set because it cannot be automatically converted. + APIVersion string `json:"apiVersion,omitempty" protobuf:"bytes,1,opt,name=apiVersion"` + // Fields identifies a set of fields. + Fields FieldSet `json:"fields,omitempty" protobuf:"bytes,2,opt,name=fields,casttype=FieldSet"` +} + +// FieldSet stores a set of fields in a data structure like a Trie. +// To understand how this is used, see: https://github.com/kubernetes-sigs/structured-merge-diff +type FieldSet struct { + // Members lists fields that are part of the set. + Members []FieldPathElement `json:"members" protobuf:"bytes,1,rep,name=members"` + // Children lists child fields which themselves have children that are + // members of the set. Appearance in this list does not imply membership, + // although it does imply some descendant field is a member. + // Note: this is a tree, not an arbitrary graph. + Children []FieldSetNode `json:"children" protobuf:"bytes,2,rep,name=children"` +} + +// FieldPathElement describes how to select a child field given a containing object. +type FieldPathElement struct { + // Exactly one of the following fields should be non-nil. + // FieldName selects a single field from a map (reminder: this is also + // how structs are represented). The containing object must be a map. + // +optional + FieldName *string `json:"fieldName,omitempty" protobuf:"bytes,1,opt,name=fieldName"` + // Key selects the list element which has fields matching those given. + // The containing object must be an associative list with map typed + // elements. + // +optional + Key []FieldNameValuePair `json:"key,omitempty" protobuf:"bytes,2,rep,name=key"` + // Value selects the list element with the given value. The containing + // object must be an associative list with a primitive typed element + // (i.e., a set). + // +optional + Value *FieldValue `json:"value,omitempty" protobuf:"bytes,3,opt,name=value,casttype=FieldValue"` + // Index selects a list element by its index number. The containing + // object must be an atomic list. + // +optional + Index *int32 `json:"index,omitempty" protobuf:"varint,4,opt,name=index"` +} + +// FieldSetNode is a pair of FieldPathElement / FieldSet, for the purpose of expressing +// nested set membership. +type FieldSetNode struct { + // PathElement identifies which field this node expresses child membership for. + PathElement FieldPathElement `json:"pathElement" protobuf:"bytes,1,opt,name=pathElement,casttype=FieldPathElement"` + // Set identifies which child fields of this node are part of the FieldSet. + Set *FieldSet `json:"set" protobuf:"bytes,2,opt,name=set,casttype=FieldSet"` +} + +// FieldNameValuePair is an individual key-value pair. +type FieldNameValuePair struct { + // Name is the field's name. + Name string `json:"name" protobuf:"bytes,1,opt,name=name"` + // Value is the field's value. + Value FieldValue `json:"value" protobuf:"bytes,2,opt,name=value,casttype=FieldValue"` +} + +// FieldValue represents a concrete value, either for a key-value pair or identifying an item in a set. +type FieldValue struct { + // Exactly one of the following fields should be set. + // FloatValue is a primitive float value. + // +optional + FloatValue *float64 `json:"floatValue,omitempty" protobuf:"bytes,1,opt,name=floatValue"` + // IntValue is a primitive int value. + // +optional + IntValue *int32 `json:"intValue,omitempty" protobuf:"varint,2,opt,name=intValue"` + // StringValue is a primitive string value. + // +optional + StringValue *string `json:"stringValue,omitempty" protobuf:"bytes,3,opt,name=stringValue"` + // BooleanValue is a primitive boolean value. + // +optional + BooleanValue *bool `json:"booleanValue,omitempty" protobuf:"varint,4,opt,name=booleanValue"` + // Null represents an explicit `"foo" = null` + Null bool `json:"null" protobuf:"varint,5,opt,name=null"` +} diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go index 5234604c7db..23ca5ff46b8 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go @@ -398,11 +398,15 @@ func (u *Unstructured) SetClusterName(clusterName string) { u.setNestedField(clusterName, "metadata", "clusterName") } -func (u *Unstructured) GetLastApplied() map[string]string { - val, _, _ := NestedStringMap(u.Object, "metadata", "lastApplied") - return val +func (u *Unstructured) GetManagedFields() map[string]metav1.VersionedFieldSet { + unstructuredManagedFields, _, _ := NestedStringMap(u.Object, "metadata", "managedFields") + managedFields := make(map[string]metav1.VersionedFieldSet, len(unstructuredManagedFields)) + // TODO: convert from unstructured + return managedFields } -func (u *Unstructured) SetLastApplied(lastApplied map[string]string) { - u.setNestedMap(lastApplied, "metadata", "lastApplied") +func (u *Unstructured) SetManagedFields(managedFields map[string]metav1.VersionedFieldSet) { + unstructuredManagedFields := make(map[string]string, len(managedFields)) + // TODO: convert to unstructured + u.setNestedMap(unstructuredManagedFields, "metadata", "managedFields") } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/apply.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/apply.go index b0613fea99b..ebd7c7f4e59 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/apply.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/apply.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/apply" "k8s.io/apimachinery/pkg/apply/parse" @@ -56,9 +57,10 @@ func (p *applyPatcher) extractLastIntent(obj runtime.Object, workflow string) (m return nil, fmt.Errorf("couldn't get accessor: %v", err) } last := make(map[string]interface{}) - if accessor.GetLastApplied()[workflow] != "" { - if err := json.Unmarshal([]byte(accessor.GetLastApplied()[workflow]), &last); err != nil { - return nil, fmt.Errorf("couldn't unmarshal last applied field: %v", err) + // TODO: use the managedFields correctly + if _, ok := accessor.GetManagedFields()[workflow]; ok { + if err := json.Unmarshal([]byte(accessor.GetManagedFields()[workflow].APIVersion), &last); err != nil { + return nil, fmt.Errorf("couldn't unmarshal managedFields field: %v", err) } } return last, nil @@ -105,12 +107,15 @@ func (p *applyPatcher) saveNewIntent(patch map[string]interface{}, workflow stri if err != nil { return fmt.Errorf("couldn't get accessor: %v", err) } - m := accessor.GetLastApplied() + m := accessor.GetManagedFields() if m == nil { - m = make(map[string]string) + m = make(map[string]metav1.VersionedFieldSet) } - m[workflow] = string(j) - accessor.SetLastApplied(m) + // TODO: save the managedFields correctly + m[workflow] = metav1.VersionedFieldSet{ + APIVersion: string(j), + } + accessor.SetManagedFields(m) return nil } @@ -147,7 +152,7 @@ func (p *applyPatcher) applyPatchToCurrentObject(currentObject runtime.Object) ( return nil, fmt.Errorf("failed to save last intent: %v", err) } - // TODO(apelisse): Check for conflicts with other lastApplied + // TODO(apelisse): Check for conflicts with other managedFields // and report actionable errors to users. return output, nil diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/patchhandler_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/patchhandler_test.go index e36df88e409..d388acde76d 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/patchhandler_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/patchhandler_test.go @@ -139,8 +139,8 @@ func TestPatchApply(t *testing.T) { if simpleStorage.updated.Other != "bar" { t.Errorf(`Merge should have kept initial "bar" value for Other: %v`, simpleStorage.updated.Other) } - if simpleStorage.updated.ObjectMeta.LastApplied["default"] == "" { - t.Errorf(`Expected lastApplied field to be set, but is empty`) + if _, ok := simpleStorage.updated.ObjectMeta.ManagedFields["default"]; !ok { + t.Errorf(`Expected managedFields field to be set, but is empty`) } } @@ -177,17 +177,18 @@ func TestApplyAddsGVK(t *testing.T) { if response.StatusCode != http.StatusOK { t.Errorf("Unexpected response %#v", response) } + // TODO: Need to fix this expected := `{"apiVersion":"test.group/version","kind":"Simple","labels":{"test":"yes"},"metadata":{"name":"id"}}` - if simpleStorage.updated.ObjectMeta.LastApplied["default"] != expected { + if simpleStorage.updated.ObjectMeta.ManagedFields["default"].APIVersion != expected { t.Errorf( - `Expected lastApplied field to be %q, got %q`, + `Expected managedFields field to be %q, got %q`, expected, - simpleStorage.updated.ObjectMeta.LastApplied["default"], + simpleStorage.updated.ObjectMeta.ManagedFields["default"].APIVersion, ) } } -func TestApplyCreatesWithLastApplied(t *testing.T) { +func TestApplyCreatesWithManagedFields(t *testing.T) { if err := utilfeature.DefaultFeatureGate.Set(string(genericfeatures.ServerSideApply) + "=true"); err != nil { t.Fatal(err) } @@ -212,12 +213,13 @@ func TestApplyCreatesWithLastApplied(t *testing.T) { if response.StatusCode != http.StatusOK { t.Errorf("Unexpected response %#v", response) } + // TODO: Need to fix this expected := `{"apiVersion":"test.group/version","kind":"Simple","labels":{"test":"yes"},"metadata":{"name":"id"}}` - if simpleStorage.updated.ObjectMeta.LastApplied["default"] != expected { + if simpleStorage.updated.ObjectMeta.ManagedFields["default"].APIVersion != expected { t.Errorf( - `Expected lastApplied field to be %q, got %q`, + `Expected managedFields field to be %q, got %q`, expected, - simpleStorage.updated.ObjectMeta.LastApplied["default"], + simpleStorage.updated.ObjectMeta.ManagedFields["default"].APIVersion, ) } } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go index b4ddcf77609..87babc979e6 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go @@ -97,9 +97,9 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime. objectMeta.SetInitializers(nil) } - // Ensure LastApplied is not set unless the feature is enabled + // Ensure managedFields is not set unless the feature is enabled if !utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) { - objectMeta.SetLastApplied(nil) + objectMeta.SetManagedFields(nil) } // ClusterName is ignored and should not be saved diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go index ff58c238f5e..05d7efe2f57 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go @@ -108,10 +108,10 @@ func BeforeUpdate(strategy RESTUpdateStrategy, ctx context.Context, obj, old run objectMeta.SetInitializers(nil) } - // Ensure lastApplied state is removed unless ServerSideApply is enabled + // Ensure managedFields state is removed unless ServerSideApply is enabled if !utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) { - oldMeta.SetLastApplied(map[string]string{}) - objectMeta.SetLastApplied(map[string]string{}) + oldMeta.SetManagedFields(nil) + objectMeta.SetManagedFields(nil) } strategy.PrepareForUpdate(ctx, obj, old)