-
Notifications
You must be signed in to change notification settings - Fork 39.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor mount fakes #81423
Refactor mount fakes #81423
Conversation
fc7ff43
to
922f57c
Compare
ef76cb4
to
6e04067
Compare
1af5f48
to
ecfd6e2
Compare
OMG. Just realized, thanks to this test result that moving to a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind addressing the govet failures that are causing pull-kubernetes-verify
to fail.
/test pull-kubernetes-bazel-test
Since this is such a big diff, I imagine we'll rely pretty heavily on CI for validation, so going to wait for that to be green before reviewing.
Also, to confirm - this pr changes no functionality correct? It just changes the modules of certain functionality?
ecfd6e2
to
b22e53c
Compare
@kubernetes/sig-storage-pr-reviews |
yep, of course. Should be done now.
That's the idea. The only real change is that in order for the |
f1a04c6
to
2f74456
Compare
/retest |
3 similar comments
/retest |
/retest |
/retest |
/test pull-kubernetes-e2e-gce-storage-slow |
/retest |
1 similar comment
/retest |
/lgtm |
/assign @Random-Liu @derekwaynecarr @tallclair |
/milestone v1.17 |
the kubelet changes all look like straight forward refactors. thanks! /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: codenrhoden, derekwaynecarr, jsafrane, msau42 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 |
/retest Review the full test history for this PR. Silence the bot with an |
This patch moves fake.go to mount_fake.go, and follows to principle of always returning a discrete type rather than an Interface. All callers of "FakeMounter" are changed to instead use "NewFakeMounter()". The FakeMounter "Log" struct member is changed to not be exported, and instead only access through a new "GetLog()" method.
2f74456
to
1fd8921
Compare
/lgtm |
Rebased after a new merge conflict in the same PR batch. Needed to add this line to use the new struct member added in a different PR: https://github.com/kubernetes/kubernetes/pull/81423/files#diff-b962320d780ac74dffce9da68eda4f4cR616 |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
As part of the review for kubernetes/utils#100, it was requested to put all fakes in a
testing
subpackage.After discussion, a different package structures was agreed on with @saad-ali and @thockin, see #81423 (review).
This patch moves fake.go to mount_fake.go, and follows to principle of
always returning a discrete type rather than an Interface. All callers
of "FakeMounter" are changed to instead use "NewFakeMounter()". The
FakeMounter "Log" struct member is changed to not be exported, and
instead only access through a new "GetLog()" method.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: