wandb / terraform-aws-wandb

A terraform module for deploying Weights & Biases on AWS.
Apache License 2.0
17 stars 19 forks source link

feat: Add optional path var for instance level bucket path #251

Closed levinandrew closed 2 months ago

levinandrew commented 3 months ago
  # module.wandb_infra.module.wandb.helm_release.wandb will be updated in-place
  ~ resource "helm_release" "wandb" {
        id                         = "wandb-cr"
      ~ metadata                   = [
          - {
              ...
              - values         = jsonencode(
                    {
                      - name = "wandb"
                      - spec = <<-EOT
                            "values":
                              "app": {}
                              "global":
                                "bucket":
                                  "kmsKey": "arn:aws:kms:us-west-2:770934259321:key/c88a4da3-aabd-4265-a57d-e4f0115d04b8"
                                  "name": "levin-aws-subpath-testing-file-storage-unbiased-chimp"
                                  "path": ""
                                  "provider": "s3"
                                  "region": "us-west-2"
                                "cloudProvider": "aws"
                         ...
                        EOT
                    }
                )
              - version        = "0.1.0"
            },
        ] -> (known after apply)
        name                       = "wandb-cr"
        # (26 unchanged attributes hidden)

      - set {
          - name  = "spec" -> null
          - type  = "string" -> null
          - value = <<-EOT
                "values":
                  "app": {}
                  "global":
                    "bucket":
                      "kmsKey": "arn:aws:kms:us-west-2:770934259321:key/c88a4da3-aabd-4265-a57d-e4f0115d04b8"
                      "name": "levin-aws-subpath-testing-file-storage-unbiased-chimp"
                      "path": ""
                      "provider": "s3"
                      "region": "us-west-2"
                    "cloudProvider": "aws"

            EOT -> null
        }
      + set {
          + name  = "spec"
          + type  = "string"
          + value = <<-EOT
                "values":
                  "app": {}
                  "global":
                    "bucket":
                      "kmsKey": "arn:aws:kms:us-west-2:770934259321:key/c88a4da3-aabd-4265-a57d-e4f0115d04b8"
                      "name": "levin-aws-subpath-testing-file-storage-unbiased-chimp"
                      "path": "nested/path"
                      "provider": "s3"
                      "region": "us-west-2"
                    "cloudProvider": "aws"
 ...
            EOT
        }

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
zacharyblasczyk commented 2 months ago

@levinandrew what was the driver of this change for context?

CC @wandb/delivery-tooling-team

zacharyblasczyk commented 2 months ago

The reason path is in the helm spec currently is because azure requires it for the storage account/ blob store.

levinandrew commented 2 months ago

Hey @zacharyblasczyk thanks for the review 🤜 🤛

This did come from a customer ask to be able to store all data at a path of the instance level bucket instead of its root. We are making similar changes for Azure and Google, and will handle the Azure case you commented on.

jsbroks commented 2 months ago

This PR is included in version 4.23.0 :tada: