Skip to content

Commit

Permalink
Merge pull request #8920 from SimonRichardson/default-region
Browse files Browse the repository at this point in the history
#8920

## Description of change

The following changes, inserts a default region to the cloud spec, if the cloud spec name is not localhost and if the regions are empty. In this way we can then report back stating that the region is not localhost and can be identified as a different type.

If regions do end up being added to lxd, this insertion of the default region should not happen and user specified regions will be add accordingly. 

## QA steps

1. Add a cloud using lxd
2. Add credentials accordingly
3. Bootstrap a lxd remote cluster

You should see that the region is set to "default".
Consider the `juju status` output:
```sh
$ juju status
Model Controller Cloud/Region Version SLA Timestamp
default lxd-default lxd/default 2.5-beta1.1 unsupported 14:41:42+01:00
```

## Documentation changes

No changes to the user CLI, but internally we assign a default region to lxd if it's not localhost and there are no regions.

## Bug reference

N/A
  • Loading branch information
jujubot committed Jul 12, 2018
2 parents d7a27f7 + d3e42ea commit d79c171
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 9 deletions.
4 changes: 3 additions & 1 deletion provider/lxd/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"os"
"path/filepath"

"github.com/juju/juju/provider/lxd/lxdnames"

"github.com/juju/errors"
"github.com/juju/utils"
"github.com/lxc/lxd/shared"
Expand Down Expand Up @@ -115,7 +117,7 @@ func (p environProviderCredentials) DetectCredentials() (*cloud.CloudCredential,
return nil, errors.Trace(err)
}

const credName = "localhost"
const credName = lxdnames.DefaultCloud
label := fmt.Sprintf("LXD credential %q", credName)
certCredential, err := p.finalizeLocalCertificateCredential(
ioutil.Discard, svr, string(certPEM), string(keyPEM), label,
Expand Down
8 changes: 6 additions & 2 deletions provider/lxd/lxdnames/names.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@ package lxdnames
// the local lxd daemon.
const DefaultCloud = "localhost"

// DefaultRegion is the name of the only "region" we support in lxd currently,
// DefaultLocalRegion is the name of the "region" we support in a local lxd,
// which corresponds to the local lxd daemon.
const DefaultRegion = "localhost"
const DefaultLocalRegion = "localhost"

// DefaultRemoteRegion is the name of the "region" we report if there are no
// other regions for a remote lxd server.
const DefaultRemoteRegion = "default"

// ProviderType defines the provider/cloud type for lxd.
const ProviderType = "lxd"
19 changes: 14 additions & 5 deletions provider/lxd/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func (p *environProvider) DetectCloud(name string) (cloud.Cloud, error) {
// i.e. the local LXD daemon. We may later want to detect
// locally-configured remotes.
switch name {
case "lxd", "localhost":
case lxdnames.ProviderType, lxdnames.DefaultCloud:
return localhostCloud, nil
}
return cloud.Cloud{}, errors.NotFoundf("cloud %s", name)
Expand All @@ -164,12 +164,11 @@ func (p *environProvider) FinalizeCloud(
ctx environs.FinalizeCloudContext,
in cloud.Cloud,
) (cloud.Cloud, error) {

var endpoint string
resolveEndpoint := func(name string, ep *string) error {
// If the name doesn't equal "localhost" then we shouldn't resolve
// the end point, instead we should just accept what we already have.
if name != "localhost" || *ep != "" {
if name != lxdnames.DefaultCloud || *ep != "" {
return nil
}
if endpoint == "" {
Expand All @@ -186,6 +185,8 @@ func (p *environProvider) FinalizeCloud(
return nil
}

// If any of the endpoints point to localhost, go through and backfill the
// cloud spec with local host addresses.
if err := resolveEndpoint(in.Name, &in.Endpoint); err != nil {
return cloud.Cloud{}, errors.Trace(err)
}
Expand All @@ -194,6 +195,14 @@ func (p *environProvider) FinalizeCloud(
return cloud.Cloud{}, errors.Trace(err)
}
}
// If the provider type is not named localhost and there is no region, set the
// region to be a default region
if in.Name != lxdnames.DefaultCloud && len(in.Regions) == 0 {
in.Regions = append(in.Regions, cloud.Region{
Name: lxdnames.DefaultRemoteRegion,
Endpoint: in.Endpoint,
})
}
return in, nil
}

Expand Down Expand Up @@ -228,7 +237,7 @@ var localhostCloud = cloud.Cloud{
},
Endpoint: "",
Regions: []cloud.Region{{
Name: lxdnames.DefaultRegion,
Name: lxdnames.DefaultLocalRegion,
Endpoint: "",
}},
Description: cloud.DefaultCloudDescription(lxdnames.ProviderType),
Expand All @@ -239,7 +248,7 @@ func (*environProvider) DetectRegions() ([]cloud.Region, error) {
// For now we just return a hard-coded "localhost" region,
// i.e. the local LXD daemon. We may later want to detect
// locally-configured remotes.
return []cloud.Region{{Name: lxdnames.DefaultRegion}}, nil
return []cloud.Region{{Name: lxdnames.DefaultLocalRegion}}, nil
}

// Schema returns the configuration schema for an environment.
Expand Down
33 changes: 32 additions & 1 deletion provider/lxd/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,37 @@ func (s *providerSuite) TestFinalizeCloudWithRemoteProviderWithMixedRegions(c *g
})
}

func (s *providerSuite) TestFinalizeCloudWithRemoteProviderWithNoRegion(c *gc.C) {
ctrl := gomock.NewController(c)
defer ctrl.Finish()

provider, _, _ := s.createProvider(ctrl)
cloudFinalizer := provider.(environs.CloudFinalizer)

cloudSpec := cloud.Cloud{
Name: "test",
Type: "lxd",
Endpoint: "https://192.0.0.1:8443",
AuthTypes: []cloud.AuthType{cloud.CertificateAuthType},
Regions: []cloud.Region{},
}

ctx := testing.NewMockFinalizeCloudContext(ctrl)

got, err := cloudFinalizer.FinalizeCloud(ctx, cloudSpec)
c.Assert(err, jc.ErrorIsNil)
c.Assert(got, gc.DeepEquals, cloud.Cloud{
Name: "test",
Type: "lxd",
Endpoint: "https://192.0.0.1:8443",
AuthTypes: []cloud.AuthType{cloud.CertificateAuthType},
Regions: []cloud.Region{{
Name: "default",
Endpoint: "https://192.0.0.1:8443",
}},
})
}

func (s *providerSuite) TestFinalizeCloudNotListening(c *gc.C) {
if !containerLXD.HasSupport() {
c.Skip("To be rewritten during LXD code refactoring for cluster support")
Expand Down Expand Up @@ -276,7 +307,7 @@ func (s *providerSuite) TestFinalizeCloudAlreadyListeningHTTPS(c *gc.C) {
func (s *providerSuite) TestDetectRegions(c *gc.C) {
regions, err := s.Provider.DetectRegions()
c.Assert(err, jc.ErrorIsNil)
c.Assert(regions, jc.DeepEquals, []cloud.Region{{Name: lxdnames.DefaultRegion}})
c.Assert(regions, jc.DeepEquals, []cloud.Region{{Name: lxdnames.DefaultLocalRegion}})
}

func (s *providerSuite) TestValidate(c *gc.C) {
Expand Down

0 comments on commit d79c171

Please sign in to comment.