Skip to content

Commit

Permalink
Merge pull request juju#18092 from wallyworld/cleanup-unit-insertion
Browse files Browse the repository at this point in the history
juju#18092

In application state, the logic for inserting and updating a unit was intertwined after code for register caas unit was added some time after the initial vm add unit code.
They are now separate operations. The branching logic in state has been moved up a layer.

## QA steps

juju bootstrap microk8s
juju deploy snappass-test

## Links

**Jira card:** [JUJU-6059](https://warthogs.atlassian.net/browse/JUJU-6059)



[JUJU-6059]: https://warthogs.atlassian.net/browse/JUJU-6059?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
jujubot authored Sep 18, 2024
2 parents 0f3d12b + 79a5046 commit 4594746
Show file tree
Hide file tree
Showing 6 changed files with 187 additions and 87 deletions.
8 changes: 4 additions & 4 deletions domain/application/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ const (
// does not exist.
UnitNotFound = errors.ConstError("unit not found")

// UnitNotAssigned describes an error that occurs when the unit being operated on
// is not assigned.
UnitNotAssigned = errors.ConstError("unit not assigned")

// UnitAlreadyExists describes an error that occurs when the
// unit being created already exists.
UnitAlreadyExists = errors.ConstError("unit already exists")

// UnitNotAssigned describes an error that occurs when the unit being operated on
// is not assigned.
UnitNotAssigned = errors.ConstError("unit not assigned")

// UnitHasSubordinates describes an error that occurs when trying to set a unit's life
// to Dead but it still has subordinates.
UnitHasSubordinates = errors.ConstError("unit has subordinates")
Expand Down
22 changes: 13 additions & 9 deletions domain/application/service/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,14 @@ type AtomicApplicationState interface {
// application is not found.
GetApplicationID(ctx domain.AtomicContext, name string) (coreapplication.ID, error)

// UpsertUnit creates or updates the specified application unit, returning
// an error satisfying [applicationerrors.ApplicationNotFoundError] if the
// application doesn't exist.
UpsertUnit(domain.AtomicContext, coreapplication.ID, application.UpsertUnitArg) error
// InsertUnit insert the specified application unit, returning an error
// satisfying [applicationerrors.UnitAlreadyExists]
// if the unit exists.
InsertUnit(domain.AtomicContext, coreapplication.ID, application.UpsertUnitArg) error

// UpdateUnit updates the specified application unit, returning an error
// satisfying [applicationerrors.UnitNotFoundError] if the unit doesn't exist.
UpdateUnit(domain.AtomicContext, coreapplication.ID, application.UpsertUnitArg) error

// GetApplicationLife looks up the life of the specified application, returning an error
// satisfying [applicationerrors.ApplicationNotFoundError] if the application is not found.
Expand Down Expand Up @@ -298,11 +302,13 @@ func makeUpsertUnitArgs(in AddUnitArg) application.UpsertUnitArg {
Ports: in.CloudContainer.Ports,
}
if in.CloudContainer.Address != nil {
// TODO(units) - handle the in.CloudContainer.Address space ID
// For k8s we'll initially create a /32 subnet off the container address
// and add that to the default space.
result.CloudContainer.Address = &application.Address{
Value: in.CloudContainer.Address.Value,
AddressType: string(in.CloudContainer.Address.AddressType()),
Scope: string(in.CloudContainer.Address.Scope),
SpaceID: in.CloudContainer.Address.SpaceID,
Origin: string(network.OriginProvider),
}
if in.CloudContainer.AddressOrigin != nil {
Expand Down Expand Up @@ -516,8 +522,6 @@ func (s *ApplicationService) RegisterCAASUnit(ctx context.Context, appName strin
}
if args.Address != nil {
addr := network.NewSpaceAddress(*args.Address, network.WithScope(network.ScopeMachineLocal))
// k8s doesn't support spaces yet.
addr.SpaceID = network.AlphaSpaceId
p.CloudContainer.Address = &addr
origin := network.OriginProvider
p.CloudContainer.AddressOrigin = &origin
Expand All @@ -537,7 +541,7 @@ func (s *ApplicationService) RegisterCAASUnit(ctx context.Context, appName strin
if unitLife == life.Dead {
return fmt.Errorf("dead unit %q already exists%w", args.UnitName, errors.Hide(applicationerrors.UnitAlreadyExists))
}
return s.st.UpsertUnit(ctx, appID, unitArg)
return s.st.UpdateUnit(ctx, appID, unitArg)
})
return errors.Annotatef(err, "saving caas unit %q", args.UnitName)
}
Expand All @@ -553,7 +557,7 @@ func (s *ApplicationService) insertCAASUnit(
(appScale.Scaling && orderedID >= appScale.ScaleTarget) {
return fmt.Errorf("unrequired unit %s is not assigned%w", *args.UnitName, errors.Hide(applicationerrors.UnitNotAssigned))
}
return s.st.UpsertUnit(ctx, appID, args)
return s.st.InsertUnit(ctx, appID, args)
}

// DeleteApplication deletes the specified application, returning an error
Expand Down
86 changes: 62 additions & 24 deletions domain/application/service/package_mock_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 4594746

Please sign in to comment.