-
Notifications
You must be signed in to change notification settings - Fork 133
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
Bumping csi-lib-utils to 0.9.0 #129
Conversation
cmd/csi-resizer/main.go
Outdated
// Start HTTP server for metrics + leader election healthz | ||
go func() { | ||
klog.Infof("ServeMux listening at %q", addr) | ||
err := http.ListenAndServe(addr, mux) |
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.
do we need to check addr empty?
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.
thanks for the catch!
pkg/resizer/csi_resizer.go
Outdated
metricsAddress, metricsPath string) (Resizer, error) { | ||
driverName, err := getDriverName(csiClient, timeout) | ||
if err != nil { | ||
return nil, fmt.Errorf("get driver name failed: %v", err) | ||
} | ||
|
||
klog.V(2).Infof("CSI driver name: %q", driverName) | ||
metricsManager.SetDriverName(driverName) | ||
metricsManager.StartMetricsEndpoint(metricsAddress, metricsPath) | ||
if metricsAddress != "" { |
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.
is it possible to move this outside so that it's similar to how csi-attacher does it?
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 added the refactor as a separate commit. WDYT?
71e12cc
to
a110795
Compare
a110795
to
7dfc7b0
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: msau42, 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 |
/test pull-kubernetes-csi-external-resizer-1-17-on-kubernetes-1-17 |
7bc70e5 Merge pull request kubernetes-csi#129 from pohly/squash-documentation e0b02e7 README.md: document usage of --squash 316cb95 Merge pull request kubernetes-csi#132 from yiyang5055/bugfix/boilerplate 26e2ab1 fix: default boilerplate path 1add8c1 Merge pull request kubernetes-csi#133 from pohly/kubernetes-1.20-tag 3e811d6 prow.sh: fix "on-master" prow jobs git-subtree-dir: release-tools git-subtree-split: 7bc70e5264a5ce5f47780bdbc6c7b7f4e79243fa
…ncy-openshift-4.11-ose-csi-external-resizer Updating ose-csi-external-resizer images to be consistent with ART
What type of PR is this?
/kind feature
Does this PR introduce a user-facing change?:
/assign @msau42