⚠️ Fakeclient: Fix status handling #2259
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
the
size/L
alvaroaleman
force-pushed
the
fake-status
branch
2 times, most recently
from
ea6cd8c
to
927b57f
Compare
alvaroaleman
force-pushed
the
fake-status
branch
from
927b57f
to
9759b2e
Compare
alvaroaleman
changed the title
|
/assign @FillZpp |
alvaroaleman
force-pushed
the
fake-status
branch
2 times, most recently
from
f438bb8
to
388a58d
Compare
|
/retest |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/hold |
k8s-ci-robot
added
the
do-not-merge/hold
alvaroaleman
force-pushed
the
fake-status
branch
from
388a58d
to
d569207
Compare
k8s-ci-robot
removed
the
lgtm
Added that. /hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
This change fixes the status handling of the fake status client by
* Adding a new `WithStatusSubresource` option and pre-poluating it with
all in-tree resources that have a status subresource
* Making its `Update` and `Patch` not change the status of any such
resource
* Making its status client `Update` and `Patch` only change the status
for any such resources
* Making it error if `Status().{Update,Patch}` is called on a resource
that isn't configured to have the status subresource
Since this was completely broken before in that both non-status and
status Update/Patch would always update everything and the status
resources get pre-populated, this is a breaking change. Any test that
relied on the previous behavior would pass incorrectly though, as it
didn't match what the Kubernetes API does.
alvaroaleman
force-pushed
the
fake-status
branch
from
d569207
to
0bff6fc
Compare
aleskandro
added a commit
to aleskandro/machine-api-provider-azure
that referenced
this pull request
Jun 2, 2023
…esource set kubernetes-sigs/controller-runtime#2259 fixed the status handling of the fake client by adding a new WithStatusSubresource method to include the GVKs in a mock set used to distinguish whether an object being updated through the *fakeSubResourceClient.Patch method should be considered or throw errNotFound. Ref.: https://github.com/kubernetes-sigs/controller-runtime/pull/2259/files#diff-20ecedbf30721c01c33fb67d911da11c277e29990497a600d20cb0ec7215affdR387-R398
aleskandro
added a commit
to aleskandro/machine-api-provider-azure
that referenced
this pull request
Jun 5, 2023
…esource set kubernetes-sigs/controller-runtime#2259 fixed the status handling of the fake client by adding a new WithStatusSubresource method to include the GVKs in a mock set used to distinguish whether an object being updated through the *fakeSubResourceClient.Patch method should be considered or throw errNotFound. Ref.: https://github.com/kubernetes-sigs/controller-runtime/pull/2259/files#diff-20ecedbf30721c01c33fb67d911da11c277e29990497a600d20cb0ec7215affdR387-R398
7 tasks
lleshchi
added a commit
to lleshchi/hive
that referenced
this pull request
Aug 10, 2023
Rewrite unittests to accommodate changes in controller-runtime v0.15.0 and create a hive specific scheme. One such major change involves the behavior of the fake client deletionTimestamp. The fake client will panic if initialized with an object that has a DeletionTimestamp and no finalizer, and will silently ignore a DeletionTimestamp on an object in a Create request, matching the behavior of the kube-apiserver. Tests have been rewritten to work with this new behavior. kubernetes-sigs/controller-runtime#2316 This update also includes a requirement to register a fake client WithStatusSubresource in order to successfully update the Status subresource of an object. Due to this change, the helper function NewFakeClientBuilder() is added to pkg/test/fake to allow for easier setup of a fake client with the necessary sub-resources. kubernetes-sigs/controller-runtime#2259 Lastly, a new singleton scheme has been introduced to the codebase. The hive specific scheme is registered once, with all necessary packages, and used throughout the codebase, removing the need to specify unique schemes for each use case, and decreasing the overall amount of imports. Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
lleshchi
added a commit
to lleshchi/hive
that referenced
this pull request
Aug 14, 2023
Rewrite unittests to accommodate changes in controller-runtime v0.15.0 and create a hive specific scheme. One such major change involves the behavior of the fake client deletionTimestamp. The fake client will panic if initialized with an object that has a DeletionTimestamp and no finalizer, and will silently ignore a DeletionTimestamp on an object in a Create request, matching the behavior of the kube-apiserver. Tests have been rewritten to work with this new behavior. kubernetes-sigs/controller-runtime#2316 This update also includes a requirement to register a fake client WithStatusSubresource in order to successfully update the Status subresource of an object. Due to this change, the helper function NewFakeClientBuilder() is added to pkg/test/fake to allow for easier setup of a fake client with the necessary sub-resources. kubernetes-sigs/controller-runtime#2259 Lastly, a new singleton scheme has been introduced to the codebase. The hive specific scheme is registered once, with all necessary packages, and used throughout the codebase, removing the need to specify unique schemes for each use case, and decreasing the overall amount of imports. Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
lleshchi
added a commit
to lleshchi/hive
that referenced
this pull request
Aug 15, 2023
Rewrite unittests to accommodate changes in controller-runtime v0.15.0 and create a hive specific scheme. One such major change involves the behavior of the fake client deletionTimestamp. The fake client will panic if initialized with an object that has a DeletionTimestamp and no finalizer, and will silently ignore a DeletionTimestamp on an object in a Create request, matching the behavior of the kube-apiserver. Tests have been rewritten to work with this new behavior. kubernetes-sigs/controller-runtime#2316 This update also includes a requirement to register a fake client WithStatusSubresource in order to successfully update the Status subresource of an object. Due to this change, the helper function NewFakeClientBuilder() is added to pkg/test/fake to allow for easier setup of a fake client with the necessary sub-resources. kubernetes-sigs/controller-runtime#2259 Lastly, a new singleton scheme has been introduced to the codebase. The hive specific scheme is registered once, with all necessary packages, and used throughout the codebase, removing the need to specify unique schemes for each use case, and decreasing the overall amount of imports. Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
lleshchi
added a commit
to lleshchi/hive
that referenced
this pull request
Aug 15, 2023
Rewrite unittests to accommodate changes in controller-runtime v0.15.0 and create a hive specific scheme. One such major change involves the behavior of the fake client deletionTimestamp. The fake client will panic if initialized with an object that has a DeletionTimestamp and no finalizer, and will silently ignore a DeletionTimestamp on an object in a Create request, matching the behavior of the kube-apiserver. Tests have been rewritten to work with this new behavior. kubernetes-sigs/controller-runtime#2316 This update also includes a requirement to register a fake client WithStatusSubresource in order to successfully update the Status subresource of an object. Due to this change, the helper function NewFakeClientBuilder() is added to pkg/test/fake to allow for easier setup of a fake client with the necessary sub-resources. kubernetes-sigs/controller-runtime#2259 Lastly, a new singleton scheme has been introduced to the codebase. The hive specific scheme is registered once, with all necessary packages, and used throughout the codebase, removing the need to specify unique schemes for each use case, and decreasing the overall amount of imports. Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
lleshchi
added a commit
to lleshchi/hive
that referenced
this pull request
Aug 16, 2023
Rewrite unittests to accommodate changes in controller-runtime v0.15.0 and create a hive specific scheme. One such major change involves the behavior of the fake client deletionTimestamp. The fake client will panic if initialized with an object that has a DeletionTimestamp and no finalizer, and will silently ignore a DeletionTimestamp on an object in a Create request, matching the behavior of the kube-apiserver. Tests have been rewritten to work with this new behavior. kubernetes-sigs/controller-runtime#2316 This update also includes a requirement to register a fake client WithStatusSubresource in order to successfully update the Status subresource of an object. Due to this change, the helper function NewFakeClientBuilder() is added to pkg/test/fake to allow for easier setup of a fake client with the necessary sub-resources. kubernetes-sigs/controller-runtime#2259 Lastly, a new singleton scheme has been introduced to the codebase. The hive specific scheme is registered once, with all necessary packages, and used throughout the codebase, removing the need to specify unique schemes for each use case, and decreasing the overall amount of imports. Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
lleshchi
added a commit
to lleshchi/hive
that referenced
this pull request
Aug 16, 2023
Rewrite unittests to accommodate changes in controller-runtime v0.15.0 and create a hive specific scheme. One such major change involves the behavior of the fake client deletionTimestamp. The fake client will panic if initialized with an object that has a DeletionTimestamp and no finalizer, and will silently ignore a DeletionTimestamp on an object in a Create request, matching the behavior of the kube-apiserver. Tests have been rewritten to work with this new behavior. kubernetes-sigs/controller-runtime#2316 This update also includes a requirement to register a fake client WithStatusSubresource in order to successfully update the Status subresource of an object. Due to this change, the helper function NewFakeClientBuilder() is added to pkg/test/fake to allow for easier setup of a fake client with the necessary sub-resources. kubernetes-sigs/controller-runtime#2259 Lastly, a new singleton scheme has been introduced to the codebase. The hive specific scheme is registered once, with all necessary packages, and used throughout the codebase, removing the need to specify unique schemes for each use case, and decreasing the overall amount of imports. Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
Closed
rhmdnd
added a commit
to ComplianceAsCode/compliance-operator
that referenced
this pull request
Oct 25, 2023
The fake client we use in our unit tests was recently updated to be more inline with what the actual kubernetes API does: kubernetes-sigs/controller-runtime#2259 As a result, some of our tests broken because we were lumping Status and other updates into the same Update() request. This commit refactors those tests so that Status updates are implemented separately from the rest of the runtime objects. We need to do this before we can upgrade controller-runtime to 0.16.3.
1 task
rhmdnd
added a commit
to ComplianceAsCode/compliance-operator
that referenced
this pull request
Oct 26, 2023
The fake client we use in our unit tests was recently updated to be more inline with what the actual kubernetes API does: kubernetes-sigs/controller-runtime#2259 As a result, some of our tests broken because we were lumping Status and other updates into the same Update() request. This commit refactors those tests so that Status updates are implemented separately from the rest of the runtime objects. We need to do this before we can upgrade controller-runtime to 0.16.3.
yiannistri
added a commit
to weaveworks/weave-gitops-enterprise
that referenced
this pull request
Nov 6, 2023
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change fixes the status handling of the fake client by
WithStatusSubresourceoption and pre-poluating it withall in-tree resources that have a status subresource
UpdateandPatchnot change the status of any suchresource
UpdateandPatchonly change the statusfor any such resources
Since this was completely broken before in that both non-status and
status Update/Patch would always update everything and the status
resources get pre-populated, this is a breaking change. Any test that
relied on the previous behavior would pass incorrectly though, as it
didn't match what the Kubernetes API does.
Fixes #2230