Skip to content

Commit 735afad

Browse files
jack-w-shawjack-w-shaw
committed
refactor(units): add unit.Name core static type
Wire this new type up into all domains which return unit names, or take unit names as parameters
1 parent f786427 commit 735afad

File tree

78 files changed

+1022
-555
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

78 files changed

+1022
-555
lines changed

‎apiserver/facades/agent/agent/agent.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/juju/juju/core/credential"
2121
"github.com/juju/juju/core/life"
2222
"github.com/juju/juju/core/model"
23+
coreunit "github.com/juju/juju/core/unit"
2324
applicationerrors "github.com/juju/juju/domain/application/errors"
2425
"github.com/juju/juju/internal/mongo"
2526
"github.com/juju/juju/rpc/params"
@@ -115,9 +116,14 @@ func (api *AgentAPI) GetEntities(ctx context.Context, args params.Entities) para
115116
// Handle units using the domain service.
116117
// Eventually all entities will be supported via dqlite.
117118
if tag.Kind() == names.UnitTagKind {
118-
lifeValue, err := api.applicationService.GetUnitLife(ctx, tag.Id())
119+
unitName, err := coreunit.NewName(tag.Id())
120+
if err != nil {
121+
results.Entities[i].Error = apiservererrors.ServerError(err)
122+
continue
123+
}
124+
lifeValue, err := api.applicationService.GetUnitLife(ctx, unitName)
119125
if errors.Is(err, applicationerrors.UnitNotFound) {
120-
err = errors.NotFoundf("unit %s", tag.Id())
126+
err = errors.NotFoundf("unit %s", unitName)
121127
}
122128
results.Entities[i].Life = lifeValue
123129
results.Entities[i].Error = apiservererrors.ServerError(err)

‎apiserver/facades/agent/agent/register.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/juju/juju/apiserver/facade"
1212
"github.com/juju/juju/core/credential"
1313
"github.com/juju/juju/core/life"
14+
"github.com/juju/juju/core/unit"
1415
"github.com/juju/juju/core/watcher"
1516
"github.com/juju/juju/domain/application/service"
1617
"github.com/juju/juju/internal/storage"
@@ -29,7 +30,7 @@ type CredentialService interface {
2930

3031
// ApplicationService provides access to the application service.
3132
type ApplicationService interface {
32-
GetUnitLife(ctx context.Context, name string) (life.Value, error)
33+
GetUnitLife(ctx context.Context, name unit.Name) (life.Value, error)
3334
}
3435

3536
// NewAgentAPIV3 returns an object implementing version 3 of the Agent API

‎apiserver/facades/agent/caasapplication/application.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package caasapplication
55

66
import (
77
"context"
8-
"fmt"
98
"path"
109
"strconv"
1110
"strings"
@@ -25,6 +24,7 @@ import (
2524
"github.com/juju/juju/core/logger"
2625
"github.com/juju/juju/core/network"
2726
"github.com/juju/juju/core/paths"
27+
"github.com/juju/juju/core/unit"
2828
applicationerrors "github.com/juju/juju/domain/application/errors"
2929
applicationservice "github.com/juju/juju/domain/application/service"
3030
"github.com/juju/juju/internal/password"
@@ -42,7 +42,7 @@ type ApplicationService interface {
4242
RegisterCAASUnit(ctx context.Context, appName string, unit applicationservice.RegisterCAASUnitParams) error
4343
CAASUnitTerminating(ctx context.Context, appName string, unitNum int, broker applicationservice.Broker) (bool, error)
4444
GetApplicationLife(ctx context.Context, appName string) (life.Value, error)
45-
GetUnitLife(ctx context.Context, unitName string) (life.Value, error)
45+
GetUnitLife(ctx context.Context, unitName unit.Name) (life.Value, error)
4646
}
4747

4848
// ModelAgentService provides access to the Juju agent version for the model.
@@ -114,7 +114,7 @@ func (f *Facade) UnitIntroduction(ctx context.Context, args params.CAASUnitIntro
114114
return params.CAASUnitIntroductionResult{}, apiservererrors.ErrPerm
115115
}
116116

117-
var unitName string
117+
var unitName unit.Name
118118
errResp := func(err error) (params.CAASUnitIntroductionResult, error) {
119119
f.logger.Warningf("error introducing k8s pod %q: %v", args.PodName, err)
120120
if errors.Is(err, applicationerrors.ApplicationNotFound) {
@@ -158,8 +158,12 @@ func (f *Facade) UnitIntroduction(ctx context.Context, args params.CAASUnitIntro
158158
if err != nil {
159159
return errResp(err)
160160
}
161-
unitName = fmt.Sprintf("%s/%d", appName, ord)
162-
upsert.UnitName = &unitName
161+
unitName, err = unit.NewNameFromParts(appName, ord)
162+
if err != nil {
163+
return errResp(err)
164+
}
165+
unitNameStr := unitName.String()
166+
upsert.UnitName = &unitNameStr
163167
upsert.OrderedId = ord
164168
upsert.OrderedScale = true
165169
default:
@@ -305,8 +309,12 @@ func (f *Facade) UnitTerminating(ctx context.Context, args params.Entity) (param
305309
if unitTag != tag {
306310
return params.CAASUnitTerminationResult{}, apiservererrors.ErrPerm
307311
}
312+
unitName, err := unit.NewName(unitTag.Id())
313+
if err != nil {
314+
return errResp(err)
315+
}
308316

309-
unitLife, err := f.applicationService.GetUnitLife(ctx, unitTag.Id())
317+
unitLife, err := f.applicationService.GetUnitLife(ctx, unitName)
310318
if err != nil {
311319
return errResp(err)
312320
}

‎apiserver/facades/agent/caasapplication/application_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
apiservertesting "github.com/juju/juju/apiserver/testing"
2121
"github.com/juju/juju/caas"
2222
corecharm "github.com/juju/juju/core/charm"
23+
"github.com/juju/juju/core/unit"
2324
"github.com/juju/juju/domain/application/service"
2425
"github.com/juju/juju/domain/services/testing"
2526
loggertesting "github.com/juju/juju/internal/logger/testing"
@@ -58,7 +59,7 @@ func (s *CAASApplicationSuite) SetUpTest(c *gc.C) {
5859
// Seed the model with an application, this will be used to allow the
5960
// upserting of units.
6061
domainServices := s.DefaultModelDomainServices(c)
61-
unitName := "gitlab/0"
62+
unitName := unit.Name("gitlab/0")
6263
s.applicationService = domainServices.Application(service.ApplicationServiceParams{
6364
StorageRegistry: provider.CommonStorageProviders(),
6465
Secrets: service.NotImplementedSecretService{},

‎apiserver/facades/agent/deployer/deployer.go

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/juju/juju/core/leadership"
1717
"github.com/juju/juju/core/life"
1818
"github.com/juju/juju/core/objectstore"
19+
coreunit "github.com/juju/juju/core/unit"
1920
applicationerrors "github.com/juju/juju/domain/application/errors"
2021
"github.com/juju/juju/rpc/params"
2122
"github.com/juju/juju/state"
@@ -31,9 +32,9 @@ type ControllerConfigGetter interface {
3132

3233
// ApplicationService removes a unit from the dqlite database.
3334
type ApplicationService interface {
34-
GetUnitLife(context.Context, string) (life.Value, error)
35-
EnsureUnitDead(context.Context, string, leadership.Revoker) error
36-
RemoveUnit(context.Context, string, leadership.Revoker) error
35+
GetUnitLife(context.Context, coreunit.Name) (life.Value, error)
36+
EnsureUnitDead(context.Context, coreunit.Name, leadership.Revoker) error
37+
RemoveUnit(context.Context, coreunit.Name, leadership.Revoker) error
3738
}
3839

3940
// DeployerAPI provides access to the Deployer API facade.
@@ -175,9 +176,14 @@ func (d *DeployerAPI) Life(ctx context.Context, args params.Entities) (params.Li
175176
result.Results[i].Error = apiservererrors.ServerError(apiservererrors.ErrPerm)
176177
continue
177178
}
178-
lifeValue, err := d.applicationService.GetUnitLife(ctx, tag.Id())
179+
unitName, err := coreunit.NewName(tag.Id())
180+
if err != nil {
181+
result.Results[i].Error = apiservererrors.ServerError(err)
182+
continue
183+
}
184+
lifeValue, err := d.applicationService.GetUnitLife(ctx, unitName)
179185
if errors.Is(err, applicationerrors.UnitNotFound) {
180-
err = errors.NotFoundf("unit %s", tag.Id())
186+
err = errors.NotFoundf("unit %s", unitName)
181187
}
182188
result.Results[i].Life = lifeValue
183189
result.Results[i].Error = apiservererrors.ServerError(err)
@@ -244,19 +250,24 @@ func (d *DeployerAPI) Remove(ctx context.Context, args params.Entities) (params.
244250
continue
245251
}
246252

253+
unitName, err := coreunit.NewName(tag.Id())
254+
if err != nil {
255+
result.Results[i].Error = apiservererrors.ServerError(err)
256+
continue
257+
}
247258
// Given the way dual write works, we need this for now.
248-
if err = d.applicationService.EnsureUnitDead(ctx, tag.Id(), d.leadershipRevoker); err != nil {
259+
if err = d.applicationService.EnsureUnitDead(ctx, unitName, d.leadershipRevoker); err != nil {
249260
if errors.Is(err, applicationerrors.UnitNotFound) {
250-
err = errors.NotFoundf("unit %s", tag.Id())
261+
err = errors.NotFoundf("unit %s", unitName)
251262
}
252263
result.Results[i].Error = apiservererrors.ServerError(err)
253264
continue
254265
}
255266
// This is the call we will keep once mongo is removed.
256267
// We will need to remove the alive check.
257-
if err = d.applicationService.RemoveUnit(ctx, tag.Id(), d.leadershipRevoker); err != nil {
268+
if err = d.applicationService.RemoveUnit(ctx, unitName, d.leadershipRevoker); err != nil {
258269
if errors.Is(err, applicationerrors.UnitNotFound) {
259-
err = errors.NotFoundf("unit %s", tag.Id())
270+
err = errors.NotFoundf("unit %s", unitName)
260271
}
261272
result.Results[i].Error = apiservererrors.ServerError(err)
262273
continue

‎apiserver/facades/agent/deployer/deployer_test.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/juju/juju/core/life"
2727
"github.com/juju/juju/core/network"
2828
"github.com/juju/juju/core/status"
29+
coreunit "github.com/juju/juju/core/unit"
2930
"github.com/juju/juju/core/watcher/watchertest"
3031
coretesting "github.com/juju/juju/internal/testing"
3132
"github.com/juju/juju/internal/testing/factory"
@@ -42,8 +43,8 @@ type mockLeadershipRevoker struct {
4243
revoked set.Strings
4344
}
4445

45-
func (s *mockLeadershipRevoker) RevokeLeadership(applicationId, unitId string) error {
46-
s.revoked.Add(unitId)
46+
func (s *mockLeadershipRevoker) RevokeLeadership(applicationName string, unitName coreunit.Name) error {
47+
s.revoked.Add(unitName.String())
4748
return nil
4849
}
4950

@@ -262,9 +263,9 @@ func (s *deployerSuite) TestLife(c *gc.C) {
262263
defer s.setupMocks(c).Finish()
263264
s.makeDeployerAPI(c)
264265

265-
s.applicationService.EXPECT().GetUnitLife(gomock.Any(), "mysql/0").Return(life.Alive, nil)
266-
s.applicationService.EXPECT().GetUnitLife(gomock.Any(), "logging/0").Return(life.Dead, nil)
267-
s.applicationService.EXPECT().GetUnitLife(gomock.Any(), "logging/0").Return("", apiservererrors.ErrPerm)
266+
s.applicationService.EXPECT().GetUnitLife(gomock.Any(), coreunit.Name("mysql/0")).Return(life.Alive, nil)
267+
s.applicationService.EXPECT().GetUnitLife(gomock.Any(), coreunit.Name("logging/0")).Return(life.Dead, nil)
268+
s.applicationService.EXPECT().GetUnitLife(gomock.Any(), coreunit.Name("logging/0")).Return("", apiservererrors.ErrPerm)
268269

269270
err := s.subordinate0.EnsureDead()
270271
c.Assert(err, jc.ErrorIsNil)
@@ -318,10 +319,10 @@ func (s *deployerSuite) TestRemove(c *gc.C) {
318319
s.makeDeployerAPI(c)
319320

320321
gomock.InOrder(
321-
s.applicationService.EXPECT().EnsureUnitDead(gomock.Any(), "logging/0", gomock.Any()),
322-
s.applicationService.EXPECT().RemoveUnit(gomock.Any(), "logging/0", gomock.Any()).
323-
DoAndReturn(func(ctx context.Context, unitName string, revoker leadership.Revoker) error {
324-
appName, _ := names.UnitApplication(unitName)
322+
s.applicationService.EXPECT().EnsureUnitDead(gomock.Any(), coreunit.Name("logging/0"), gomock.Any()),
323+
s.applicationService.EXPECT().RemoveUnit(gomock.Any(), coreunit.Name("logging/0"), gomock.Any()).
324+
DoAndReturn(func(ctx context.Context, unitName coreunit.Name, revoker leadership.Revoker) error {
325+
appName, _ := names.UnitApplication(unitName.String())
325326
return revoker.RevokeLeadership(appName, unitName)
326327
}),
327328
)

‎apiserver/facades/agent/deployer/mocks/domain_mock.go

Lines changed: 10 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎apiserver/facades/agent/unitassigner/unitassigner.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/juju/juju/apiserver/facade"
1414
"github.com/juju/juju/core/machine"
1515
"github.com/juju/juju/core/network"
16+
"github.com/juju/juju/core/unit"
1617
machineerrors "github.com/juju/juju/domain/machine/errors"
1718
internalerrors "github.com/juju/juju/internal/errors"
1819
"github.com/juju/juju/rpc/params"
@@ -55,7 +56,7 @@ type StubService interface {
5556
//
5657
// Deprecated: AssignUnitsToMachines will become redundant once the machine and
5758
// application domains have become fully implemented.
58-
AssignUnitsToMachines(context.Context, map[string][]string) error
59+
AssignUnitsToMachines(context.Context, map[string][]unit.Name) error
5960
}
6061

6162
// API implements the functionality for assigning units to machines.
@@ -95,7 +96,7 @@ func (a *API) AssignUnits(ctx context.Context, args params.Entities) (params.Err
9596
return result, apiservererrors.ServerError(err)
9697
}
9798

98-
machineToUnitMap := make(map[string][]string)
99+
machineToUnitMap := make(map[string][]unit.Name)
99100

100101
// The results come back from state in an undetermined order and do not
101102
// include results for units that were not found, so we have to make up for
@@ -116,7 +117,7 @@ func (a *API) AssignUnits(ctx context.Context, args params.Entities) (params.Err
116117
if err := a.saveMachineInfo(ctx, machineId); err != nil {
117118
resultMap[r.Unit] = err
118119
}
119-
machineToUnitMap[machineId] = append(machineToUnitMap[machineId], r.Unit)
120+
machineToUnitMap[machineId] = append(machineToUnitMap[machineId], unit.Name(r.Unit))
120121
}
121122

122123
// Assign units to machines via net nodes.

‎apiserver/facades/agent/unitassigner/unitassigner_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/juju/juju/apiserver/common"
1313
"github.com/juju/juju/core/machine"
1414
"github.com/juju/juju/core/network"
15+
"github.com/juju/juju/core/unit"
1516
"github.com/juju/juju/rpc/params"
1617
"github.com/juju/juju/state"
1718
)
@@ -26,7 +27,7 @@ func (testsuite) TestAssignUnits(c *gc.C) {
2627
}
2728
f.results = []state.UnitAssignmentResult{{Unit: "foo/0"}}
2829
machineService := &fakeMachineService{}
29-
stubService := &fakeStubService{assignments: map[string][]string{}}
30+
stubService := &fakeStubService{assignments: map[string][]unit.Name{}}
3031
api := API{
3132
st: f,
3233
res: common.NewResources(),
@@ -43,7 +44,7 @@ func (testsuite) TestAssignUnits(c *gc.C) {
4344
c.Assert(res.Results[0].Error, gc.IsNil)
4445
c.Assert(res.Results[1].Error, gc.ErrorMatches, `unit "unit-bar-1" not found`)
4546
c.Assert(machineService.machineNames, jc.SameContents, []machine.Name{machine.Name("1"), machine.Name("1/lxd/2")})
46-
c.Assert(stubService.assignments, jc.DeepEquals, map[string][]string{"1/lxd/2": {"foo/0"}})
47+
c.Assert(stubService.assignments, jc.DeepEquals, map[string][]unit.Name{"1/lxd/2": {"foo/0"}})
4748
}
4849

4950
func (testsuite) TestWatchUnitAssignment(c *gc.C) {
@@ -88,10 +89,10 @@ func (f *fakeNetworkService) GetAllSpaces(_ context.Context) (network.SpaceInfos
8889
}
8990

9091
type fakeStubService struct {
91-
assignments map[string][]string
92+
assignments map[string][]unit.Name
9293
}
9394

94-
func (f *fakeStubService) AssignUnitsToMachines(_ context.Context, machineToUnitMap map[string][]string) error {
95+
func (f *fakeStubService) AssignUnitsToMachines(_ context.Context, machineToUnitMap map[string][]unit.Name) error {
9596
for machine, units := range machineToUnitMap {
9697
f.assignments[machine] = append(f.assignments[machine], units...)
9798
}

0 commit comments

Comments
 (0)