Skip to content

Resolve issue causing missing endpoints in cluster config#1470

Merged
Steven Gagniere (sgagniere) merged 3 commits intomainfrom
kafka-rest-provisioning
Oct 18, 2022
Merged

Resolve issue causing missing endpoints in cluster config#1470
Steven Gagniere (sgagniere) merged 3 commits intomainfrom
kafka-rest-provisioning

Conversation

@sgagniere
Copy link
Member

Checklist

  1. [CRUCIAL] Is the change for CP or CCloud functionalities that are already live in prod?
    • yes: ok

What

Calling FindKafkaCluster in dynamic_context.go before the cluster has finished provisioning causes the cluster info to be saved in config.json with bootstrap_servers equal to "" and no api_endpoint or rest_endpoint. As the config is only updated after at least one week since the timestamp saved in config.json, this prevents use of the cluster unless the timestamp is manually changed to trick the CLI into thinking it should be updated.

Found by calling kafka cluster describe on a provisioning cluster; this command now uses kafka REST for one of the description fields. Other commands using kafka REST also create this issue, e.g. kafka topic commands.

This PR makes a single change: if the config is not nil and Bootstrap is the empty string, then refresh the config.

References

n/a

Test & Review

Tested by calling kafka cluster describe on a provisioning cluster, and then again after it finished provisioning to check that it now prints the description as expected.

Open questions / Follow ups

Follow ups:

  • Modify kafka cluster describe to not check for topics while the status is PROVISIONING and instead print an empty value. This command still errors out at the moment.
  • Remove the backup code and/or improve the error messaging when commands using kafka REST are called during provisioning. Currently, the CLI prints an arcane backend error about empty schemas.

@sgagniere Steven Gagniere (sgagniere) requested a review from a team as a code owner October 18, 2022 03:10
Copy link
Contributor

@DABH David Hyde (DABH) left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 once CI passes!

@sgagniere Steven Gagniere (sgagniere) marked this pull request as draft October 18, 2022 03:15
@sgagniere Steven Gagniere (sgagniere) marked this pull request as ready for review October 18, 2022 18:54
KafkaClusterContext: &v1.KafkaClusterContext{
KafkaClusterConfigs: map[string]*v1.KafkaClusterConfig{
"lkc-123456": {LastUpdate: update},
"lkc-123456": {LastUpdate: update, Bootstrap: "test"},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want to use a more realistic value here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this new version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants