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-3654] Added ApplicationNotFound error for better error control. #206

Conversation

juanmanuel-tirado
Copy link
Contributor

@juanmanuel-tirado juanmanuel-tirado commented May 10, 2023

Description

Revisit how the returned data from the ApplicationInfo was processed in order to avoid problems when applications are not found. Apart from that, add the ApplicationNotFoundError that will help to handle erroneous situations using the handleApplicationNotFoundError function. This pattern will have to be propagated to the other resources.

Additionally, I have replaced the ubuntu charms to skip the storage issue with the recent ubuntu charm.

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.3.7

QA steps

Create an application with a plan, remove the application using Juju and see how the provider finds the application to be missing and tries to recreate it.

provider "juju" {

}

resource "juju_model" "test" {
  name = "test"
}

resource "juju_application" "test" {
  model = juju_model.test.name
  name  = "test"
  charm {
    name = "tiny-bash"
  }
}
terraform init
terraform apply
juju remove-application test -m test
terraform plan

This should display the application to be added

# juju_application.test will be created
  + resource "juju_application" "test" {
      + constraints = (known after apply)
      + id          = (known after apply)
      + model       = "test"
      + name        = "test"
      + placement   = (known after apply)
      + principal   = (known after apply)
      + trust       = false
      + units       = 1

      + charm {
          + channel  = "latest/stable"
          + name     = "tiny-bash"
          + revision = (known after apply)
          + series   = (known after apply)
        }
    }

Additional notes

This change follows the suggestions of @amandahla in #205

@juanmanuel-tirado juanmanuel-tirado marked this pull request as ready for review May 11, 2023 08:11
@juanmanuel-tirado juanmanuel-tirado self-assigned this May 11, 2023
@juanmanuel-tirado juanmanuel-tirado added the kind/bug indicates a bug in the project label May 11, 2023
@juanmanuel-tirado juanmanuel-tirado added this to the 0.7.0 milestone May 11, 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.

This LGTM 👍

@juanmanuel-tirado juanmanuel-tirado merged commit 116e37f into juju:main May 11, 2023
@juanmanuel-tirado juanmanuel-tirado deleted the JUJU-3654_check_errors_reading_apps branch May 11, 2023 14:29
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.

2 participants