Fix owners merge in apply unstructured

Signed-off-by: Jian Qiu <jqiu@redhat.com>
This commit is contained in:
Jian Qiu
2021-09-16 15:37:38 +08:00
parent f260e0b135
commit 72de5d1594
9 changed files with 168 additions and 26 deletions

View File

@@ -71,18 +71,18 @@ spec:
included in this manifestwork
type: object
properties:
Name:
group:
description: Group is the api group of the resources
in the workload that the strategy is applied
type: string
name:
description: Name is the names of the resources in the
workload that the strategy is applied
type: string
Namespace:
namespace:
description: Namespace is the namespaces of the resources
in the workload that the strategy is applied
type: string
group:
description: Group is the api group of the resources
in the workload that the strategy is applied
type: string
resource:
description: Resource is the resources in the workload
that the strategy is applied

2
go.mod
View File

@@ -21,7 +21,7 @@ require (
k8s.io/klog/v2 v2.9.0
k8s.io/kube-aggregator v0.22.1
k8s.io/utils v0.0.0-20210722164352-7f3ee0f31471
open-cluster-management.io/api v0.0.0-20210823013037-9667ae902e4b
open-cluster-management.io/api v0.0.0-20210916013819-2e58cdb938f9
sigs.k8s.io/controller-runtime v0.9.6
)

4
go.sum
View File

@@ -1132,8 +1132,8 @@ modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=
modernc.org/strutil v1.0.0/go.mod h1:lstksw84oURvj9y3tn8lGvRxyRC1S2+g5uuIzNfIOBs=
modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I=
open-cluster-management.io/api v0.0.0-20210823013037-9667ae902e4b h1:B2vuGyZdIULcE704+81Q4Kisd2ANzlv/+d38GVdcw6w=
open-cluster-management.io/api v0.0.0-20210823013037-9667ae902e4b/go.mod h1:9qiA5h/8kvPQnJEOlAPHVjRO9a1jCmDhGzvgMBvXEaE=
open-cluster-management.io/api v0.0.0-20210916013819-2e58cdb938f9 h1:ySrjJFbSuPbHEN0OvzTeQO8Bt93rjgvbce7lo2cQeZY=
open-cluster-management.io/api v0.0.0-20210916013819-2e58cdb938f9/go.mod h1:9qiA5h/8kvPQnJEOlAPHVjRO9a1jCmDhGzvgMBvXEaE=
rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=

View File

