Skip to content

Commit 8265c39

Browse files
kcp-ci-botkcp-ci-bot
authored
Merge pull request #3648 from olamilekan000/persist-original-rv-in-cache-server
add original rv and uid to object annotation
2 parents d2e3d9e + a00e687 commit 8265c39

File tree

6 files changed

+97
-16
lines changed

6 files changed

+97
-16
lines changed

‎pkg/reconciler/cache/cachedresources/replication/replication_reconcile.go‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,14 @@ import (
4545
)
4646

4747
const (
48-
LabelKeyObjectSchema = "cache.kcp.io/object-schema"
49-
LabelKeyObjectGroup = "cache.kcp.io/object-group"
50-
LabelKeyObjectVersion = "cache.kcp.io/object-version"
51-
LabelKeyObjectResource = "cache.kcp.io/object-resource"
52-
LabelKeyObjectOriginalName = "cache.kcp.io/object-original-name"
53-
LabelKeyObjectOriginalNamespace = "cache.kcp.io/object-original-namespace"
48+
LabelKeyObjectSchema = "cache.kcp.io/object-schema"
49+
LabelKeyObjectGroup = "cache.kcp.io/object-group"
50+
LabelKeyObjectVersion = "cache.kcp.io/object-version"
51+
LabelKeyObjectResource = "cache.kcp.io/object-resource"
52+
LabelKeyObjectOriginalName = "cache.kcp.io/object-original-name"
53+
LabelKeyObjectOriginalNamespace = "cache.kcp.io/object-original-namespace"
54+
AnnotationKeyOriginalResourceVersion = "cache.kcp.io/original-resource-version"
55+
AnnotationKeyOriginalResourceUID = "cache.kcp.io/original-resource-UID"
5456
)
5557

5658
func GenCachedObjectName(gvr schema.GroupVersionResource, namespace, name string) string {
@@ -306,14 +308,18 @@ func (r *replicationReconciler) reconcile(ctx context.Context, key string) error
306308

307309
// local exists, global doesn't. Create in cache.
308310
if !globalExists {
309-
// TODO: in the future the original RV will have to be stored in an annotation (?)
310-
// so that the clients that need to modify the original/local object can do it
311+
originalRV := localCopy.GetResourceVersion()
312+
originalUID := localCopy.GetUID()
313+
311314
localCopy.SetResourceVersion("")
312315
annotations := localCopy.GetAnnotations()
313316
if annotations == nil {
314317
annotations = map[string]string{}
315318
}
316319
annotations[genericrequest.ShardAnnotationKey] = r.shardName
320+
annotations[AnnotationKeyOriginalResourceVersion] = originalRV
321+
annotations[AnnotationKeyOriginalResourceUID] = string(originalUID)
322+
317323
localCopy.SetAnnotations(annotations)
318324

319325
logger.V(2).Info("Creating object in global cache")

‎pkg/reconciler/cache/cachedresources/replication/replication_reconcile_unstructured.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,22 @@ func ensureMeta(cacheObject *unstructured.Unstructured, localObject *unstructure
7474
}
7575
}()
7676
}
77-
// TODO: in the future the original RV will be stored in an annotation
77+
if originalRV, hasOriginalRV := cacheObjAnnotations[AnnotationKeyOriginalResourceVersion]; hasOriginalRV {
78+
unstructured.RemoveNestedField(cacheObjAnnotations, AnnotationKeyOriginalResourceVersion)
79+
defer func() {
80+
if err == nil {
81+
err = unstructured.SetNestedField(cacheObject.Object, originalRV, "metadata", "annotations", AnnotationKeyOriginalResourceVersion)
82+
}
83+
}()
84+
}
85+
if originalUID, hasOriginalUID := cacheObjAnnotations[AnnotationKeyOriginalResourceUID]; hasOriginalUID {
86+
unstructured.RemoveNestedField(cacheObjAnnotations, AnnotationKeyOriginalResourceUID)
87+
defer func() {
88+
if err == nil {
89+
err = unstructured.SetNestedField(cacheObject.Object, originalUID, "metadata", "annotations", AnnotationKeyOriginalResourceUID)
90+
}
91+
}()
92+
}
7893
}
7994

