Skip to content

Commit

Permalink
Merge pull request #10409 from k8s-infra-cherrypick-robot/cherry-pick…
Browse files Browse the repository at this point in the history
…-10408-to-release-1.7

[release-1.7] 🐛 SSA: recover gvk after scheme.Convert
  • Loading branch information
k8s-ci-robot authored Apr 9, 2024
2 parents a1dcccc + 2233bf3 commit c9136af
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
15 changes: 10 additions & 5 deletions internal/util/ssa/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ func Patch(ctx context.Context, c client.Client, fieldManager string, modified c
return err
}

gvk, err := apiutil.GVKForObject(modifiedUnstructured, c.Scheme())
if err != nil {
return errors.Wrapf(err, "failed to apply object: failed to get GroupVersionKind of modified object %s", klog.KObj(modifiedUnstructured))
}

var requestIdentifier string
if options.WithCachingProxy {
// Check if the request is cached.
Expand All @@ -88,15 +93,12 @@ func Patch(ctx context.Context, c client.Client, fieldManager string, modified c
if err := c.Scheme().Convert(options.Original, modified, ctx); err != nil {
return errors.Wrapf(err, "failed to write original into modified object")
}
// Recover gvk e.g. for logging.
modified.GetObjectKind().SetGroupVersionKind(gvk)
return nil
}
}

gvk, err := apiutil.GVKForObject(modifiedUnstructured, c.Scheme())
if err != nil {
return errors.Wrapf(err, "failed to apply object: failed to get GroupVersionKind of modified object %s", klog.KObj(modifiedUnstructured))
}

patchOptions := []client.PatchOption{
client.ForceOwnership,
client.FieldOwner(fieldManager),
Expand All @@ -110,6 +112,9 @@ func Patch(ctx context.Context, c client.Client, fieldManager string, modified c
return errors.Wrapf(err, "failed to write modified object")
}

// Recover gvk e.g. for logging.
modified.GetObjectKind().SetGroupVersionKind(gvk)

if options.WithCachingProxy {
// If the SSA call did not update the object, add the request to the cache.
if options.Original.GetResourceVersion() == modifiedUnstructured.GetResourceVersion() {
Expand Down
6 changes: 6 additions & 0 deletions internal/util/ssa/patch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ func TestPatch(t *testing.T) {
// 1. Create the object
createObject := initialObject.DeepCopy()
g.Expect(Patch(ctx, env.GetClient(), fieldManager, createObject)).To(Succeed())
// Verify that gvk is still set
g.Expect(createObject.GroupVersionKind()).To(Equal(initialObject.GroupVersionKind()))
// Note: We have to patch the status here to explicitly set these two status fields.
// If we don't do it the Machine defaulting webhook will try to set the two fields to false.
// For an unknown reason this will happen with the 2nd update call (3.) below and not before.
Expand Down Expand Up @@ -154,6 +156,8 @@ func TestPatch(t *testing.T) {
g.Expect(err).ToNot(HaveOccurred())
// Update the object
g.Expect(Patch(ctx, env.GetClient(), fieldManager, modifiedObject, WithCachingProxy{Cache: ssaCache, Original: originalObject})).To(Succeed())
// Verify that gvk is still set
g.Expect(modifiedObject.GroupVersionKind()).To(Equal(initialObject.GroupVersionKind()))
// Verify that request was not cached (as it changed the object)
g.Expect(ssaCache.Has(requestIdentifier)).To(BeFalse())

Expand All @@ -173,5 +177,7 @@ func TestPatch(t *testing.T) {
g.Expect(Patch(ctx, env.GetClient(), fieldManager, modifiedObject, WithCachingProxy{Cache: ssaCache, Original: originalObject})).To(Succeed())
// Verify that request was cached (as it did not change the object)
g.Expect(ssaCache.Has(requestIdentifier)).To(BeTrue())
// Verify that gvk is still set
g.Expect(modifiedObject.GroupVersionKind()).To(Equal(initialObject.GroupVersionKind()))
})
}

0 comments on commit c9136af

Please sign in to comment.