BYOVnet - Reduce duplication of ENV variables and change to centralus region #338
Conversation
👷 Deploy request for rp-cloud pending review.Visit the deploys page to approve it
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes update the Azure BYOVNet Redpanda cluster setup documentation and configuration files. The Azure region is changed from "eastus" to "centralus," and availability zones are updated accordingly. A comprehensive set of environment variables is introduced to parameterize resource names, IDs, and configuration values, replacing hardcoded strings and Terraform output parsing. All references in configuration files and JSON variable files are updated to use these environment variables. Instructions for deriving environment variables from Terraform outputs are removed, with the assumption that variables are set explicitly beforehand. No changes are made to exported or public code entities. Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
modules/get-started/pages/cluster-types/byoc/azure/vnet-azure.adoc (2)
128-138: Boolean written as string breaks Terraform
"create_resource_groups": "false"is a string, but the variable invars.condition.tfis of typebool. Terraform will complain with "cannot parse bool from string".- "create_resource_groups": "false", + "create_resource_groups": false,
418-420: Undefined variable – cluster JSON will render an empty value
${REDPANDA_CLUSTER_RESOURCE_GROUP_NAME}has never been exported (onlyREDPANDA_RESOURCE_GROUP_NAMEexists).
At render-time this expands to an empty string, producing invalid JSON and a 400 from the Cloud API.- "redpanda_resource_group" : { "name": "${REDPANDA_CLUSTER_RESOURCE_GROUP_NAME}" }, + "redpanda_resource_group" : { "name": "${REDPANDA_RESOURCE_GROUP_NAME}" },
🧹 Nitpick comments (1)
modules/get-started/pages/cluster-types/byoc/azure/vnet-azure.adoc (1)
99-103: Environment-variable bloat – consider a single prefix mapSeventeen separate
*_RESOURCE_GROUP_NAMEexports all point to the samedyu-redpanda-rgstring.
This is hard to maintain and easy to get out of sync. A single variable (e.g.,export REDPANDA_RG=dyu-redpanda-rg) or a JSON/yaml file consumed by both Terraform and Bash would DRY the docs considerably.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/get-started/pages/cluster-types/byoc/azure/vnet-azure.adoc(7 hunks)
🔇 Additional comments (1)
modules/get-started/pages/cluster-types/byoc/azure/vnet-azure.adoc (1)
50-50: Confirm Central US is the intended regionThe examples now hard-code
AZURE_REGION=centralus.
✓ Make sure the target subscription actually has capacity for DSv5 (or the required SKU) in Central US; otherwise the whole flow will error out atterraform apply.
✓ Consider adding a short note in Prerequisites so users double-check quota/availability first.
2 participants
Description
Page previews
Checks