juju / terraform-provider-juju

A Terraform provider for Juju
Apache License 2.0
21 stars 38 forks source link

Terraform reports storage changes on applications that are not using storage #550

Closed gtato closed 1 month ago

gtato commented 2 months ago

Description

I have a terraform plan consisting of multiple applications, one of them using storage directives (a postgres app) and others that don't. Whenever I upgrade some configuration of the apps that are not using storage, terraform plan shows as if the storage of the postgres application will be deleted. If the plan is executed, the storage is not actually deleted, but I think the plan is confusing.

Urgency

Annoying bug in our test suite

Terraform Juju Provider version

0.13.0

Terraform version

v1.7.2

Juju version

3.1.9

Terraform Configuration(s)

terraform {
  required_version = "~> 1.7.2"
  required_providers {
    juju = {
      version = "0.13.0"
      source  = "juju/juju"
    }
  }
}

variable "model" {
  type    = string
  default = "test"
}

resource "juju_application" "postgresql-k8s" {
  name  = "test-postgresql"
  model = var.model
  trust = true

  charm {
    name     = "postgresql-k8s"
    channel  = "14/stable"
    revision = 198
  }

  storage_directives = {
    pgdata = "1G,kubernetes"
  }
}

resource "juju_application" "data-integrator" {
  name  = "test-data-integrator"
  model = var.model
  trust = true

  charm {
    name     = "data-integrator"
    channel  = "latest/stable"
    revision = 40
  }

  config = {
    database-name    = "test"
    extra-user-roles = "admin"
  }

}

resource "juju_integration" "postgres-di" {
  model = var.model

  application {
    name = "test-postgresql"
  }

  application {
    name = "test-data-integrator"
  }
}

Reproduce / Test

terraform init 
terraform apply 
# change the revision of the data-integrator charm from 40 to 41
terraform apply 
# the plan would show as below...
# i find it confusing why the pgdata-19 appears as a storage of the data-integrator 
# which is about to be destroyed, when the data-integrator doesn't have any storage

Debug/Panic Output

Terraform will perform the following actions:

  # juju_application.data-integrator will be updated in-place
  ~ resource "juju_application" "data-integrator" {
        id          = "test:test-data-integrator"
        name        = "test-data-integrator"
      + principal   = (known after apply)
      ~ storage     = [
          - {
              - count = 1 -> null
              - label = "pgdata-19" -> null
              - pool  = "kubernetes" -> null
              - size  = "1G" -> null
            },
        ] -> (known after apply)
        # (5 unchanged attributes hidden)

      ~ charm {
            name     = "data-integrator"
          ~ revision = 40 -> 41
            # (3 unchanged attributes hidden)
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Notes & References

I had this issue in production and I was afraid to continue the plan because I wasn't sure if that would destroy the database.

hemanthnakkina commented 2 months ago

+1 on this terraform state should not hold unrelated storage details for application resource. I mentioned in this bug as well https://github.com/juju/terraform-provider-juju/issues/539

anvial commented 2 months ago

Hey @hemanthnakkina,

Failed to reproduce it; here is the result: https://paste.ubuntu.com/p/78zkhZG5kV/ (juju 3.5 + microk8s)

It looks like it's working. The only concern is that storage pgdata was not deleted after plan update:

└─▪ juju status --storage
Model  Controller  Cloud/Region        Version  SLA          Timestamp
test   microk8s    microk8s/localhost  3.5.3    unsupported  13:52:06+03:00

App                   Version  Status  Scale  Charm            Channel        Rev  Address         Exposed  Message
test-data-integrator           active      1  data-integrator  latest/stable   41  10.152.183.109  no
test-postgresql       14.11    active      1  postgresql-k8s   14/stable      198  10.152.183.206  no

Unit                     Workload  Agent  Address      Ports  Message
test-data-integrator/0*  active    idle   10.1.52.196
test-postgresql/0*       active    idle   10.1.52.251         Primary

Storage Unit       Storage ID  Type        Pool        Mountpoint                Size     Status    Message
test-postgresql/0  pgdata/0    filesystem  kubernetes  /var/lib/postgresql/data  1.0 GiB  attached

hm...

gtato commented 2 months ago

@anvial I think you have reproduced it. Lines 288-295 are the ones I am talking about. Why is there a storage associated with data-integrator?

anvial commented 2 months ago

Yep, now I see. It looks like there is a bug in case we change something with the integrated app.

Interesting that it wishes to remove (according to terraform apply), but actually didn't remove, but frankly is shouldn't remove.

hmlanigan commented 2 months ago

The problem is because we're getting full juju status data for the entire model and not checking the storage listed is for the application we're interested in. The fix will be to call status for only the application we're interested in.

This change will also fix #539 and help with #535