@@ -264,7 +264,7 @@ func (m *ManifestWorkController) applyOneManifest(
// TODO we should check the certain error.
// Use dynamic client when scheme cannot decode manifest or typed client cannot handle the object
if isDecodeError(result.Error) || isUnhandledError(result.Error) || isUnsupportedError(result.Error) {
result.Result, result.Changed, result.Error = m.applyUnstructured(ctx, manifest.Raw, owner, deleteOption, gvr, recorder)
result.Result, result.Changed, result.Error = m.applyUnstructured(ctx, manifest.Raw, owner, gvr, recorder)
}
return result
@@ -288,7 +288,6 @@ func (m *ManifestWorkController) applyUnstructured(
ctx context.Context,
data []byte,
owner metav1.OwnerReference,
deleteOption *workapiv1.DeleteOption,
gvr schema.GroupVersionResource,
recorder events.Recorder) (*unstructured.Unstructured, bool, error) {
@@ -297,6 +296,8 @@ func (m *ManifestWorkController) applyUnstructured(
return nil, false, err
}
required.SetOwnerReferences([]metav1.OwnerReference{owner})
existing, err := m.spokeDynamicClient.
Resource(gvr).
Namespace(required.GetNamespace()).
@@ -314,15 +315,13 @@ func (m *ManifestWorkController) applyUnstructured(
}
// Merge OwnerRefs.
required.SetOwnerReferences([]metav1.OwnerReference{owner})
existingOwners := existing.GetOwnerReferences()
modified := resourcemerge.BoolPtr(false)
resourcemerge.MergeOwnerRefs(modified, &existingOwners, required.GetOwnerReferences())
if *modified {
required.SetOwnerReferences(existingOwners)
}
// Always overwrite required ownerrefs from existing, since ownerrefs of required has been merged to existing
required.SetOwnerReferences(existingOwners)
// Compare and update the unstrcuctured.
if isSameUnstructured(required, existing) {

View File

@@ -1,6 +1,8 @@
package manifestcontroller
import (
"context"
"encoding/json"
"fmt"
"testing"
"time"
@@ -733,3 +735,144 @@ func TestManageOwner(t *testing.T) {
})
}
}
func TestApplyUnstructred(t *testing.T) {
cases := []struct {
name string
owner metav1.OwnerReference
existingObject []runtime.Object
required *unstructured.Unstructured
gvr schema.GroupVersionResource
validateActions func(t *testing.T, actions []clienttesting.Action)
}{
{
name: "create a new object with owner",
existingObject: []runtime.Object{},
owner: metav1.OwnerReference{Name: "test", UID: "testowner"},
required: spoketesting.NewUnstructured("v1", "Secret", "ns1", "test"),
gvr: schema.GroupVersionResource{Version: "v1", Resource: "secrets"},
validateActions: func(t *testing.T, actions []clienttesting.Action) {
if len(actions) != 2 {
t.Errorf("Expect 2 actions, but have %d", len(actions))
}
spoketesting.AssertAction(t, actions[0], "get")
spoketesting.AssertAction(t, actions[1], "create")
obj := actions[1].(clienttesting.CreateActionImpl).Object.(*unstructured.Unstructured)
owners := obj.GetOwnerReferences()
if len(owners) != 1 {
t.Errorf("Expect 2 owners, but have %d", len(owners))
}
if owners[0].UID != "testowner" {
t.Errorf("Owner UId is not correct, got %s", owners[0].UID)
}
},
},
{
name: "create a new object without owner",
existingObject: []runtime.Object{},
owner: metav1.OwnerReference{Name: "test", UID: "testowner-"},
required: spoketesting.NewUnstructured("v1", "Secret", "ns1", "test"),
gvr: schema.GroupVersionResource{Version: "v1", Resource: "secrets"},
validateActions: func(t *testing.T, actions []clienttesting.Action) {
if len(actions) != 2 {
t.Errorf("Expect 2 actions, but have %d", len(actions))
}
spoketesting.AssertAction(t, actions[0], "get")
spoketesting.AssertAction(t, actions[1], "create")
obj := actions[1].(clienttesting.CreateActionImpl).Object.(*unstructured.Unstructured)
owners := obj.GetOwnerReferences()
if len(owners) != 0 {
t.Errorf("Expect 1 owners, but have %d", len(owners))
}
},
},
{
name: "update an object owner",
existingObject: []runtime.Object{spoketesting.NewUnstructured("v1", "Secret", "ns1", "test", metav1.OwnerReference{Name: "test1", UID: "testowner1"})},
owner: metav1.OwnerReference{Name: "test", UID: "testowner"},
required: spoketesting.NewUnstructured("v1", "Secret", "ns1", "test"),
gvr: schema.GroupVersionResource{Version: "v1", Resource: "secrets"},
validateActions: func(t *testing.T, actions []clienttesting.Action) {
if len(actions) != 2 {
t.Errorf("Expect 2 actions, but have %d", len(actions))
}
spoketesting.AssertAction(t, actions[0], "get")
spoketesting.AssertAction(t, actions[1], "update")
obj := actions[1].(clienttesting.UpdateActionImpl).Object.(*unstructured.Unstructured)
owners := obj.GetOwnerReferences()
if len(owners) != 2 {
t.Errorf("Expect 2 owners, but have %d", len(owners))
}
if owners[0].UID != "testowner1" {
t.Errorf("Owner UId is not correct, got %s", owners[0].UID)
}
if owners[1].UID != "testowner" {
t.Errorf("Owner UId is not correct, got %s", owners[1].UID)
}
},
},
{
name: "update an object without owner",
existingObject: []runtime.Object{spoketesting.NewUnstructured("v1", "Secret", "ns1", "test", metav1.OwnerReference{Name: "test1", UID: "testowner1"})},
owner: metav1.OwnerReference{Name: "test", UID: "testowner-"},
required: spoketesting.NewUnstructured("v1", "Secret", "ns1", "test"),
gvr: schema.GroupVersionResource{Version: "v1", Resource: "secrets"},
validateActions: func(t *testing.T, actions []clienttesting.Action) {
if len(actions) != 1 {
t.Errorf("Expect 1 actions, but have %d", len(actions))
}
spoketesting.AssertAction(t, actions[0], "get")
},
},
{
name: "remove an object owner",
existingObject: []runtime.Object{spoketesting.NewUnstructured("v1", "Secret", "ns1", "test", metav1.OwnerReference{Name: "test", UID: "testowner"})},
owner: metav1.OwnerReference{Name: "test", UID: "testowner-"},
required: spoketesting.NewUnstructured("v1", "Secret", "ns1", "test"),
gvr: schema.GroupVersionResource{Version: "v1", Resource: "secrets"},
validateActions: func(t *testing.T, actions []clienttesting.Action) {
if len(actions) != 2 {
t.Errorf("Expect 2 actions, but have %d", len(actions))
}
spoketesting.AssertAction(t, actions[0], "get")
spoketesting.AssertAction(t, actions[1], "update")
obj := actions[1].(clienttesting.UpdateActionImpl).Object.(*unstructured.Unstructured)
owners := obj.GetOwnerReferences()
if len(owners) != 0 {
t.Errorf("Expect 0 owner, but have %d", len(owners))
}
},
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
work, workKey := spoketesting.NewManifestWork(0)
work.Finalizers = []string{controllers.ManifestWorkFinalizer}
controller := newController(work, nil, spoketesting.NewFakeRestMapper()).
withUnstructuredObject(c.existingObject...)
syncContext := spoketesting.NewFakeSyncContext(t, workKey)
data, _ := json.Marshal(c.required)
_, _, err := controller.controller.applyUnstructured(
context.TODO(), data, c.owner, c.gvr, syncContext.Recorder())
if err != nil {
t.Errorf("expect no error, but got %v", err)
}
c.validateActions(t, controller.dynamicClient.Actions())
})
}
}

