diff --git a/internal/juju/applications.go b/internal/juju/applications.go index f9b101ce..b01e01f2 100644 --- a/internal/juju/applications.go +++ b/internal/juju/applications.go @@ -71,6 +71,17 @@ func (se *storageNotFoundError) Error() string { return fmt.Sprintf("storage %s not found", se.storageName) } +var RetryReadError = &retryReadError{} + +// retryReadError +type retryReadError struct { + msg string +} + +func (e *retryReadError) Error() string { + return fmt.Sprintf("retrying: %s", e.msg) +} + type applicationsClient struct { SharedClient controllerVersion version.Number @@ -756,11 +767,7 @@ func (c applicationsClient) ReadApplicationWithRetryOnNotFound(ctx context.Conte if errors.As(err, &ApplicationNotFoundError) || errors.As(err, &StorageNotFoundError) { return err } else if err != nil { - // Log the error to the terraform Diagnostics to be - // caught in the provider. Return nil so we stop - // retrying. - c.Errorf(err, fmt.Sprintf("ReadApplicationWithRetryOnNotFound %q", input.AppName)) - return nil + return err } // NOTE: Applications can always have storage. However, they @@ -768,9 +775,9 @@ func (c applicationsClient) ReadApplicationWithRetryOnNotFound(ctx context.Conte // 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 { + for label, storage := range output.Storage { if storage.Pool == "" || storage.Size == 0 { - return fmt.Errorf("ReadApplicationWithRetryOnNotFound: no storage found in output") + return &retryReadError{msg: fmt.Sprintf("storage label %q missing detail", label)} } } @@ -784,21 +791,25 @@ func (c applicationsClient) ReadApplicationWithRetryOnNotFound(ctx context.Conte return nil } if output.Placement == "" { - return fmt.Errorf("ReadApplicationWithRetryOnNotFound: no machines found in output") + return &retryReadError{msg: "no machines found in output"} } machines := strings.Split(output.Placement, ",") if len(machines) != output.Units { - return fmt.Errorf("ReadApplicationWithRetryOnNotFound: need %d machines, have %d", output.Units, len(machines)) + return &retryReadError{msg: fmt.Sprintf("need %d machines, have %d", output.Units, len(machines))} } - // 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 - // run by terraform before the application resource finishes. Thus - // do not block here for subordinates. c.Tracef("Have machines - returning", map[string]interface{}{"output": *output}) return nil }, + IsFatalError: func(err error) bool { + if errors.As(err, &ApplicationNotFoundError) || + errors.As(err, &StorageNotFoundError) || + errors.As(err, &RetryReadError) || + strings.Contains(err.Error(), "connection refused") { + return false + } + return true + }, NotifyFunc: func(err error, attempt int) { if attempt%4 == 0 { message := fmt.Sprintf("waiting for application %q", input.AppName) @@ -874,8 +885,7 @@ func (c applicationsClient) ReadApplication(input *ReadApplicationInput) (*ReadA apps, err := applicationAPIClient.ApplicationsInfo([]names.ApplicationTag{names.NewApplicationTag(input.AppName)}) if err != nil { - c.Errorf(err, "found when querying the applications info") - return nil, err + return nil, jujuerrors.Annotate(err, "when querying the applications info") } if len(apps) > 1 { return nil, fmt.Errorf("more than one result for application: %s", input.AppName) diff --git a/internal/juju/applications_test.go b/internal/juju/applications_test.go index ec7b996f..0235cc8f 100644 --- a/internal/juju/applications_test.go +++ b/internal/juju/applications_test.go @@ -6,6 +6,7 @@ package juju // Basic imports import ( "context" + "fmt" "testing" "github.com/juju/juju/api" @@ -168,6 +169,24 @@ func (s *ApplicationSuite) TestReadApplicationRetry() { s.Assert().Equal("ubuntu@22.04", resp.Base) } +func (s *ApplicationSuite) TestReadApplicationRetryDoNotPanic() { + defer s.setupMocks(s.T()).Finish() + s.mockSharedClient.EXPECT().ModelType(gomock.Any()).Return(model.IAAS, nil).AnyTimes() + + appName := "testapplication" + aExp := s.mockApplicationClient.EXPECT() + + aExp.ApplicationsInfo(gomock.Any()).Return(nil, fmt.Errorf("don't panic")) + + client := s.getApplicationsClient() + _, err := client.ReadApplicationWithRetryOnNotFound(context.Background(), + &ReadApplicationInput{ + ModelName: s.testModelName, + AppName: appName, + }) + s.Require().Error(err, "don't panic") +} + func (s *ApplicationSuite) TestReadApplicationRetryWaitForMachines() { defer s.setupMocks(s.T()).Finish() s.mockSharedClient.EXPECT().ModelType(gomock.Any()).Return(model.IAAS, nil).AnyTimes()