-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Enable parameterized kubelet mount path during node-agent installation #9074
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
Enable parameterized kubelet mount path during node-agent installation #9074
Conversation
Signed-off-by: longyuxiang <[email protected]>
Signed-off-by: longyuxiang <[email protected]>
…lero into nodeagent-kubelet-root-dir
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9074 +/- ##
==========================================
- Coverage 60.06% 60.05% -0.01%
==========================================
Files 379 379
Lines 43483 43496 +13
==========================================
+ Hits 26116 26122 +6
- Misses 15801 15807 +6
- Partials 1566 1567 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kaovilai
left a comment
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.
You may want to update docs/ to explain this.. but lgtm.
Signed-off-by: longyuxiang <[email protected]>
I've made the requested documentation updates and pushed a new commit with these changes to this PR. |
a2a1a9b to
182d72f
Compare
182d72f to
805237a
Compare
…lero into nodeagent-kubelet-root-dir
go.mod
Outdated
| github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.27.0 // indirect | ||
| github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.51.0 // indirect | ||
| github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.51.0 // indirect | ||
| github.com/Microsoft/go-winio v0.6.2 // indirect |
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.
Many of the new dependencies are not related to the current change, please double check and add the necessary changes only.
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.
~/git/velero nodeagent-kubelet-root-dir
❯ go mod why github.com/Microsoft/go-winio
# github.com/Microsoft/go-winio
github.com/vmware-tanzu/velero/pkg/install
k8s.io/kubernetes/pkg/kubelet/config
k8s.io/kubernetes/pkg/kubelet/types
k8s.io/cri-client/pkg/logs
k8s.io/cri-client/pkg
k8s.io/cri-client/pkg/util
github.com/Microsoft/go-winio
Came in via c49517b which imports k8s.io/kubernetes/pkg/kubelet/config to replace redeifning
- hostPodsVolumePath := filepath.Join(c.kubeletRootDir, "pods")
- hostPluginsVolumePath := filepath.Join(c.kubeletRootDir, "plugins")
+ hostPodsVolumePath := filepath.Join(c.kubeletRootDir, kubeletconfig.DefaultKubeletPodsDirName)
+ hostPluginsVolumePath := filepath.Join(c.kubeletRootDir, kubeletconfig.DefaultKubeletPluginsDirName)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.
which IIUC is @blackpiglet request
There appears to be no alternative k8s.io package that provides the same symbol. https://pkg.go.dev/search?q=DefaultKubeletPodsDirName&ref=opensearch
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.
if we are not okay with the added imports, we might have to settle with the prior change.
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.
I think that is because kubelet is platform dependent.
My opinion is we always keep the code platform independent unless really necessary, so in comparison, copying the default rootDir strings to Velero is acceptable.
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.
Maybe revert to ths commit then 2d1a8bc
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.
79259f9 to
2d1a8bc
Compare
vmware-tanzu#9074) Enable parameterized kubelet mount path during node-agent installation Signed-off-by: longyuxiang <[email protected]>
Enables the parameterization of the kubelet mount path during node-agent installation
This PR enables the parameterization of the kubelet mount path during node-agent installation, allowing users to specify custom mount paths through configuration parameters. This addresses scenarios where different Kubernetes environments may use non-standard kubelet directories.
--kubelet-root-dirduring node-agent installation.Does your change fix a particular issue?
Fixes #(issue)
Please indicate you've done the following:
make new-changelog) or comment/kind changelog-not-requiredon this PR.site/content/docs/main.