refactor(nas): get node information only in losetup mode #1592
Conversation
k8s-ci-robot
added
kind/cleanup
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: iltyty The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
approved
iltyty
force-pushed
the
nas-losetup-cleanup
branch
from
e61918c to
19155b8
Compare
pkg/nas/internal/config.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| config.NodeName = nodeName |
There was a problem hiding this comment.
config.NodeName is also used as CSI NodeID, So I think we still need this when !config.EnableLosetup
There was a problem hiding this comment.
How about renaming sidecar mode's nodeName to podName, as I haven't find use case for nodeName in the nas driver.
There was a problem hiding this comment.
CI failure also caused by this issue:
I1226 18:57:46.163485 18655 main.go:109] "Received NotifyRegistrationStatus call" status="&RegistrationStatus{PluginRegistered:false,Error:RegisterPlugin error -- plugin registration failed with err: error adding CSI driver node info: driverNodeID must not be empty,}"
E1226 18:57:46.163520 18655 main.go:111] "Registration process failed with error, restarting registration container" err="RegisterPlugin error -- plugin registration failed with err: error adding CSI driver node info: driverNodeID must not be empty"
There was a problem hiding this comment.
How about renaming sidecar mode's nodeName to podName
I think we can still use KUBE_NODE_NAME env name, and let caller to determine what value is appropriate.
iltyty
force-pushed
the
nas-losetup-cleanup
branch
from
19155b8 to
aa9dad9
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
4 participants
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Get node information only in losetup mode, otherwise the nas driver will failed to start when no kubeconfig is provided.
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.: