fix azure deployments so they can have a dash in project name #2423
Conversation
|
TODO: At a minimum I would need to do a migration of the nebari config file for Azure deployments during |
Adam-D-Lewis
changed the title
| # storage account must be globally unique | ||
| storage_account_name=f"{nebari_config.escaped_project_name}{nebari_config.namespace}{nebari_config.azure.storage_account_postfix}", | ||
| container_name=f"{nebari_config.escaped_project_name}-{nebari_config.namespace}-state", | ||
| # DELETEME: This is where we look up the storage account |
There was a problem hiding this comment.
I'll delete this comment before merge.
| @@ -4,9 +4,9 @@ resource "azurerm_resource_group" "terraform-state-resource-group" { | |||
| tags = var.tags | |||
| } | |||
|
|
|||
| # DELETEME: This is where we create the storage account | |||
There was a problem hiding this comment.
I'll delete this comment before merge.
| @@ -70,12 +70,9 @@ def is_version_accepted(cls, v): | |||
|
|
|||
| @property | |||
| def escaped_project_name(self): | |||
| """Escaped project-name know to be compatible with all clouds""" | |||
| """Escaped project-name known to be compatible with all clouds""" | |||
| # name, can only consist of lowercase letters and numbers, and must be between 3 and 24 characters long | ||
| name = replace("${var.name}${var.storage_account_postfix}", "-", "") # must be unique across the entire Azure service |
There was a problem hiding this comment.
This was one of the causes of the bug. The mutation logic where we create the terraform storage account is different from the mutation logic where we read the storage account. I've moved the logic into the python layer so it's the same logic on creation of the storage account and reading of the storage account (terraform import).
|
It's possible to fix #2405 with a simpler PR, but I feel that this PR has several advantages:
I expect upgrading to need to set the account name and storage container name in the nebari config. I also expect upgrading to delete/create the container registry, but that shouldn't be a major problem. |
|
This causes a change in the Nebari config of Azure deployments from provider: azure
namespace: dev
nebari_version: 2024.4.2
project_name: ne-bri6
domain: adam.nebari.dev
terraform_state:
type: remote
security:
keycloak:
initial_root_password: broot
authentication:
type: password
azure:
kubernetes_version: 1.29.0
region: centralus
storage_account_postfix: xhz2 # This key gets deleted ----------------------------------------------------
certificate:
type: lets-encrypt
acme_email: alewis@quansight.com
to provider: azure
namespace: dev
nebari_version: 2024.4.2.dev2+g85d3a757
project_name: ne-bri6
domain: adam.nebari.dev
terraform_state:
type: remote
storage_account_name: nebri6xhz2 # This key gets added ----------------------------------------------------
storage_container_name: nebri6 # This key gets added ----------------------------------------------------
security:
keycloak:
initial_root_password: df3vvlph2vcd53lcfsjq9p6nqzrn7j63
authentication:
type: password
azure:
kubernetes_version: 1.29.2
region: centralus
certificate:
type: lets-encrypt
acme_email: alewis@quansight.comNotice the |
| class TerraformState(schema.Base): | ||
| type: TerraformStateEnum = TerraformStateEnum.remote | ||
| type: TerraformStateEnum | ||
|
|
||
|
|
||
| class ExistingTerraformState(TerraformState): | ||
| type: TerraformStateEnum = TerraformStateEnum.existing | ||
| backend: Optional[str] = None | ||
| config: Dict[str, str] = {} |
There was a problem hiding this comment.
I need to check that this won't cause problems when we don't specify a terraform state.
Also I need to make sure it won't mess up a non Azure deployment with remote terraform state e.g. GCP.
Reference Issues or PRs
Fixes #2405
What does this implement/fix?
Put a
xin the boxes that applyTesting
Any other comments?