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

fix(application) wait for application to be destroyed #524

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ require (
github.com/juju/cmd/v3 v3.0.14
github.com/juju/collections v1.0.4
github.com/juju/errors v1.0.0
github.com/juju/names/v4 v4.0.0-20220207005702-9c6532a52823
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we mistakenly started using names/v4 instead v5 somewhere. I think we need to use just v5

github.com/juju/names/v5 v5.0.0
github.com/juju/retry v1.0.0
github.com/juju/utils/v3 v3.1.1
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,6 @@ github.com/juju/mgo/v3 v3.0.4 h1:ek6YDy71tqikpoFSpvLkpCZ7zvYNYH+xSk/MebMkCEE=
github.com/juju/mgo/v3 v3.0.4/go.mod h1:fAvhDCRbUlEbRIae6UQT8RvPUoLwKnJsBgO6OzHKNxw=
github.com/juju/mutex/v2 v2.0.0 h1:rVmJdOaXGWF8rjcFHBNd4x57/1tks5CgXHx55O55SB0=
github.com/juju/mutex/v2 v2.0.0/go.mod h1:jwCfBs/smYDaeZLqeaCi8CB8M+tOes4yf827HoOEoqk=
github.com/juju/names/v4 v4.0.0-20220207005702-9c6532a52823 h1:Sv0+v4107/GHA0S25ay/rgGVmLyc+5Fjp0NnTksW/IQ=
github.com/juju/names/v4 v4.0.0-20220207005702-9c6532a52823/go.mod h1:xpkrQpHbz1DGY+0Geo32ZnyognGA/2vSB++rpu/Z+Lc=
github.com/juju/names/v5 v5.0.0 h1:3IkRTUaniNXsgjy4lNqbJx7dVdsONlzuH6YMYT7uXss=
github.com/juju/names/v5 v5.0.0/go.mod h1:PkvHbErUTniKvLu1ejJ5m/AbXOW55MFn1jsGVEbVXk8=
github.com/juju/naturalsort v1.0.0 h1:kGmUUy3h8mJ5/SJYaqKOBR3f3owEd5R52Lh+Tjg/dNM=
Expand Down
96 changes: 67 additions & 29 deletions internal/juju/applications.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,28 +49,6 @@
goyaml "gopkg.in/yaml.v2"
)

var ApplicationNotFoundError = &applicationNotFoundError{}

// ApplicationNotFoundError
type applicationNotFoundError struct {
appName string
}

func (ae *applicationNotFoundError) Error() string {
return fmt.Sprintf("application %s not found", ae.appName)
}

var StorageNotFoundError = &storageNotFoundError{}

// StorageNotFoundError
type storageNotFoundError struct {
storageName string
}

func (se *storageNotFoundError) Error() string {
return fmt.Sprintf("storage %s not found", se.storageName)
}

