🐛 ssa: fix flaky test TestPatch/Test patch with Machine

[chrischdi]

During the test at the additional Patch calls the defaulting webhook did always set the .spec.infrastructureRef.namespace field.

This caused flaky behaviour because there was a chance for the resourceVersion to getting bumped with the only change being the timestamp in managedFields.

What this PR does / why we need it:

This flake was reproducible by either the following command:

go test -race -count 120 -failfast -shuffle on -timeout 30s -run '^TestPatch$' sigs.k8s.io/cluster-api/internal/util/ssa

Or by adding a time.Sleep(time.Second) at:

cluster-api/internal/util/ssa/patch_test.go

Line 157 in ec92803

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #8478

Area example:
/area testing

[chrischdi]

/cherry-pick release-1.6

[k8s-infra-cherrypick-robot]

@chrischdi: once the present PR merges, I will cherry-pick it on top of release-1.6 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[chrischdi]

/cherry-pick release-1.5

[k8s-infra-cherrypick-robot]

@chrischdi: once the present PR merges, I will cherry-pick it on top of release-1.5 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[chrischdi]

Note: this flake also exists on the still getting tested release-1.4 branch. Should we also cherry-pick to release-1.4 while this branch is already out of support?

[fabriziopandini]

Nice catch!
/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [fabriziopandini]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: efb1f2e8b1e0b511cd92e424e5374b19f55e2e45

[k8s-infra-cherrypick-robot]

@chrischdi: new pull request created: #9915

In response to this:

/cherry-pick release-1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-infra-cherrypick-robot]

@chrischdi: new pull request created: #9916

In response to this:

/cherry-pick release-1.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[chrischdi]

/cherry-pick release-1.4

[k8s-infra-cherrypick-robot]

@chrischdi: new pull request created: #9917

In response to this:

/cherry-pick release-1.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[sbueringer]

@chrischdi Nice catch! So the test failed when we hit the case where the timestamp was >1s higher than initially? Otherwise there was no diff?

Thinking about the practical implications. Does this mean we are continuously updating objects as soon as a single field is not set by us (the client) and defaulted by a webhook instead? (i.e. the cache doesn't work in this case)

[chrischdi]

@chrischdi Nice catch! So the test failed when we hit the case where the timestamp was >1s higher than initially? Otherwise there was no diff?

Thinking about the practical implications. Does this mean we are continuously updating objects as soon as a single field is not set by us (the client) and defaulted by a webhook instead? (i.e. the cache doesn't work in this case)

Yes that was the case in the test. Don't know if this also gets hit in a real scenario 🤔 maybe worth to try it