Skip to content

Commit

Permalink
Merge pull request juju#16850 from manadart/dqlite-endpoint-bindings-…
Browse files Browse the repository at this point in the history
…minus-default-space

juju#16850

Functionality for endpoint bindings uses a method in state called `DefaultEndpointBindingSpace`, which takes the space name configured for `default-binding` in model configuration, and looks up its ID.

Here, we add some vice code to remove the lookup, ensuring that we always just use the `alpha` space ID.

This eases the work of migrating the space domain to Dqlite.

## QA steps

Passing tests.

## Documentation changes

No, but noted in [risk register](https://docs.google.com/spreadsheets/d/1vSd3s0asNtpENdeC3tPkEo5mDja4inS5KwjCFxhyE5I).

## Links

**Jira card:** JUJU-5367
  • Loading branch information
jujubot committed Jan 29, 2024
2 parents 7565c2c + 465dbd5 commit 21799c5
Show file tree
Hide file tree
Showing 11 changed files with 17 additions and 143 deletions.
1 change: 0 additions & 1 deletion apiserver/common/modelmanagerinterface.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ type ModelManagerBackend interface {
AddSpace(string, network.Id, []string) (*state.Space, error)
AllEndpointBindingsSpaceNames() (set.Strings, error)
ConstraintsBySpaceName(string) ([]*state.Constraints, error)
DefaultEndpointBindingSpace() (string, error)
// TODO(nvinuesa): This method is necessary only until the spaces
// migration to dqlite is finished:
Space(id string) (*state.Space, error)
Expand Down
1 change: 0 additions & 1 deletion apiserver/facades/client/application/deployrepository.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ type DeployFromRepositoryState interface {
services.StateBackend

network.SpaceLookup
DefaultEndpointBindingSpace() (string, error)
Space(id string) (*state.Space, error)
}

Expand Down

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

15 changes: 0 additions & 15 deletions apiserver/facades/client/application/mocks/application_mock.go

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

15 changes: 0 additions & 15 deletions apiserver/facades/client/spaces/package_mock_test.go

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

11 changes: 4 additions & 7 deletions environs/space/spaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ type ReloadSpacesState interface {
// ConstraintsBySpaceName returns all Constraints that include a positive
// or negative space constraint for the input space name.
ConstraintsBySpaceName(string) ([]Constraints, error)
// DefaultEndpointBindingSpace returns the current space ID to be used for
// the default endpoint binding.
DefaultEndpointBindingSpace() (string, error)
// AllEndpointBindingsSpaceNames returns a set of spaces names for all the
// endpoint bindings.
AllEndpointBindingsSpaceNames() (set.Strings, error)
Expand Down Expand Up @@ -185,10 +182,10 @@ func (s *ProviderSpaces) DeleteSpaces() ([]string, error) {
return nil, nil
}

defaultEndpointBinding, err := s.state.DefaultEndpointBindingSpace()
if err != nil {
return nil, errors.Trace(err)
}
// TODO (manadart 2024-01-29): The alpha space ID here is scaffolding and
// should be replaced with the configured model default space upon
// migrating this logic to Dqlite.
defaultEndpointBinding := network.AlphaSpaceId

allEndpointBindings, err := s.state.AllEndpointBindingsSpaceNames()
if err != nil {
Expand Down
15 changes: 0 additions & 15 deletions environs/space/spaces_mock_test.go

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

8 changes: 2 additions & 6 deletions environs/space/spaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ func (s *providerSpacesSuite) TestDeleteSpaces(c *gc.C) {
defer ctrl.Finish()

mockState := NewMockReloadSpacesState(ctrl)
mockState.EXPECT().DefaultEndpointBindingSpace().Return("2", nil)
mockState.EXPECT().AllEndpointBindingsSpaceNames().Return(set.NewStrings(), nil)
mockState.EXPECT().ConstraintsBySpaceName("1").Return(nil, nil)
mockState.EXPECT().Remove("1").Return(nil)
Expand All @@ -229,7 +228,6 @@ func (s *providerSpacesSuite) TestDeleteSpacesMatchesAlphaSpace(c *gc.C) {
defer ctrl.Finish()

mockState := NewMockReloadSpacesState(ctrl)
mockState.EXPECT().DefaultEndpointBindingSpace().Return("1", nil)
mockState.EXPECT().AllEndpointBindingsSpaceNames().Return(set.NewStrings(), nil)

provider := NewProviderSpaces(mockState)
Expand All @@ -248,11 +246,12 @@ func (s *providerSpacesSuite) TestDeleteSpacesMatchesAlphaSpace(c *gc.C) {
}

func (s *providerSpacesSuite) TestDeleteSpacesMatchesDefaultBindingSpace(c *gc.C) {
c.Skip("The default space is always alpha due to scaffolding in service of Dqlite migration.")

ctrl := gomock.NewController(c)
defer ctrl.Finish()

mockState := NewMockReloadSpacesState(ctrl)
mockState.EXPECT().DefaultEndpointBindingSpace().Return("1", nil)
mockState.EXPECT().AllEndpointBindingsSpaceNames().Return(set.NewStrings(), nil)

provider := NewProviderSpaces(mockState)
Expand All @@ -275,7 +274,6 @@ func (s *providerSpacesSuite) TestDeleteSpacesContainedInAllEndpointBindings(c *
defer ctrl.Finish()

mockState := NewMockReloadSpacesState(ctrl)
mockState.EXPECT().DefaultEndpointBindingSpace().Return("2", nil)
mockState.EXPECT().AllEndpointBindingsSpaceNames().Return(set.NewStrings("1"), nil)

provider := NewProviderSpaces(mockState)
Expand All @@ -298,7 +296,6 @@ func (s *providerSpacesSuite) TestDeleteSpacesContainsConstraintsSpace(c *gc.C)
defer ctrl.Finish()

mockState := NewMockReloadSpacesState(ctrl)
mockState.EXPECT().DefaultEndpointBindingSpace().Return("2", nil)
mockState.EXPECT().AllEndpointBindingsSpaceNames().Return(set.NewStrings(), nil)
mockState.EXPECT().ConstraintsBySpaceName("1").Return([]Constraints{struct{}{}}, nil)

Expand Down Expand Up @@ -358,7 +355,6 @@ func (s *providerSpacesSuite) TestProviderSpacesRun(c *gc.C) {
},
})

mockState.EXPECT().DefaultEndpointBindingSpace().Return("2", nil)
mockState.EXPECT().AllEndpointBindingsSpaceNames().Return(set.NewStrings(), nil)
mockState.EXPECT().ConstraintsBySpaceName("space1").Return(nil, nil)

Expand Down
49 changes: 10 additions & 39 deletions state/endpoint_bindings.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,10 @@ func (b *Bindings) Merge(mergeWith map[string]string, meta *charm.Meta) (bool, e

defaultBinding, mergeOK := b.bindingsMap[defaultEndpointName]
if !mergeOK {
var err error
defaultBinding, err = b.st.DefaultEndpointBindingSpace()
if err != nil {
return false, errors.Trace(err)
}
// TODO (manadart 2024-01-29): The alpha space ID here is scaffolding and
// should be replaced with the configured model default space upon
// migrating this logic to Dqlite.
defaultBinding = network.AlphaSpaceId
}
if newDefaultBinding, newOk := mergeMap[defaultEndpointName]; newOk {
// new default binding supersedes the old default binding
Expand Down Expand Up @@ -447,47 +446,20 @@ func (b *Bindings) validateForCharm(charmMeta *charm.Meta) error {
return nil
}

// DefaultEndpointBindingSpace returns the current space ID to be used for
// the default endpoint binding.
func (st *State) DefaultEndpointBindingSpace() (string, error) {
model, err := st.Model()
if err != nil {
return "", errors.Trace(err)
}

cfg, err := model.Config()
if err != nil {
return "", errors.Trace(err)
}

defaultBinding := network.AlphaSpaceId

space, err := st.SpaceByName(cfg.DefaultSpace())
if err != nil && !errors.Is(err, errors.NotFound) {
return "", errors.Trace(err)
}
if err == nil {
defaultBinding = space.Id()
}

return defaultBinding, nil
}

// DefaultEndpointBindingsForCharm populates a bindings map containing each
// endpoint of the given charm metadata (relation name or extra-binding name)
// bound to an empty space.
func DefaultEndpointBindingsForCharm(st EndpointBinding, charmMeta *charm.Meta) (map[string]string, error) {
defaultBindingSpaceID, err := st.DefaultEndpointBindingSpace()
if err != nil {
return nil, err
}
// TODO (manadart 2024-01-29): The alpha space ID here is scaffolding and
// should be replaced with the configured model default space upon
// migrating this logic to Dqlite.
func DefaultEndpointBindingsForCharm(_ EndpointBinding, charmMeta *charm.Meta) (map[string]string, error) {
allRelations := charmMeta.CombinedRelations()
bindings := make(map[string]string, len(allRelations)+len(charmMeta.ExtraBindings))
for name := range allRelations {
bindings[name] = defaultBindingSpaceID
bindings[name] = network.AlphaSpaceId
}
for name := range charmMeta.ExtraBindings {
bindings[name] = defaultBindingSpaceID
bindings[name] = network.AlphaSpaceId
}
return bindings, nil
}
Expand All @@ -498,7 +470,6 @@ func DefaultEndpointBindingsForCharm(st EndpointBinding, charmMeta *charm.Meta)
//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/endpointbinding_mock.go github.com/juju/juju/state EndpointBinding
type EndpointBinding interface {
network.SpaceLookup
DefaultEndpointBindingSpace() (string, error)
Space(id string) (*Space, error)
}

Expand Down
15 changes: 1 addition & 14 deletions state/endpoint_bindings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ func (s *bindingsSuite) TestMergeBindings(c *gc.C) {
}

func (s *bindingsSuite) TestMergeWithModelConfigNonDefaultSpace(c *gc.C) {
c.Skip("The default space is always alpha due to scaffolding in service of Dqlite migration.")
err := s.Model.UpdateModelConfig(map[string]interface{}{"default-space": s.appsSpace.Name()}, nil)
c.Assert(err, jc.ErrorIsNil)

Expand All @@ -310,20 +311,6 @@ func (s *bindingsSuite) TestMergeWithModelConfigNonDefaultSpace(c *gc.C) {
c.Check(isModified, gc.Equals, true)
}

func (s *bindingsSuite) TestDefaultEndpointBindingSpaceNotDefault(c *gc.C) {
err := s.Model.UpdateModelConfig(map[string]interface{}{"default-space": s.clientSpace.Name()}, nil)
c.Assert(err, jc.ErrorIsNil)
id, err := s.State.DefaultEndpointBindingSpace()
c.Assert(err, jc.ErrorIsNil)
c.Assert(id, gc.Equals, s.clientSpace.Id())
}

func (s *bindingsSuite) TestDefaultEndpointBindingSpaceDefault(c *gc.C) {
id, err := s.State.DefaultEndpointBindingSpace()
c.Assert(err, jc.ErrorIsNil)
c.Assert(id, gc.Equals, network.AlphaSpaceId)
}

func (s *bindingsSuite) copyMap(input map[string]string) map[string]string {
output := make(map[string]string, len(input))
for key, value := range input {
Expand Down
15 changes: 0 additions & 15 deletions state/mocks/endpointbinding_mock.go

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

0 comments on commit 21799c5

Please sign in to comment.