type applicationsClient struct {
SharedClient
controllerVersion version.Number
Expand Down Expand Up @@ -158,7 +136,7 @@
// validateAndTransform returns transformedCreateApplicationInput which
// validated and in the proper format for both the new and legacy deployment
// methods. Select input is not transformed due to differences in the
// 2 deployement methods, such as config.
// 2 deployment methods, such as config.
func (input CreateApplicationInput) validateAndTransform(conn api.Connection) (parsed transformedCreateApplicationInput, err error) {
parsed.charmChannel = input.CharmChannel
parsed.charmName = input.CharmName
Expand Down Expand Up @@ -1268,29 +1246,89 @@
return nil
}

func (c applicationsClient) DestroyApplication(input *DestroyApplicationInput) error {
func (c applicationsClient) DestroyApplication(ctx context.Context, input *DestroyApplicationInput) error {
conn, err := c.GetConnection(&input.ModelName)
if err != nil {
return err
}
defer func() { _ = conn.Close() }()

applicationAPIClient := apiapplication.NewClient(conn)
applicationAPIClient := c.getApplicationAPIClient(conn)
appName := input.ApplicationName

var destroyParams = apiapplication.DestroyApplicationsParams{
Applications: []string{
input.ApplicationName,
appName,
},
DestroyStorage: true,
}

_, err = applicationAPIClient.DestroyApplications(destroyParams)

results, err := applicationAPIClient.DestroyApplications(destroyParams)
if err != nil {
return err
}
if len(results) != 1 {
return errors.New(fmt.Sprintf("Unexpected number of results from DestroyApplication for %q", appName))

Check failure on line 1271 in internal/juju/applications.go

View workflow job for this annotation

GitHub Actions / golangci-lint

S1028: should use fmt.Errorf(...) instead of errors.New(fmt.Sprintf(...)) (gosimple)
}
if results[0].Error != nil {
err := typedError(results[0].Error)
// No need to error if the application is not found. It may have been
// destroyed as part of a different destroy operation, e.g. machine.
// Only care that it's gone.
if jujuerrors.Is(err, jujuerrors.NotFound) {
return nil
}
return err
}

return nil
// Best effort wait for the application to be destroyed.
retryErr := retry.Call(retry.CallArgs{
Func: func() error {
apps, err := applicationAPIClient.ApplicationsInfo([]names.ApplicationTag{names.NewApplicationTag(appName)})
if err != nil {
return jujuerrors.Annotatef(err, fmt.Sprintf("querying the applications info after destroy on %q", appName))
}
if len(apps) != 1 {
c.Debugf(fmt.Sprintf("unexpected number of results (%d) for application %q info after destroy", len(apps), appName))
return nil
}
if apps[0].Error != nil {
typedErr := typedError(apps[0].Error)
if jujuerrors.Is(typedErr, jujuerrors.NotFound) {
c.Tracef(fmt.Sprintf("%q not found, application has been destroyed", appName))
return nil
}
return jujuerrors.Annotatef(typedErr, fmt.Sprintf("verifying application %q distruction", appName))
}

life := apps[0].Result.Life
return &keepWaitingForDestroyError{
itemDestroying: appName,
life: life,
}
},
NotifyFunc: func(err error, attempt int) {
if attempt%4 == 0 {
message := fmt.Sprintf("waiting for application %q to be destroyed", appName)
if attempt != 4 {
message = "still " + message
}
c.Debugf(message, map[string]interface{}{"attempt": attempt, "err": err})
}
},
IsFatalError: func(err error) bool {
if errors.As(err, &KeepWaitingForDestroyError) {

Check failure on line 1320 in internal/juju/applications.go

View workflow job for this annotation

GitHub Actions / golangci-lint

S1008: should use 'return !errors.As(err, &KeepWaitingForDestroyError)' instead of 'if errors.As(err, &KeepWaitingForDestroyError) { return false }; return true' (gosimple)
return false
}
return true
},
BackoffFunc: retry.DoubleDelay,
Attempts: 30,
Delay: time.Second,
Clock: clock.WallClock,
Stop: ctx.Done(),
})
return retryErr
}

// computeSetCharmConfig populates the corresponding configuration object
Expand Down
54 changes: 53 additions & 1 deletion internal/juju/applications_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/juju/juju/core/resources"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/rpc/params"
"github.com/juju/names/v4"
"github.com/juju/names/v5"
"github.com/juju/utils/v3"
"github.com/juju/version/v2"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -364,6 +364,58 @@ func (s *ApplicationSuite) TestReadApplicationRetryNotFoundStorageNotFoundError(
s.Assert().Equal("[email protected]", resp.Base)
}

func (s *ApplicationSuite) TestDestroyApplicationDoNotFailOnNotFound() {
defer s.setupMocks(s.T()).Finish()
s.mockSharedClient.EXPECT().ModelType(gomock.Any()).Return(model.IAAS, nil).AnyTimes()

appName := "testapplication"
aExp := s.mockApplicationClient.EXPECT()

aExp.DestroyApplications(gomock.Any()).Return([]params.DestroyApplicationResult{{
Error: &params.Error{Message: `application "testapplication" not found`, Code: "not found"},
}}, nil)

client := s.getApplicationsClient()
err := client.DestroyApplication(context.Background(),
&DestroyApplicationInput{
ApplicationName: appName,
ModelName: s.testModelName,
})
s.Require().NoError(err)
}

func (s *ApplicationSuite) TestDestroyApplicationRetry() {
defer s.setupMocks(s.T()).Finish()
s.mockSharedClient.EXPECT().ModelType(gomock.Any()).Return(model.IAAS, nil).AnyTimes()

appName := "testapplication"
aExp := s.mockApplicationClient.EXPECT()

aExp.DestroyApplications(gomock.Any()).Return([]params.DestroyApplicationResult{{
Info: nil, Error: nil,
}}, nil)

infoResult := params.ApplicationInfoResult{
Result: &params.ApplicationResult{
Life: "dying",
},
Error: nil,
}
aExp.ApplicationsInfo(gomock.Any()).Return([]params.ApplicationInfoResult{infoResult}, nil)

aExp.ApplicationsInfo(gomock.Any()).Return([]params.ApplicationInfoResult{{
Error: &params.Error{Message: `application "testapplication" not found`, Code: "not found"},
}}, nil)

client := s.getApplicationsClient()
err := client.DestroyApplication(context.Background(),
&DestroyApplicationInput{
ApplicationName: appName,
ModelName: s.testModelName,
})
s.Require().NoError(err)
}

// In order for 'go test' to run this suite, we need to create
// a normal test function and pass our suite to suite.Run
func TestApplicationSuite(t *testing.T) {
Expand Down
76 changes: 76 additions & 0 deletions internal/juju/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package juju

import (
"fmt"
"strings"

jujuerrors "github.com/juju/errors"
Expand All @@ -30,3 +31,78 @@ func typedError(err error) error {
return err
}
}

var ApplicationNotFoundError = &applicationNotFoundError{}

// ApplicationNotFoundError
type applicationNotFoundError struct {
appName string
}

func (ae *applicationNotFoundError) Error() string {
return fmt.Sprintf("application %q not found", ae.appName)
}

var StorageNotFoundError = &storageNotFoundError{}

// StorageNotFoundError
type storageNotFoundError struct {
storageName string
}

func (se *storageNotFoundError) Error() string {
return fmt.Sprintf("storage %q not found", se.storageName)
}

var KeepWaitingForDestroyError = &keepWaitingForDestroyError{}

// keepWaitingForDestroyError
type keepWaitingForDestroyError struct {
itemDestroying string
life string
}

func (e *keepWaitingForDestroyError) Error() string {

return fmt.Sprintf("%q still alive, life = %s", e.itemDestroying, e.life)
}

var NoIntegrationFoundError = &noIntegrationFoundError{}

// NoIntegrationFoundError
type noIntegrationFoundError struct {
ModelUUID string
}

func (ie *noIntegrationFoundError) Error() string {
return fmt.Sprintf("no integrations exist in model %v", ie.ModelUUID)
}

var ModelNotFoundError = &modelNotFoundError{}

type modelNotFoundError struct {
uuid string
name string
}

func (me *modelNotFoundError) Error() string {
toReturn := "model %q was not found"
if me.name != "" {
return fmt.Sprintf(toReturn, me.name)
}
return fmt.Sprintf(toReturn, me.uuid)
}

var SecretNotFoundError = &secretNotFoundError{}

type secretNotFoundError struct {
secretId string
}

func (se *secretNotFoundError) Error() string {
if se.secretId != "" {
return fmt.Sprintf("secret %q was not found", se.secretId)
} else {
return "secret was not found"
}
}
11 changes: 0 additions & 11 deletions internal/juju/integrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,6 @@ const (
IntegrationAppAvailableTimeout = time.Second * 60
)

var NoIntegrationFoundError = &noIntegrationFoundError{}

// NoIntegrationFoundError
type noIntegrationFoundError struct {
ModelUUID string
}

func (ie *noIntegrationFoundError) Error() string {
return fmt.Sprintf("no integrations exist in model %v", ie.ModelUUID)
}

type integrationsClient struct {
SharedClient
}
Expand Down
28 changes: 8 additions & 20 deletions internal/juju/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,14 @@ import (
"fmt"
"time"

"github.com/juju/errors"
jujuerrors "github.com/juju/errors"
"github.com/juju/juju/api/client/modelconfig"
"github.com/juju/juju/api/client/modelmanager"
"github.com/juju/juju/core/constraints"
"github.com/juju/juju/rpc/params"
"github.com/juju/names/v5"
)

var ModelNotFoundError = &modelNotFoundError{}

type modelNotFoundError struct {
uuid string
name string
}

func (me *modelNotFoundError) Error() string {
toReturn := "model %q was not found"
if me.name != "" {
return fmt.Sprintf(toReturn, me.name)
}
return fmt.Sprintf(toReturn, me.uuid)
}

type modelsClient struct {
SharedClient
}
Expand Down Expand Up @@ -209,7 +194,7 @@ func (c *modelsClient) ReadModel(name string) (*ReadModelResponse, error) {

modelconfigConn, err := c.GetConnection(&name)
if err != nil {
return nil, errors.Wrap(err, &modelNotFoundError{uuid: name})
return nil, jujuerrors.Wrap(err, &modelNotFoundError{uuid: name})
}
defer func() { _ = modelconfigConn.Close() }()

Expand All @@ -218,7 +203,7 @@ func (c *modelsClient) ReadModel(name string) (*ReadModelResponse, error) {

modelUUIDTag, modelOk := modelconfigConn.ModelTag()
if !modelOk {
return nil, errors.Errorf("Not connected to model %q", name)
return nil, jujuerrors.Errorf("Not connected to model %q", name)
}
models, err := modelmanagerClient.ModelInfo([]names.ModelTag{modelUUIDTag})
if err != nil {
Expand Down Expand Up @@ -304,7 +289,7 @@ func (c *modelsClient) UpdateModel(input UpdateModelInput) error {
defer func() { _ = conn.Close() }()
modelUUIDTag, modelOk := conn.ModelTag()
if !modelOk {
return errors.Errorf("Not connected to model %q", input.Name)
return jujuerrors.Errorf("Not connected to model %q", input.Name)
}
clientModelManager := modelmanager.NewClient(connModelManager)
if err := clientModelManager.ChangeModelCredential(modelUUIDTag, *cloudCredTag); err != nil {
Expand Down Expand Up @@ -334,7 +319,10 @@ func (c *modelsClient) DestroyModel(input DestroyModelInput) error {

err = client.DestroyModel(tag, &destroyStorage, &forceDestroy, &maxWait, &timeout)
if err != nil {
return err
typedErr := typedError(err)
if !jujuerrors.Is(typedErr, jujuerrors.NotFound) {
return err
}
}

c.RemoveModel(input.UUID)
Expand Down
Loading
Loading