-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
Improve event msg for PV controller when using external provisioner #43679
Conversation
Hi @xingzhou. 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 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. I understand the commands that are listed here. |
@k8s-bot ok to test |
@@ -1250,7 +1250,7 @@ func (ctrl *PersistentVolumeController) provisionClaimOperation(claimObj interfa | |||
// This means that an unknown provisioner is requested. Report an event | |||
// and wait for the external provisioner | |||
if storageClass != nil { | |||
msg := fmt.Sprintf("cannot find provisioner %q, expecting that a volume for the claim is provisioned either manually or via external software", storageClass.Provisioner) | |||
msg := fmt.Sprintf("waiting for external provisioner %q to provision a volume or creating a volume mannually for the claim", storageClass.Provisioner) |
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.
- typo
mannually
->manually
- in my (imperfect) English the sentence "waiting for external provisioner %q to provision a volume or creating a volume manually" means that Kubernetes is either "waiting for external provisioner" or "Kubernetes is creating a volume manually", which is not the case. Kubernetes is waiting for someone to create the volume, be it external provisioner or system admin.
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.
@jsafrane, how about we changing the message to:
“waiting for a volume to be created, either by external provisioner %q or manually created by system administrator”
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.
that sounds much better
/lgtm |
/lgtm cancel |
/assign @jsafrane |
Improve event msg for PV controller when using external provisioner
@jsafrane, have updated the msg, PTAL, thanks! |
@jsafrane, have updated the message, any comments for the change? |
/lgtm |
/release-note-none |
thanks for the PR! |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
Improve event msg for PV controller when using external provisioner
Which issue this PR fixes *:
Fixed part of #42121
Special notes for your reviewer:
@jsafrane, as many of our users are confused by the original message, can we fix the message first and then consider how to control the count of the events?