Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JUJU-3863] Process region value in cloud models and force it to be computed #214

Conversation

juanmanuel-tirado
Copy link
Contributor

Description

The region parameter in the cloud section from the resource_model should be computed by the provider when empty. In this manner, something like:

resource "juju_model" "terraform" {
  name = "terraform"
  cloud {
    name   = "localhost"
  }
}

will automatically set the region value to be localhost which is the default value returned by the Juju provider.

Fix #209

Type of change

Please mark if proceeds.

  • New resource (a new resource in the schema)
  • Changed resource (changes in an existing resource)
  • Logic changes in resources (the API interaction with Juju has been changed)
  • Test changes (one or several tests have been changed)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Other (describe)

Environment

  • Juju controller version: 2.9.42
  • Terraform version: 1.36

QA steps

Try to apply the plan described in #209 twice. No new values are detected now.

terraform {
  required_providers {
    juju = {
      version = "~> 0.7.0"
      source  = "juju/juju"
    }
  }
}


provider "juju" {}

resource "juju_model" "terraform" {
  name = "terraform"

  cloud {
    name   = "localhost"
  }
}
terraform init
terraform apply --auto-approve
Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the
following symbols:
  + create

Terraform will perform the following actions:

  # juju_model.terraform will be created
  + resource "juju_model" "terraform" {
      + credential = (known after apply)
      + id         = (known after apply)
      + name       = "terraform"
      + type       = (known after apply)

      + cloud {
          + name   = "localhost"
          + region = (known after apply)
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.
juju_model.terraform: Creating...
juju_model.terraform: Creation complete after 6s [id=77a75e5b-1236-48b5-8564-6c234f36767e]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
terraform apply --auto-approve
juju_model.terraform: Refreshing state... [id=77a75e5b-1236-48b5-8564-6c234f36767e]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are
needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

@juanmanuel-tirado juanmanuel-tirado added the kind/bug indicates a bug in the project label May 29, 2023
@juanmanuel-tirado juanmanuel-tirado added this to the 0.8.0 milestone May 29, 2023
@juanmanuel-tirado juanmanuel-tirado self-assigned this May 29, 2023
Copy link
Contributor

@cderici cderici left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (modulo a possible fix required in code), QA went well 👍

for _, c := range cloud {
cloudEntry := c.(map[string]interface{})
cloudNameInput = cloudEntry["name"].(string)
cloudRegionInput = cloudEntry["region"].(string)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this part of the code dead? cloud is set on d below only when the cloudChanged is true, and that happens only when either cloudNameInput == "" or cloudRegionInput == "".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case cloud is obtained from cloud := d.Get("cloud").([]interface{}) and that is basically the value set by the user in the plan. Because it's defined as a list of maps, we have to do all that weird workaround.

@juanmanuel-tirado juanmanuel-tirado merged commit 05efe28 into juju:main May 30, 2023
@juanmanuel-tirado juanmanuel-tirado deleted the JUJU-3863_model_fields_update_failure branch May 30, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug indicates a bug in the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

terraform reports region value update which doesn't get resolved on apply.
2 participants