8095
// before we can compare with the local object we need to

‎pkg/reconciler/cache/replication/replication_reconcile.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ import (
3333
"github.com/kcp-dev/logicalcluster/v3"
3434
)
3535

36+
const (
37+
AnnotationKeyOriginalResourceVersion = "cache.kcp.io/original-resource-version"
38+
AnnotationKeyOriginalResourceUID = "cache.kcp.io/original-resource-UID"
39+
)
40+
3641
func (c *controller) reconcile(ctx context.Context, gvrKey string) error {
3742
// split apart the gvr from the key
3843
keyParts := strings.Split(gvrKey, "::")
@@ -165,14 +170,18 @@ func (r *reconciler) reconcile(ctx context.Context, key string) error {
165170

166171
// local exists, global doesn't. Create in cache.
167172
if !globalExists {
168-
// TODO: in the future the original RV will have to be stored in an annotation (?)
169-
// so that the clients that need to modify the original/local object can do it
173+
originalRV := localCopy.GetResourceVersion()
174+
originalUID := localCopy.GetUID()
175+
170176
localCopy.SetResourceVersion("")
171177
annotations := localCopy.GetAnnotations()
172178
if annotations == nil {
173179
annotations = map[string]string{}
174180
}
175181
annotations[genericrequest.ShardAnnotationKey] = r.shardName
182+
annotations[AnnotationKeyOriginalResourceVersion] = originalRV
183+
annotations[AnnotationKeyOriginalResourceUID] = string(originalUID)
184+
176185
localCopy.SetAnnotations(annotations)
177186

178187
logger.V(2).Info("Creating object in global cache")

‎pkg/reconciler/cache/replication/replication_reconcile_test.go‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,17 @@ func TestReconcile(t *testing.T) {
7878
}
7979
return nil, errors.NewNotFound(gr, name)
8080
},
81-
getGlobalCopy: getCopyNotFoundFunc,
82-
key: "root|zoo/dumbo",
83-
expectedCreate: WithShardName(WithoutResourceVersion(elephant.DeepCopy()), "root"),
81+
getGlobalCopy: getCopyNotFoundFunc,
82+
key: "root|zoo/dumbo",
83+
expectedCreate: WithShardName(
84+
WithoutResourceVersion(
85+
WithOriginalResourceVersion(
86+
elephant.DeepCopy(),
87+
elephant.GetResourceVersion(),
88+
string(elephant.GetUID()),
89+
),
90+
),
91+
"root"),
8492
},
8593
{
8694
name: "case 2: cached object is removed when local object was removed",
@@ -242,3 +250,17 @@ func WithChange(u *unstructured.Unstructured, path []string, value interface{})
242250
func getCopyNotFoundFunc(cluster logicalcluster.Name, namespace, name string) (*unstructured.Unstructured, error) {
243251
return nil, errors.NewNotFound(schema.GroupResource{Group: "example.com", Resource: "elephants"}, name)
244252
}
253+
254+
func WithOriginalResourceVersion(u *unstructured.Unstructured, originalRV, originalUID string) *unstructured.Unstructured {
255+
ann := u.GetAnnotations()
256+
if ann == nil {
257+
ann = map[string]string{}
258+
}
259+
260+
ann[AnnotationKeyOriginalResourceVersion] = originalRV
261+
ann[AnnotationKeyOriginalResourceUID] = originalUID
262+
263+
u.SetAnnotations(ann)
264+
265+
return u
266+
}

‎pkg/reconciler/cache/replication/replication_reconcile_unstructured.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,22 @@ func ensureMeta(cacheObject *unstructured.Unstructured, localObject *unstructure
8282
}
8383
}()
8484
}
85-
// TODO: in the future the original RV will be stored in an annotation
85+
if originalRV, hasOriginalRV := cacheObjAnnotations[AnnotationKeyOriginalResourceVersion]; hasOriginalRV {
86+
unstructured.RemoveNestedField(cacheObjAnnotations, AnnotationKeyOriginalResourceVersion)
87+
defer func() {
88+
if err == nil {
89+
err = unstructured.SetNestedField(cacheObject.Object, originalRV, "metadata", "annotations", AnnotationKeyOriginalResourceVersion)
90+
}
91+
}()
92+
}
93+
if originalUID, hasOriginalUID := cacheObjAnnotations[AnnotationKeyOriginalResourceUID]; hasOriginalUID {
94+
unstructured.RemoveNestedField(cacheObjAnnotations, AnnotationKeyOriginalResourceUID)
95+
defer func() {
96+
if err == nil {
97+
err = unstructured.SetNestedField(cacheObject.Object, originalUID, "metadata", "annotations", AnnotationKeyOriginalResourceUID)
98+
}
99+
}()
100+
}
86101
}
87102

