Conversation
imanenami
left a comment
There was a problem hiding this comment.
The tutorial looks good to me.
But one general suggestion: I think it would be neater and easier to follow if we put all the contents of TF files in the tutorial into sub-folders in terraform/examples and point the reader to these files. Also makes future updates easier.
We can also add a section for advanced TF deployments and instruct users on how to edit these examples for more complex deployments.
Co-authored-by: Iman Enami <44609233+imanenami@users.noreply.github.com> Signed-off-by: Vladimir Izmalkov <48120135+izmalk@users.noreply.github.com>
Combine main.tf code blocks to be standalone when possible. Hide big code blocks in expandable sections. Put the reference section into a dedicated page.
|
Addressed the comments and refactored the guide. I also made the The admin user section is still an addendum, which seems like the most intuitive approach here. I also moved the reference from the how-to guide into a dedicated page in the reference section. |
|
|
||
| ## Prerequisites | ||
|
|
||
| * A Juju controller bootstrapped on a **non-Kubernetes** cloud (see [Juju CLI deployment guide](how-to-deploy-anywhere) for setup instructions) |
There was a problem hiding this comment.
todo: Mark that the non-Kubernetes part is only for this repo. Link to the equivalent on kafka-k8s-operator
There was a problem hiding this comment.
As discussed on a call - we'll add the link later, when we add K8s version of this doc.
|
|
||
| <summary>Terraform configuration for production</summary> | ||
|
|
||
| Save the following as `main.tf` in a new working directory. The module is sourced directly from the [`terraform/` directory](https://github.com/canonical/kafka-operator/tree/main/terraform) in the Charmed Apache Kafka GitHub repository. |
There was a problem hiding this comment.
todo: I don't think we should encourage this. We have a terraform module available at git::https://github.com/canonical/kafka-bundle//terraform, and they should be encouraged to use that.
There was a problem hiding this comment.
As such, the contents here should be how they configure their .tfvars file with the various deployment configurations - split, colocated etc
There was a problem hiding this comment.
So the contents of the main.tf file they need would just be:
terraform {
required_providers {
juju = {
source = "juju/juju"
version = ">= 1.0.0"
}
}
}
provider "juju" {}
module "kafka" {
source = "git::https://github.com/canonical/kafka-bundle//terraform?ref=main"
broker = var.broker
connect = var.connect
...
}And then we'd share also a kafka.tfvars file:
broker = {
app_name = "kafka"
channel = "4/stable"
units = 3
...
}
connect = {
app_name = "connect"
# if they want it omitted
units = 0
...
}There was a problem hiding this comment.
Refactored into having one main.tf and two tfvars files, as we discussed. 263412d
|
|
||
| ```shell | ||
| terraform init | ||
| terraform plan -var "model_name=<model-name>" |
There was a problem hiding this comment.
todo: Similarly, I believe this -var ... can be part of the .tfvars file
| (reference-terraform)= | ||
| # Terraform module reference | ||
|
|
||
| Reference for the [Charmed Apache Kafka Terraform module](https://github.com/canonical/kafka-operator/tree/main/terraform), used with the [Juju Terraform provider](https://registry.terraform.io/providers/juju/juju/latest/docs). |
There was a problem hiding this comment.
todo: Here should be the bundle link, not just the Kafka app.
There was a problem hiding this comment.
Updated the whole reference according to kafka-bundle, rather than kafka-operator in b891e27
| ```{warning} | ||
| This is not recommended for production deployments. Apache Kafka brokers rely on the KRaft controllers to coordinate. If both services go down at the same time, the risk of cluster instability increases. | ||
| ``` |
There was a problem hiding this comment.
I think this is redundant, we already specify that this is a non-production setup in the first sentence.
There was a problem hiding this comment.
Good idea. Now that we have specific tfvars and have to explicitly select one of them, it kind of makes sense to drop unnecessary warning. Fixed in 0a1548d
| terraform apply -var-file="<profile>.tfvars" | ||
| ``` | ||
|
|
||
| Replace `<profile>` with either `production` or `testing`, depending on the desired deployment. |
There was a problem hiding this comment.
Maybe a link to configurations#profile could be useful, since we mention it several times. I also noticed we don't have a clear place where we define what profiles do exactly. A good TODO for a future reference doc.
There was a problem hiding this comment.
Yes, the existing profile documentation is not very comprehensive. We have planned working on expanding refernces and explanations next cycle. For now, I've added the link to the configurations#profile on charmhub.io here in 0a1548d
Description
Added a Terraform deployment guide.
Updated Juju CLI deployment guide to be a nested page isntead of higher level Landing page.
Updated references.
Checklist