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
Delaying kubeclient and csi client injection into CSI plugin #68633
Conversation
/priority critical-urgent |
/retest |
/lgtm |
This is good for a short term fix. Longer term volume plugin initialization code should fail dependencies are nil. And the code triggering volume plugin initialization should determine if volume plugins are required and skip volume plugin initialization if volume plugins are not needed. |
The PR changed the interface of NodeInfoManager, causing the unit test failures. Looking into it. |
da20769
to
0c8f4be
Compare
/lgtm |
0c8f4be
to
0519bfd
Compare
/kind bug |
/test pull-kubernetes-kubemark-e2e-gce-big |
pkg/volume/csi/csi_plugin.go
Outdated
p.csiDriverInformer = factory.Csi().V1alpha1().CSIDrivers() | ||
p.csiDriverLister = p.csiDriverInformer.Lister() | ||
go factory.Start(wait.NeverStop) | ||
} |
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.
log something if client is nil?
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.
Watching for this to merge and will pull into release-1.12 via branchff once it has.
0519bfd
to
e169485
Compare
e169485
to
a8e282e
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: saad-ali, verult 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 |
This brakes kubelet in stand-alone mode; all gke tests are broken. |
AFAIK, this PR fixed GKE tests which have been running without issue since it was merged: https://k8s-testgrid.appspot.com/google-gke#gci-gke Following up with @immutableT offline to clarify. |
Verified that @immutableT was running an older version, and with this fix the issue is resolved. |
What this PR does / why we need it:
Fixes #68509
Note for storage reviewers: the original flag gate check around CSI client initialization in
Init()
is covered inside nodeInfoManager calls and informer initialization inInit()
.Release note:
/sig storage
/sig gcp
/assign @msau42 @saad-ali @AishSundar