88103
// before we can compare with the local object we need to

‎test/e2e/reconciler/cache/replication_test.go‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import (
4545

4646
cacheclient "github.com/kcp-dev/kcp/pkg/cache/client"
4747
"github.com/kcp-dev/kcp/pkg/cache/client/shard"
48+
"github.com/kcp-dev/kcp/pkg/reconciler/cache/cachedresources/replication"
4849
apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
4950
apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2"
5051
"github.com/kcp-dev/kcp/sdk/apis/core"
@@ -688,14 +689,25 @@ func (b *replicateResourceScenario) verifyResourceReplicationHelper(ctx context.
688689
}
689690
return false, err.Error()
690691
}
691-
t.Logf("Compare if both the original and replicated resources (%s %s/%s) are the same except %s annotation and ResourceVersion", b.gvr, cluster, b.resourceName, genericapirequest.ShardAnnotationKey)
692+
t.Logf(
693+
"Compare if both the original and replicated resources (%s %s/%s) are the same except %s annotation, %s annotation, %s annotation, and ResourceVersion",
694+
b.gvr, cluster, b.resourceName, genericapirequest.ShardAnnotationKey,
695+
replication.AnnotationKeyOriginalResourceVersion,
696+
replication.AnnotationKeyOriginalResourceUID,
697+
)
692698
cachedResourceMeta, err := meta.Accessor(cachedResource)
693699
if err != nil {
694700
return false, err.Error()
695701
}
696702
if _, found := cachedResourceMeta.GetAnnotations()[genericapirequest.ShardAnnotationKey]; !found {
697703
t.Fatalf("replicated %s root|%s/%s, doesn't have %s annotation", b.gvr, cluster, cachedResourceMeta.GetName(), genericapirequest.ShardAnnotationKey)
698704
}
705+
if _, found := cachedResourceMeta.GetAnnotations()[replication.AnnotationKeyOriginalResourceVersion]; !found {
706+
t.Fatalf("replicated %s root|%s/%s, doesn't have %s annotation", b.gvr, cluster, cachedResourceMeta.GetName(), replication.AnnotationKeyOriginalResourceVersion)
707+
}
708+
if _, found := cachedResourceMeta.GetAnnotations()[replication.AnnotationKeyOriginalResourceUID]; !found {
709+
t.Fatalf("replicated %s root|%s/%s, doesn't have %s annotation", b.gvr, cluster, cachedResourceMeta.GetName(), replication.AnnotationKeyOriginalResourceUID)
710+
}
699711
unstructured.RemoveNestedField(originalResource.Object, "metadata", "resourceVersion")
700712
unstructured.RemoveNestedField(cachedResource.Object, "metadata", "resourceVersion")
701713

@@ -708,6 +720,8 @@ func (b *replicateResourceScenario) verifyResourceReplicationHelper(ctx context.
708720
}
709721

710722
unstructured.RemoveNestedField(cachedResource.Object, "metadata", "annotations", genericapirequest.ShardAnnotationKey)
723+
unstructured.RemoveNestedField(cachedResource.Object, "metadata", "annotations", replication.AnnotationKeyOriginalResourceVersion)
724+
unstructured.RemoveNestedField(cachedResource.Object, "metadata", "annotations", replication.AnnotationKeyOriginalResourceUID)
711725
if cachedStatus, ok := cachedResource.Object["status"]; ok && cachedStatus == nil || (cachedStatus != nil && len(cachedStatus.(map[string]interface{})) == 0) {
712726
// TODO: worth investigating:
713727
// for some reason cached resources have an empty status set whereas the original resources don't

0 commit comments

Comments
 (0)