2
vendor/modules.txt vendored
View File

@@ -1075,7 +1075,7 @@ k8s.io/utils/net
k8s.io/utils/path
k8s.io/utils/pointer
k8s.io/utils/trace
# open-cluster-management.io/api v0.0.0-20210823013037-9667ae902e4b
# open-cluster-management.io/api v0.0.0-20210916013819-2e58cdb938f9
## explicit
open-cluster-management.io/api/client/work/clientset/versioned
open-cluster-management.io/api/client/work/clientset/versioned/fake

View File

@@ -71,18 +71,18 @@ spec:
included in this manifestwork
type: object
properties:
Name:
group:
description: Group is the api group of the resources
in the workload that the strategy is applied
type: string
name:
description: Name is the names of the resources in the
workload that the strategy is applied
type: string
Namespace:
namespace:
description: Namespace is the namespaces of the resources
in the workload that the strategy is applied
type: string
group:
description: Group is the api group of the resources
in the workload that the strategy is applied
type: string
resource:
description: Resource is the resources in the workload
that the strategy is applied

View File

@@ -98,10 +98,10 @@ type OrphaningRule struct {
Resource string `json:"resource"`
// Namespace is the namespaces of the resources in the workload that the strategy is applied
// +optional
Namespace string `json:"Namespace"`
Namespace string `json:"namespace"`
// Name is the names of the resources in the workload that the strategy is applied
// +required
Name string `json:"Name"`
Name string `json:"name"`
}
// ManifestResourceMeta represents the group, version, kind, as well as the group, version, resource, name and namespace of a resoure.

View File

@@ -168,8 +168,8 @@ var map_OrphaningRule = map[string]string{
"": "OrphaningRule identifies a single resource included in this manifestwork",
"group": "Group is the api group of the resources in the workload that the strategy is applied",
"resource": "Resource is the resources in the workload that the strategy is applied",
"Namespace": "Namespace is the namespaces of the resources in the workload that the strategy is applied",
"Name": "Name is the names of the resources in the workload that the strategy is applied",
"namespace": "Namespace is the namespaces of the resources in the workload that the strategy is applied",
"name": "Name is the names of the resources in the workload that the strategy is applied",
}
func (OrphaningRule) SwaggerDoc() map[string]string {