kubernetes / mount-utils

Package mount defines an interface to mounting filesystems.
Apache License 2.0
57 stars 30 forks source link

Adjust log level for removePathIfNotMountPoint #10

Closed gmarks-ntap closed 1 year ago

gmarks-ntap commented 1 year ago

While looking at logs for a CSI driver we noticed we were seeing warning log messages coming from the following snippet of code from the removePathIfNotMountPoint() function from the mount_helper_common.go file.

if notMnt {
        klog.Warningf("Warning: %q is not a mountpoint, deleting", mountPath)
        return notMnt, os.Remove(mountPath)
    }

Looking at the comment for the removePathIfNotMountPoint() function, and from the name of the function, it appears that removal of a path is an intended result of the function.

// removePathIfNotMountPoint verifies if given mountPath is a mount point if not it attempts
// to remove the directory. Returns true and nil if directory was not a mount point and removed.
func removePathIfNotMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool) (bool, error) {

Given that the path removal is expected from this function it seems a little bit odd that a warning level log message would be produced in conjunction with removing the path element. It seems to me that a higher level V style Info message would be better fitting for this log message. In that scenario the log message would be filtered out at normal logging levels but would still be available for scenarios where additional info/debug level detail was desired. It may also be less confusing for someone running across these log messages to see an Info message rather than a Warning message about the path removal since the removal is expected there and not necessarily an error situation.

Does this make enough sense to change this klog.Warningf() usage to something more like a klog.V(4).Infof() usage? The exact V level could probably be debated, but 4 or 5 seem reasonable to me.

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

k8s-ci-robot commented 1 year ago

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to [this](https://github.com/kubernetes/mount-utils/issues/10#issuecomment-1528179978): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue with `/reopen` >- Mark this issue as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close not-planned > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.