Skip to content
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

csi_attacher: improve attach/detach timeout message #108628

Merged
merged 1 commit into from
Mar 16, 2022

Conversation

RomanBednar
Copy link
Contributor

@RomanBednar RomanBednar commented Mar 10, 2022

What type of PR is this?

/kind bug

What this PR does / why we need it:

If we time out waiting for volume to be attached the message given
to user is not informative enough:

"Attach timeout for volume vol-123"

It would be better if we provide more information on what's going on
and even include name of the driver that's causing the problem, e.g.:

"Timed out waiting for external-attacher of ebs.csi.aws.com CSI driver to attach volume vol-123"

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Tested locally (local-up-cluster.sh):

#kubectl -n testdisk describe pod/efs-app

BEFORE PATCH:

Events:
 Type   Reason       		Age    	From           		Message
 ----   ------       		----   	----           		-------
 Normal  Scheduled      	14m	default-scheduler    	Successfully assigned testdisk/efs-app to *************
 Warning FailedAttachVolume 	17s	attachdetach-controller AttachVolume.Attach failed for volume “efs-pv” : Attach timeout for volume fs-0bd37fa1a498fc2c7
 Warning FailedMount     	84s	kubelet         	Unable to attach or mount volumes: unmounted volumes=[persistent-storage], unattached volumes=[persistent-storage kube-api-access-8fz5r]: timed out waiting for the condition

AFTER PATCH:

Events:
  Type     Reason              Age    	From                     Message
  ----     ------              ----   	----                     -------
  Normal   Scheduled           2m48s  	default-scheduler        Successfully assigned testdisk/efs-app to *************
  Warning  FailedAttachVolume  48s    	attachdetach-controller  AttachVolume.Attach failed for volume "efs-pv" : Timed out waiting for external-attacher of efs1.csi.aws.com CSI driver to attach volume fs-0bd37fa1a498fc2c7
  Warning  FailedMount         45s    	kubelet                  Unable to attach or mount volumes: unmounted volumes=[persistent-storage], unattached volumes=[persistent-storage kube-api-access-5c4sl]: timed out waiting for the condition

Does this PR introduce a user-facing change?

Improved logging when volume times out waiting for attach/detach.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 10, 2022
@k8s-ci-robot
Copy link
Contributor

@RomanBednar: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Mar 10, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @RomanBednar. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 10, 2022
@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 10, 2022
@chrishenzie
Copy link
Member

/ok-to-test

Thanks for improving this!

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 11, 2022
@jsafrane
Copy link
Member

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 11, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsafrane, RomanBednar

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 11, 2022
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 11, 2022
@RomanBednar
Copy link
Contributor Author

/test pull-kubernetes-e2e-gce-ubuntu-containerd

If we time out waiting for volume to be attached the message given
to user is not informative enough:

"Attach timeout for volume vol-123"

It would be better if we provide more information on what's going on
and even include name of the driver that's causing the problem, e.g.:

"timed out waiting for external-attacher of ebs.csi.aws.com CSI driver to attach volume vol-123"
@RomanBednar
Copy link
Contributor Author

/test pull-kubernetes-e2e-kind-ipv6

@jsafrane
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 16, 2022
@k8s-ci-robot k8s-ci-robot merged commit aa343fa into kubernetes:master Mar 16, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.24 milestone Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants