Skip to content

K
k3s
Commit d0735b90 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub
Browse files
Options

Merge pull request #48065 from ironcladlou/unstructured-field-fix

Automatic merge from submit-queue (batch tested with PRs 48183, 45611, 48065) Fix Unstructured field accessor Fix the Unstructured GetDeletionGracePeriodSeconds accessor which was always returning nil regardless of the underlying stored value. The field value always appearing nil prevents Custom Resource instances from being deleted when garbage collection is enabled for CRs and when DeletePropagationOrphan is used. More generally, this fix means that delete-on-update now works for CR instances. Add some test coverage for Unstructured metadata deserialization. The Unstructured DeletionGracePeriodSeconds field marshals as a value type from JSON and as a pointer type via SetDeletionGracePeriodSeconds. The GetDeletionGracePeriodSeconds method now supports handling both int64 and *int64 values so that either underlying value can be returned. Add a reflection-based unit test which attempts to exercise all the Object Get/Set methods for nil handling. ```release-note Registries backed by the generic Store's `Update` implementation support delete-on-update, which allows resources to be automatically deleted during an update provided: * Garbage collection is enabled for the Store * The resource being updated has no finalizers * The resource being updated has a non-nil DeletionGracePeriodSeconds equal to 0 With this fix, Custom Resource instances now also support delete-on-update behavior under the same circumstances. ```
parents 7c656ab4 547d8205
Hide whitespace changes
Inline Side-by-side
Showing with 114 additions and 30 deletions
pkg/apimachinery/tests/apis_meta_v1_unstructed_unstructure_test.go
View file @ d0735b90
...@@ -87,13 +87,13 @@ func TestDecode(t *testing.T) { ...@@ -87,13 +87,13 @@ func TestDecode(t *testing.T) {
}, },
}, },
{ {
json: []byte(`{"items": [{"metadata": {"name": "object1"}, "apiVersion": "test", "kind": "test_kind"}, {"metadata": {"name": "object2"}, "apiVersion": "test", "kind": "test_kind"}], "apiVersion": "test", "kind": "test_list"}`), json: []byte(`{"items": [{"metadata": {"name": "object1", "deletionGracePeriodSeconds": 10}, "apiVersion": "test", "kind": "test_kind"}, {"metadata": {"name": "object2"}, "apiVersion": "test", "kind": "test_kind"}], "apiVersion": "test", "kind": "test_list"}`),
want: &unstructured.UnstructuredList{ want: &unstructured.UnstructuredList{
Object: map[string]interface{}{"apiVersion": "test", "kind": "test_list"}, Object: map[string]interface{}{"apiVersion": "test", "kind": "test_list"},
Items: []unstructured.Unstructured{ Items: []unstructured.Unstructured{
{ {
Object: map[string]interface{}{ Object: map[string]interface{}{
"metadata": map[string]interface{}{"name": "object1"}, "metadata": map[string]interface{}{"name": "object1", "deletionGracePeriodSeconds": int64(10)},
"apiVersion": "test", "apiVersion": "test",
"kind": "test_kind", "kind": "test_kind",
}, },
...@@ -125,19 +125,22 @@ func TestDecode(t *testing.T) { ...@@ -125,19 +125,22 @@ func TestDecode(t *testing.T) {
func TestUnstructuredGetters(t *testing.T) { func TestUnstructuredGetters(t *testing.T) {
trueVar := true trueVar := true
ten := int64(10)
unstruct := unstructured.Unstructured{ unstruct := unstructured.Unstructured{
Object: map[string]interface{}{ Object: map[string]interface{}{
"kind": "test_kind", "kind": "test_kind",
"apiVersion": "test_version", "apiVersion": "test_version",
"metadata": map[string]interface{}{ "metadata": map[string]interface{}{
"name": "test_name", "name": "test_name",
"namespace": "test_namespace", "namespace": "test_namespace",
"generateName": "test_generateName", "generateName": "test_generateName",
"uid": "test_uid", "uid": "test_uid",
"resourceVersion": "test_resourceVersion", "resourceVersion": "test_resourceVersion",
"selfLink": "test_selfLink", "generation": ten,
"creationTimestamp": "2009-11-10T23:00:00Z", "deletionGracePeriodSeconds": ten,
"deletionTimestamp": "2010-11-10T23:00:00Z", "selfLink": "test_selfLink",
"creationTimestamp": "2009-11-10T23:00:00Z",
"deletionTimestamp": "2010-11-10T23:00:00Z",
"labels": map[string]interface{}{ "labels": map[string]interface{}{
"test_label": "test_value", "test_label": "test_value",
}, },
...@@ -244,25 +247,34 @@ func TestUnstructuredGetters(t *testing.T) { ...@@ -244,25 +247,34 @@ func TestUnstructuredGetters(t *testing.T) {
if got, want := unstruct.GetClusterName(), "cluster123"; got != want { if got, want := unstruct.GetClusterName(), "cluster123"; got != want {
t.Errorf("GetClusterName()=%v, want %v", got, want) t.Errorf("GetClusterName()=%v, want %v", got, want)
} }
if got, want := unstruct.GetDeletionGracePeriodSeconds(), &ten; !reflect.DeepEqual(got, want) {
t.Errorf("GetDeletionGracePeriodSeconds()=%v, want %v", got, want)
}
if got, want := unstruct.GetGeneration(), ten; !reflect.DeepEqual(got, want) {
t.Errorf("GetGeneration()=%v, want %v", got, want)
}
} }
func TestUnstructuredSetters(t *testing.T) { func TestUnstructuredSetters(t *testing.T) {
unstruct := unstructured.Unstructured{} unstruct := unstructured.Unstructured{}
trueVar := true trueVar := true
ten := int64(10)
want := unstructured.Unstructured{ want := unstructured.Unstructured{
Object: map[string]interface{}{ Object: map[string]interface{}{
"kind": "test_kind", "kind": "test_kind",
"apiVersion": "test_version", "apiVersion": "test_version",
"metadata": map[string]interface{}{ "metadata": map[string]interface{}{
"name": "test_name", "name": "test_name",
"namespace": "test_namespace", "namespace": "test_namespace",
"generateName": "test_generateName", "generateName": "test_generateName",
"uid": "test_uid", "uid": "test_uid",
"resourceVersion": "test_resourceVersion", "resourceVersion": "test_resourceVersion",
"selfLink": "test_selfLink", "selfLink": "test_selfLink",
"creationTimestamp": "2009-11-10T23:00:00Z", "creationTimestamp": "2009-11-10T23:00:00Z",
"deletionTimestamp": "2010-11-10T23:00:00Z", "deletionTimestamp": "2010-11-10T23:00:00Z",
"deletionGracePeriodSeconds": &ten,
"generation": ten,
"labels": map[string]interface{}{ "labels": map[string]interface{}{
"test_label": "test_value", "test_label": "test_value",
}, },
...@@ -326,6 +338,8 @@ func TestUnstructuredSetters(t *testing.T) { ...@@ -326,6 +338,8 @@ func TestUnstructuredSetters(t *testing.T) {
unstruct.SetOwnerReferences(newOwnerReferences) unstruct.SetOwnerReferences(newOwnerReferences)
unstruct.SetFinalizers([]string{"finalizer.1", "finalizer.2"}) unstruct.SetFinalizers([]string{"finalizer.1", "finalizer.2"})
unstruct.SetClusterName("cluster123") unstruct.SetClusterName("cluster123")
unstruct.SetDeletionGracePeriodSeconds(&ten)
unstruct.SetGeneration(ten)
if !reflect.DeepEqual(unstruct, want) { if !reflect.DeepEqual(unstruct, want) {
t.Errorf("Wanted: \n%s\n Got:\n%s", want, unstruct) t.Errorf("Wanted: \n%s\n Got:\n%s", want, unstruct)
...@@ -493,3 +507,72 @@ func TestDecodeNumbers(t *testing.T) { ...@@ -493,3 +507,72 @@ func TestDecodeNumbers(t *testing.T) {
t.Fatalf("Expected\n\t%#v, got \n\t%#v", pod, pod2) t.Fatalf("Expected\n\t%#v, got \n\t%#v", pod, pod2)
} }
} }
// TestAccessorMethods does opaque roundtrip testing against an Unstructured
// instance's Object methods to ensure that what is "Set" matches what you
// subsequently "Get" without any assertions against internal state.
func TestAccessorMethods(t *testing.T) {
int64p := func(i int) *int64 {
v := int64(i)
return &v
}
tests := []struct {
accessor string
val interface{}
nilVal reflect.Value
}{
{accessor: "Namespace", val: "foo"},
{accessor: "Name", val: "bar"},
{accessor: "GenerateName", val: "baz"},
{accessor: "UID", val: types.UID("uid")},
{accessor: "ResourceVersion", val: "1"},
{accessor: "Generation", val: int64(5)},
{accessor: "SelfLink", val: "/foo"},
// TODO: Handle timestamps, which are being marshalled as UTC and
// unmarshalled as Local.
// https://github.com/kubernetes/kubernetes/issues/21402
// {accessor: "CreationTimestamp", val: someTime},
// {accessor: "DeletionTimestamp", val: someTimeP},
{accessor: "DeletionTimestamp", nilVal: reflect.ValueOf((*metav1.Time)(nil))},
{accessor: "DeletionGracePeriodSeconds", val: int64p(10)},
{accessor: "DeletionGracePeriodSeconds", val: int64p(0)},
{accessor: "DeletionGracePeriodSeconds", nilVal: reflect.ValueOf((*int64)(nil))},
{accessor: "Labels", val: map[string]string{"foo": "bar"}},
{accessor: "Annotations", val: map[string]string{"foo": "bar"}},
{accessor: "Initializers", val: &metav1.Initializers{Pending: []metav1.Initializer{{Name: "foo"}}}},
{accessor: "Initializers", val: &metav1.Initializers{}},
{accessor: "Initializers", nilVal: reflect.ValueOf((*metav1.Initializers)(nil))},
{accessor: "Finalizers", val: []string{"foo"}},
{accessor: "OwnerReferences", val: []metav1.OwnerReference{{Name: "foo"}}},
{accessor: "ClusterName", val: "foo"},
}
for i, test := range tests {
t.Logf("evaluating test %d (%s)", i, test.accessor)
u := &unstructured.Unstructured{}
setter := reflect.ValueOf(u).MethodByName("Set" + test.accessor)
getter := reflect.ValueOf(u).MethodByName("Get" + test.accessor)
args := []reflect.Value{}
if test.val != nil {
args = append(args, reflect.ValueOf(test.val))
} else {
args = append(args, test.nilVal)
}
setter.Call(args)
ret := getter.Call([]reflect.Value{})
actual := ret[0].Interface()
var expected interface{}
if test.val != nil {
expected = test.val
} else {
expected = test.nilVal.Interface()
}
if e, a := expected, actual; !reflect.DeepEqual(e, a) {
t.Fatalf("%s: expected %v (%T), got %v (%T)", test.accessor, e, e, a, a)
}
}
}
staging/src/k8s.io/apiextensions-apiserver/test/integration/finalization_test.go
View file @ d0735b90
...@@ -69,7 +69,8 @@ func TestFinalization(t *testing.T) { ...@@ -69,7 +69,8 @@ func TestFinalization(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
// Removing the finalizers to allow the following delete remove the object. // Removing the finalizers to allow the following delete remove the object.
// This step will fail if previous delete wrongly removed the object. // This step will fail if previous delete wrongly removed the object. The
// object will be deleted as part of the finalizer update.
for { for {
gottenNoxuInstance.SetFinalizers(nil) gottenNoxuInstance.SetFinalizers(nil)
_, err = noxuResourceClient.Update(gottenNoxuInstance) _, err = noxuResourceClient.Update(gottenNoxuInstance)
...@@ -83,14 +84,6 @@ func TestFinalization(t *testing.T) { ...@@ -83,14 +84,6 @@ func TestFinalization(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
} }
// Now when finalizer is not there it should be possible to actually remove the object from the server.
err = noxuResourceClient.Delete(name, &metav1.DeleteOptions{
Preconditions: &metav1.Preconditions{
UID: &uid,
},
})
require.NoError(t, err)
// Check that the object is actually gone. // Check that the object is actually gone.
_, err = noxuResourceClient.Get(name, metav1.GetOptions{}) _, err = noxuResourceClient.Get(name, metav1.GetOptions{})
require.Error(t, err) require.Error(t, err)
......
staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go
View file @ d0735b90
...@@ -136,10 +136,15 @@ func getNestedInt64(obj map[string]interface{}, fields ...string) int64 { ...@@ -136,10 +136,15 @@ func getNestedInt64(obj map[string]interface{}, fields ...string) int64 {
} }
func getNestedInt64Pointer(obj map[string]interface{}, fields ...string) *int64 { func getNestedInt64Pointer(obj map[string]interface{}, fields ...string) *int64 {
if str, ok := getNestedField(obj, fields...).(*int64); ok { nested := getNestedField(obj, fields...)
return str switch n := nested.(type) {
case int64:
return &n
case *int64:
return n
default:
return nil
} }
return nil
} }
func getNestedSlice(obj map[string]interface{}, fields ...string) []string { func getNestedSlice(obj map[string]interface{}, fields ...string) []string {
...@@ -470,6 +475,9 @@ func (u *Unstructured) GetInitializers() *metav1.Initializers { ...@@ -470,6 +475,9 @@ func (u *Unstructured) GetInitializers() *metav1.Initializers {
} }
func (u *Unstructured) SetInitializers(initializers *metav1.Initializers) { func (u *Unstructured) SetInitializers(initializers *metav1.Initializers) {
if u.Object == nil {
u.Object = make(map[string]interface{})
}
if initializers == nil { if initializers == nil {
setNestedField(u.Object, nil, "metadata", "initializers") setNestedField(u.Object, nil, "metadata", "initializers")
return return
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment