Skip to content

Commit

Permalink
Merge pull request #557 from hmlanigan/storage-per-application
Browse files Browse the repository at this point in the history
#557

## Description

FullStatus was called without a filter. The storage returned in the structure was assumed to be for the application being read. In reality it was for all application's in the model. Unfortunately piecing which storage is for which application is non-trivial as the data is embedded deep in the structure on a unit level.

Instead call FullStatus with a filter of the application name when Reading, this ensures that the returned storage is for the current application only.

Updated an application acceptance test to ensure storage isn't written for applications without storage.

Fixes: #535, #539, #550

## Type of change

- Bug fix (non-breaking change which fixes an issue)

## QA steps

```tf
terraform {
 required_providers {
 juju = {
 version = ">0.12.0"
 source = "juju/juju"
 }
 }
}

provider "juju" {
}

resource "juju_model" "one" {
 name = "testing"
}

resource "juju_application" "test" {
 model = juju_model.one.name
 charm {
 name = "juju-qa-test"
 }
}

resource "juju_application" "githubrunner" {
 name = "github-runner"
 model = juju_model.one.name

 charm {
 name = "github-runner"
 channel = "latest/stable"
 base = "[email protected]"
 }
 units = 1

 storage_directives = {
 runner = "2G"
 }
}
```

```
# output from the below should include storage_directive and storage
$ terraform state show juju_application.githubrunner

# output from below should not include any mention of storage
$ terraform state show juju_application.test
```

## Additional notes

JUJU-6599
  • Loading branch information
jujubot authored Aug 29, 2024
2 parents 8d5ec5f + 4d59f2d commit 9f9fc02
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
10 changes: 5 additions & 5 deletions internal/juju/applications.go
Original file line number Diff line number Diff line change
Expand Up @@ -915,12 +915,12 @@ func (c applicationsClient) ReadApplication(input *ReadApplicationInput) (*ReadA

appInfo := apps[0].Result

// We are currently retrieving the full status, which might be more information than necessary.
// The main focus is on the application status, particularly including the storage status.
// It might be more efficient to directly query for the application status and its associated storage status.
// TODO: Investigate if there's a way to optimize this by only fetching the required information.
// Fetch data only about the application being read. This helps to limit
// the data on storage to the specific application too. Storage is not
// provided by application, rather storage data buries a unit name deep
// in the structure.
status, err := clientAPIClient.Status(&apiclient.StatusArgs{
Patterns: []string{},
Patterns: []string{input.AppName},
IncludeStorage: true,
})
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions internal/provider/resource_application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func TestAcc_ResourceApplication(t *testing.T) {
resource.TestCheckResourceAttr("juju_application.this", "charm.0.name", "jameinel-ubuntu-lite"),
resource.TestCheckResourceAttr("juju_application.this", "trust", "true"),
resource.TestCheckResourceAttr("juju_application.this", "expose.#", "1"),
resource.TestCheckNoResourceAttr("juju_application.this", "storage"),
),
},
{
Expand Down

0 comments on commit 9f9fc02

Please sign in to comment.