Display topic count 0 instead of error when calling kafka cluster describe on a provisioning cluster#1471
Merged
Steven Gagniere (sgagniere) merged 4 commits intomainfrom Oct 20, 2022
Merged
Conversation
|
In general, should we be calling any Kafka REST endpoints while a cluster is provisioning? Maybe we should integrate this into our generic Kafka REST logic instead of just this function. |
MuweiHe
reviewed
Oct 20, 2022
MuweiHe
reviewed
Oct 20, 2022
| return errors.CatchKafkaNotFoundError(err, lkc, httpResp) | ||
| } | ||
| if cluster.Status.Phase == "PROVISIONING" { | ||
| return errors.Errorf(errors.KafkaRestProvisioningErrorMsg, lkc) |
Contributor
There was a problem hiding this comment.
What if the cluster has failed. What will the behavior be like/should we handle that too?
Member
Author
There was a problem hiding this comment.
Unsure. Does a cluster that failed to provision still have endpoints? If not, then it will probably generate the same errors that happen when a cluster is provisioning. If so, then I don't know what kind of error the backend produces.
Contributor
There was a problem hiding this comment.
Shipping the fix today would be priority. let's keep an eye on this if it ever happens..
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
What
Another short PR.
Since
kafka cluster describenow shows the number of topics for the cluster, it will display an error before the cluster is provisioned. I've modified thegetTopicCountForKafkaClustermethod to return0, nilif the status isPROVISIONING. For status codesUP,EXPANDING, andSHRINKING, calling kafka REST is fine.As far as I can tell, the only other status code is
FAILED, but I don't think that needs special handling.I've also added a new error message related to calling kafka REST while the cluster is provisioning; this is an error returned by
GetKafkaREST.Since commands in
kafka topic,kafka aclandksql cluster configure-aclsdon't currently handle errors so that backup code can run, I've duplicated the provisioning check code inside oftopicandacl. This should be temporary, and removed pending removal of the backups. I didn't add it toconfigure-aclsbecause there are enough checks before it gets to that point that should stop it.References
#1470
https://confluent.slack.com/archives/C03NF1Z3069/p1657138258955639
Test & Review
Tested w/ some dedicated clusters.
Ran all tests locally.