From 4c85fc2b1638fbb981789d0e623f2cf2e694b4b2 Mon Sep 17 00:00:00 2001 From: Vitaly Antonenko Date: Fri, 12 Jul 2024 14:39:48 +0300 Subject: [PATCH] fix(application): handle storage details conversion error - Added error handling for "cannot convert storage details" in `applicationsClient.ReadApplication`. - Improved retry logic for storage readiness in `ReadApplicationWithRetryOnNotFound`. This commit ensures that the application correctly handles storage conversion errors and retries appropriately until the storage is ready. --- internal/juju/applications.go | 27 ++++---- .../provider/resource_application_test.go | 61 ++++++++++++++++++- 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/internal/juju/applications.go b/internal/juju/applications.go index c8ba9813..f9b101ce 100644 --- a/internal/juju/applications.go +++ b/internal/juju/applications.go @@ -762,6 +762,18 @@ func (c applicationsClient) ReadApplicationWithRetryOnNotFound(ctx context.Conte c.Errorf(err, fmt.Sprintf("ReadApplicationWithRetryOnNotFound %q", input.AppName)) return nil } + + // NOTE: Applications can always have storage. However, they + // will not be listed right after the application is created. So + // we need to wait for the storage to be ready. And we need to + // check if all storage constraints have pool equal "" and size equal 0 + // to drop the error. + for _, storage := range output.Storage { + if storage.Pool == "" || storage.Size == 0 { + return fmt.Errorf("ReadApplicationWithRetryOnNotFound: no storage found in output") + } + } + // NOTE: An IAAS subordinate should also have machines. However, they // will not be listed until after the relation has been created. // Those happen with the integration resource which will not be @@ -779,17 +791,6 @@ func (c applicationsClient) ReadApplicationWithRetryOnNotFound(ctx context.Conte return fmt.Errorf("ReadApplicationWithRetryOnNotFound: need %d machines, have %d", output.Units, len(machines)) } - // NOTE: Applications can always have storage. However, they - // will not be listed right after the application is created. So - // we need to wait for the storage to be ready. And we need to - // check if all storage constraints have pool equal "" and size equal 0 - // to drop the error. - for _, storage := range output.Storage { - if storage.Pool == "" || storage.Size == 0 { - return fmt.Errorf("ReadApplicationWithRetryOnNotFound: no storage found in output") - } - } - // NOTE: An IAAS subordinate should also have machines. However, they // will not be listed until after the relation has been created. // Those happen with the integration resource which will not be @@ -899,7 +900,9 @@ func (c applicationsClient) ReadApplication(input *ReadApplicationInput) (*ReadA IncludeStorage: true, }) if err != nil { - if strings.Contains(err.Error(), "filesystem for storage instance") || strings.Contains(err.Error(), "volume for storage instance") { + if strings.Contains(err.Error(), "filesystem for storage instance") || + strings.Contains(err.Error(), "volume for storage instance") || + strings.Contains(err.Error(), "cannot convert storage details") { // Retry if we get this error. It means the storage is not ready yet. return nil, &storageNotFoundError{input.AppName} } diff --git a/internal/provider/resource_application_test.go b/internal/provider/resource_application_test.go index 02f1f989..2df8e0d9 100644 --- a/internal/provider/resource_application_test.go +++ b/internal/provider/resource_application_test.go @@ -575,7 +575,7 @@ func TestAcc_ResourceApplication_UpdateEndpointBindings(t *testing.T) { }) } -func TestAcc_ResourceApplication_Storage(t *testing.T) { +func TestAcc_ResourceApplication_StorageLXD(t *testing.T) { if testingCloud != LXDCloudTesting { t.Skip(t.Name() + " only runs with LXD") } @@ -589,7 +589,7 @@ func TestAcc_ResourceApplication_Storage(t *testing.T) { ProtoV6ProviderFactories: frameworkProviderFactories, Steps: []resource.TestStep{ { - Config: testAccResourceApplicationStorage(modelName, appName, storageConstraints), + Config: testAccResourceApplicationStorageLXD(modelName, appName, storageConstraints), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("juju_application."+appName, "model", modelName), resource.TestCheckResourceAttr("juju_application."+appName, "storage_directives.runner", "2G"), @@ -603,6 +603,34 @@ func TestAcc_ResourceApplication_Storage(t *testing.T) { }) } +func TestAcc_ResourceApplication_StorageK8s(t *testing.T) { + if testingCloud != MicroK8sTesting { + t.Skip(t.Name() + " only runs with Microk8s") + } + modelName := acctest.RandomWithPrefix("tf-test-application-storage") + appName := "test-app-storage" + + storageConstraints := map[string]string{"label": "pgdata", "size": "2G"} + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: frameworkProviderFactories, + Steps: []resource.TestStep{ + { + Config: testAccResourceApplicationStorageK8s(modelName, appName, storageConstraints), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("juju_application."+appName, "model", modelName), + resource.TestCheckResourceAttr("juju_application."+appName, "storage_directives.pgdata", "2G"), + resource.TestCheckResourceAttr("juju_application."+appName, "storage.0.label", "pgdata"), + resource.TestCheckResourceAttr("juju_application."+appName, "storage.0.count", "1"), + resource.TestCheckResourceAttr("juju_application."+appName, "storage.0.size", "2G"), + resource.TestCheckResourceAttr("juju_application."+appName, "storage.0.pool", "kubernetes"), + ), + }, + }, + }) +} + func testAccResourceApplicationBasic_Minimal(modelName, charmName string) string { return fmt.Sprintf(` resource "juju_model" "testmodel" { @@ -950,7 +978,7 @@ resource "juju_application" "{{.AppName}}" { }) } -func testAccResourceApplicationStorage(modelName, appName string, storageConstraints map[string]string) string { +func testAccResourceApplicationStorageLXD(modelName, appName string, storageConstraints map[string]string) string { return internaltesting.GetStringFromTemplateWithData("testAccResourceApplicationStorage", ` resource "juju_model" "{{.ModelName}}" { name = "{{.ModelName}}" @@ -978,6 +1006,33 @@ resource "juju_application" "{{.AppName}}" { }) } +func testAccResourceApplicationStorageK8s(modelName, appName string, storageConstraints map[string]string) string { + return internaltesting.GetStringFromTemplateWithData("testAccResourceApplicationStorage", ` +resource "juju_model" "{{.ModelName}}" { + name = "{{.ModelName}}" +} + +resource "juju_application" "{{.AppName}}" { + model = juju_model.{{.ModelName}}.name + name = "{{.AppName}}" + charm { + name = "postgresql-k8s" + channel = "14/stable" + } + + storage_directives = { + {{.StorageConstraints.label}} = "{{.StorageConstraints.size}}" + } + + units = 1 +} +`, internaltesting.TemplateData{ + "ModelName": modelName, + "AppName": appName, + "StorageConstraints": storageConstraints, + }) +} + func testCheckEndpointsAreSetToCorrectSpace(modelName, appName, defaultSpace string, configuredEndpoints map[string]string) resource.TestCheckFunc { return func(s *terraform.State) error { conn, err := TestClient.Models.GetConnection(&modelName)