juju-core

Merge lp:~rogpeppe/juju-core/454-1.16-maas-changes into lp:juju-core/1.16

  1. 454-1.16-maas-changes
  2. Merge into 1.16
Proposed by Roger Peppe
Status: Merged
Approved by: Roger Peppe
Approved revision: no longer in the source branch.
Merged at revision: 1974
Proposed branch: lp:~rogpeppe/juju-core/454-1.16-maas-changes
Merge into: lp:juju-core/1.16
Diff against target: 935 lines (+267/-85)
10 files modified
provider/maas/config.go (+22/-2)
provider/maas/config_test.go (+33/-3)
provider/maas/environ.go (+4/-2)
provider/maas/environ_test.go (+36/-36)
provider/maas/environprovider.go (+18/-1)
provider/maas/environprovider_test.go (+44/-3)
provider/maas/instance_test.go (+15/-13)
provider/maas/maas_test.go (+24/-2)
provider/maas/storage.go (+22/-2)
provider/maas/storage_test.go (+49/-21)
To merge this branch: bzr merge lp:~rogpeppe/juju-core/454-1.16-maas-changes
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+191413@code.launchpad.net

Commit message

provider/maas: instance and storage uniqueness

This merges the following branches from trunk.
Fixes https://launchpad.net/bugs/1229275

------------------------------------------------------------
revno: 1990 [merge]
author: Roger Peppe <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Wed 2013-10-16 13:12:52 +0000
message:
  [r=rogpeppe] provider/maas: do not use environ-uuid

  As pointed out by fwereade, it could be
  confusing to have two things in the system
  that both purport to be a uuid for the environ.

  Also make the MAAS EnvironProvider.Validate
  check that we're not changing the maas-agent-name.

  https://codereview.appspot.com/14741045/
------------------------------------------------------------

revno: 1989 [merge]
author: Julian Edwards <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Wed 2013-10-16 06:11:50 +0000
message:
  [r=axwalk],[bug=1081247],[bug=1229275] Use the environment's UUID as a file prefix for all files in MAAS private storage to prevent different environments from overlapping.
------------------------------------------------------------

------------------------------------------------------------
revno: 1985 [merge]
author: Gavin Panella <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Wed 2013-10-16 01:56:42 +0000
message:
  [r=wallyworld] Use environment-uuid when interacting with MAAS

  The environment-uuid is passed - as agent_name - to MAAS when acquiring
  and listing nodes. This will allow multiple Juju environments to coexist
  in a single MAAS user's account.

  In addition, it prevents environment-uuid from being set in
  environments.yaml, and returns an appropriate error message. This is
  intended to stop people from inadvertently clobbering existing
  environments.

  It also starts cleaning up MAAS's providerSuite by dropping the environ
  attribute in favour of makeEnviron(). The former was initialised to a
  half-working environ that caused strange test failures.

  https://codereview.appspot.com/14644045/
------------------------------------------------------------
revno: 1984 [merge]
author: Gavin Panella <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Tue 2013-10-15 17:03:22 +0000
message:
  [r=gz] provider/maas: Add environment UUID

  We need a way to uniquely identify those machines in MAAS belonging to
  a Juju environment. Until now Juju's MAAS provider has assumed that
  all the machines that are allocated, are allocated to the environment
  that Juju is working with. This is obviously wrong, and can be
  disastrous.

  This is the first part of work on the MAAS provider to use a unique
  identifier for an environment. MAAS will record this identifier
  (called 'agent_name' in MAAS) and allow its use for filtering.
  This branch merely adds the UUID to the environment's config.

  https://codereview.appspot.com/14696043/

  R=rogpeppe, rvb

https://codereview.appspot.com/14746044/

Description of the change

provider/maas: instance and storage uniqueness

This merges the following branches from trunk.
Fixes https://launchpad.net/bugs/1229275

------------------------------------------------------------
revno: 1990 [merge]
author: Roger Peppe <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Wed 2013-10-16 13:12:52 +0000
message:
  [r=rogpeppe] provider/maas: do not use environ-uuid

  As pointed out by fwereade, it could be
  confusing to have two things in the system
  that both purport to be a uuid for the environ.

  Also make the MAAS EnvironProvider.Validate
  check that we're not changing the maas-agent-name.

  https://codereview.appspot.com/14741045/
------------------------------------------------------------

revno: 1989 [merge]
author: Julian Edwards <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Wed 2013-10-16 06:11:50 +0000
message:
  [r=axwalk],[bug=1081247],[bug=1229275] Use the environment's UUID as a file prefix for all files in MAAS private storage to prevent different environments from overlapping.
------------------------------------------------------------

------------------------------------------------------------
revno: 1985 [merge]
author: Gavin Panella <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Wed 2013-10-16 01:56:42 +0000
message:
  [r=wallyworld] Use environment-uuid when interacting with MAAS

  The environment-uuid is passed - as agent_name - to MAAS when acquiring
  and listing nodes. This will allow multiple Juju environments to coexist
  in a single MAAS user's account.

  In addition, it prevents environment-uuid from being set in
  environments.yaml, and returns an appropriate error message. This is
  intended to stop people from inadvertently clobbering existing
  environments.

  It also starts cleaning up MAAS's providerSuite by dropping the environ
  attribute in favour of makeEnviron(). The former was initialised to a
  half-working environ that caused strange test failures.

  https://codereview.appspot.com/14644045/
------------------------------------------------------------
revno: 1984 [merge]
author: Gavin Panella <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Tue 2013-10-15 17:03:22 +0000
message:
  [r=gz] provider/maas: Add environment UUID

  We need a way to uniquely identify those machines in MAAS belonging to
  a Juju environment. Until now Juju's MAAS provider has assumed that
  all the machines that are allocated, are allocated to the environment
  that Juju is working with. This is obviously wrong, and can be
  disastrous.

  This is the first part of work on the MAAS provider to use a unique
  identifier for an environment. MAAS will record this identifier
  (called 'agent_name' in MAAS) and allow its use for filtering.
  This branch merely adds the UUID to the environment's config.

  https://codereview.appspot.com/14696043/

  R=rogpeppe, rvb

https://codereview.appspot.com/14746044/

To post a comment you must log in.
Revision history for this message
Roger Peppe (rogpeppe) wrote :
Download full text (3.6 KiB)

Reviewers: mp+191413_code.launchpad.net,

Message:
Please take a look.

Description:
provider/maas: instance and storage uniqueness

This merges the following branches from trunk.
Fixes https://launchpad.net/bugs/1229275

------------------------------------------------------------
revno: 1990 [merge]
author: Roger Peppe <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Wed 2013-10-16 13:12:52 +0000
message:
   [r=rogpeppe] provider/maas: do not use environ-uuid

   As pointed out by fwereade, it could be
   confusing to have two things in the system
   that both purport to be a uuid for the environ.

   Also make the MAAS EnvironProvider.Validate
   check that we're not changing the maas-agent-name.

   https://codereview.appspot.com/14741045/
------------------------------------------------------------

revno: 1989 [merge]
author: Julian Edwards <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Wed 2013-10-16 06:11:50 +0000
message:
   [r=axwalk],[bug=1081247],[bug=1229275] Use the environment's UUID as a
file prefix for all files in MAAS private storage to prevent different
environments from overlapping.
------------------------------------------------------------

------------------------------------------------------------
revno: 1985 [merge]
author: Gavin Panella <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Wed 2013-10-16 01:56:42 +0000
message:
   [r=wallyworld] Use environment-uuid when interacting with MAAS

   The environment-uuid is passed - as agent_name - to MAAS when
acquiring
   and listing nodes. This will allow multiple Juju environments to
coexist
   in a single MAAS user's account.

   In addition, it prevents environment-uuid from being set in
   environments.yaml, and returns an appropriate error message. This is
   intended to stop people from inadvertently clobbering existing
   environments.

   It also starts cleaning up MAAS's providerSuite by dropping the
environ
   attribute in favour of makeEnviron(). The former was initialised to a
   half-working environ that caused strange test failures.

   https://codereview.appspot.com/14644045/
------------------------------------------------------------
revno: 1984 [merge]
author: Gavin Panella <email address hidden>
committer: Tarmac
branch nick: juju-core
timestamp: Tue 2013-10-15 17:03:22 +0000
message:
   [r=gz] provider/maas: Add environment UUID

   We need a way to uniquely identify those machines in MAAS belonging to
   a Juju environment. Until now Juju's MAAS provider has assumed that
   all the machines that are allocated, are allocated to the environment
   that Juju is working with. This is obviously wrong, and can be
   disastrous.

   This is the first part of work on the MAAS provider to use a unique
   identifier for an environment. MAAS will record this identifier
   (called 'agent_name' in MAAS) and allow its use for filtering.
   This branch merely adds the UUID to the environment's config.

   https://codereview.appspot.com/14696043/

   R=rogpeppe, rvb

https://code.launchpad.net/~rogpeppe/juju-core/454-1.16-maas-changes/+merge/191413

(do not edit description out o...

Read more...

Revision history for this message
Frank Mueller (themue) wrote :

LGTM, only optical comments.

https://codereview.appspot.com/14746044/diff/3001/provider/maas/storage_test.go
File provider/maas/storage_test.go (right):

https://codereview.appspot.com/14746044/diff/3001/provider/maas/storage_test.go#newcode401
provider/maas/storage_test.go:401: func (s *storageSuite)
TestprefixWithPrivateNamespacePrefixesWithAgentName(c *gc.C) {
Testprefix... instead of TestPrefix...?

https://codereview.appspot.com/14746044/diff/3001/provider/maas/storage_test.go#newcode412
provider/maas/storage_test.go:412: func (s *storageSuite)
TesttprefixWithPrivateNamespaceIgnoresAgentName(c *gc.C) {
Ditto.

https://codereview.appspot.com/14746044/

Revision history for this message
Raphaël Badin (rvb) wrote :

On 2013/10/16 13:42:08, rog wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/14746044/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (16.1 KiB)

The attempt to merge lp:~rogpeppe/juju-core/454-1.16-maas-changes into lp:juju-core/1.16 failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 0.350s
ok launchpad.net/juju-core/agent/tools 0.279s
ok launchpad.net/juju-core/bzr 9.483s
ok launchpad.net/juju-core/cert 3.225s
ok launchpad.net/juju-core/charm 0.826s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.028s
ok launchpad.net/juju-core/cmd 0.258s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 195.074s
ok launchpad.net/juju-core/cmd/jujud 49.124s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 2.436s
ok launchpad.net/juju-core/constraints 0.028s
ok launchpad.net/juju-core/container/lxc 0.293s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.303s
ok launchpad.net/juju-core/environs 3.157s
ok launchpad.net/juju-core/environs/bootstrap 5.241s
ok launchpad.net/juju-core/environs/cloudinit 0.569s
ok launchpad.net/juju-core/environs/config 0.737s
ok launchpad.net/juju-core/environs/configstore 0.095s
ok launchpad.net/juju-core/environs/filestorage 0.030s
ok launchpad.net/juju-core/environs/httpstorage 1.028s
ok launchpad.net/juju-core/environs/imagemetadata 0.521s
ok launchpad.net/juju-core/environs/instances 0.054s
ok launchpad.net/juju-core/environs/jujutest 0.286s
ok launchpad.net/juju-core/environs/manual 5.507s
ok launchpad.net/juju-core/environs/simplestreams 0.329s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.107s
ok launchpad.net/juju-core/environs/storage 1.273s
ok launchpad.net/juju-core/environs/sync 32.435s
ok launchpad.net/juju-core/environs/testing 0.259s
ok launchpad.net/juju-core/environs/tools 7.349s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.016s
ok launchpad.net/juju-core/instance 0.025s
? launchpad.net/juju-core/instance/testing [no test files]

----------------------------------------------------------------------
FAIL: apiconn_test.go:0: DeployLocalSuite.TearDownTest

Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waiting for sockets to die: 0 in use, 1 alive
Waitin...

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (51.2 KiB)

The attempt to merge lp:~rogpeppe/juju-core/454-1.16-maas-changes into lp:juju-core/1.16 failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 0.410s
ok launchpad.net/juju-core/agent/tools 0.230s
ok launchpad.net/juju-core/bzr 8.515s
ok launchpad.net/juju-core/cert 3.834s
ok launchpad.net/juju-core/charm 0.631s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.027s
ok launchpad.net/juju-core/cmd 0.214s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 193.156s

----------------------------------------------------------------------
FAIL: machine_test.go:262: MachineSuite.TestManageEnviron

[LOG] 60.43808 DEBUG juju.environs.configstore Making /tmp/gocheck-6842348953158377901/11/home/ubuntu/.juju/environments
[LOG] 60.54012 INFO juju environs/testing: uploading FAKE tools 1.16.0-precise-amd64
[LOG] 60.56811 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 60.56814 DEBUG juju.environs.tools no series specified when finding tools, looking for any
[LOG] 60.56823 DEBUG juju.environs.simplestreams fetchData failed for "http://127.0.0.1:48669/dummyenv/private/tools/streams/v1/index.sjson": file "tools/streams/v1/index.sjson" not found not found
[LOG] 60.56828 DEBUG juju.environs.simplestreams cannot load index "http://127.0.0.1:48669/dummyenv/private/tools/streams/v1/index.sjson": invalid URL "http://127.0.0.1:48669/dummyenv/private/tools/streams/v1/index.sjson" not found
[LOG] 60.56833 DEBUG juju.environs.simplestreams fetchData failed for "http://127.0.0.1:48669/dummyenv/private/tools/streams/v1/index.json": file "tools/streams/v1/index.json" not found not found
[LOG] 60.56838 DEBUG juju.environs.simplestreams cannot load index "http://127.0.0.1:48669/dummyenv/private/tools/streams/v1/index.json": invalid URL "http://127.0.0.1:48669/dummyenv/private/tools/streams/v1/index.json" not found
[LOG] 60.56865 INFO juju.environs.tools Writing tools/streams/v1/index.json
[LOG] 60.56883 INFO juju.environs.tools Writing tools/streams/v1/com.ubuntu.juju:released:tools.json
[LOG] 60.56888 INFO juju.environs.boostrap bootstrapping environment "dummyenv"
[LOG] 60.56892 INFO juju.environs.tools reading tools with major.minor version 1.16
[LOG] 60.56894 INFO juju.environs.tools filtering tools by version: 1.16.0
[LOG] 60.56896 INFO juju.environs.tools filtering tools by series: precise
[LOG] 60.56900 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 60.56906 DEBUG juju.environs.simplestreams fetchData failed for "http://127.0.0.1:48669/dummyenv/private/tools/streams/v1/index.sjson": file "tools/streams/v1/index.sjson" not found not found
[LOG] 60.56911 DEBUG juju.environs.simplestreams cannot load index "http://127.0.0.1:48669/dummyenv/private/tools/streams/v1/index.sjson": invalid URL "http://127.0.0.1:48669/dummyenv/private/tools/streams/v1/index.sjson" not found
[LOG] 60.56942 DEBUG juju.environs.simplestreams no mirror information available for {DataType:content-download MirrorConten...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'provider/maas/config.go'
2--- provider/maas/config.go 2013-08-09 04:58:34 +0000
3+++ provider/maas/config.go 2013-10-16 13:44:41 +0000
4@@ -18,8 +18,15 @@
5 // maas-oauth is a colon-separated triplet of:
6 // consumer-key:resource-token:resource-secret
7 "maas-oauth": schema.String(),
8-}
9-var configDefaults = schema.Defaults{}
10+ // maas-agent-name is an optional UUID to group the instances
11+ // acquired from MAAS, to support multiple environments per MAAS user.
12+ "maas-agent-name": schema.String(),
13+}
14+var configDefaults = schema.Defaults{
15+ // For backward-compatibility, maas-agent-name is the empty string
16+ // by default. However, new environments should all use a UUID.
17+ "maas-agent-name": "",
18+}
19
20 type maasEnvironConfig struct {
21 *config.Config
22@@ -34,6 +41,13 @@
23 return cfg.attrs["maas-oauth"].(string)
24 }
25
26+func (cfg *maasEnvironConfig) maasAgentName() string {
27+ if uuid, ok := cfg.attrs["maas-agent-name"].(string); ok {
28+ return uuid
29+ }
30+ return ""
31+}
32+
33 func (prov maasEnvironProvider) newConfig(cfg *config.Config) (*maasEnvironConfig, error) {
34 validCfg, err := prov.Validate(cfg, nil)
35 if err != nil {
36@@ -58,6 +72,12 @@
37 if err != nil {
38 return nil, err
39 }
40+ if oldCfg != nil {
41+ oldAttrs := oldCfg.UnknownAttrs()
42+ if validated["maas-agent-name"] != oldAttrs["maas-agent-name"] {
43+ return nil, fmt.Errorf("cannot change maas-agent-name")
44+ }
45+ }
46 envCfg := new(maasEnvironConfig)
47 envCfg.Config = cfg
48 envCfg.attrs = validated
49
50=== modified file 'provider/maas/config_test.go'
51--- provider/maas/config_test.go 2013-09-20 02:33:04 +0000
52+++ provider/maas/config_test.go 2013-10-16 13:44:41 +0000
53@@ -9,6 +9,7 @@
54 "launchpad.net/juju-core/environs"
55 "launchpad.net/juju-core/testing"
56 "launchpad.net/juju-core/testing/testbase"
57+ "launchpad.net/juju-core/utils"
58 )
59
60 type configSuite struct {
61@@ -43,17 +44,30 @@
62 server := "http://maas.testing.invalid/maas/"
63 oauth := "consumer-key:resource-token:resource-secret"
64 future := "futurama"
65+ uuid, err := utils.NewUUID()
66+ c.Assert(err, gc.IsNil)
67 ecfg, err := newConfig(map[string]interface{}{
68- "maas-server": server,
69- "maas-oauth": oauth,
70- "future-key": future,
71+ "maas-server": server,
72+ "maas-oauth": oauth,
73+ "maas-agent-name": uuid.String(),
74+ "future-key": future,
75 })
76 c.Assert(err, gc.IsNil)
77 c.Check(ecfg.maasServer(), gc.Equals, server)
78 c.Check(ecfg.maasOAuth(), gc.DeepEquals, oauth)
79+ c.Check(ecfg.maasAgentName(), gc.Equals, uuid.String())
80 c.Check(ecfg.UnknownAttrs()["future-key"], gc.DeepEquals, future)
81 }
82
83+func (*configSuite) TestMaasAgentNameDefault(c *gc.C) {
84+ ecfg, err := newConfig(map[string]interface{}{
85+ "maas-server": "http://maas.testing.invalid/maas/",
86+ "maas-oauth": "consumer-key:resource-token:resource-secret",
87+ })
88+ c.Assert(err, gc.IsNil)
89+ c.Check(ecfg.maasAgentName(), gc.Equals, "")
90+}
91+
92 func (*configSuite) TestChecksWellFormedMaasServer(c *gc.C) {
93 _, err := newConfig(map[string]interface{}{
94 "maas-server": "This should have been a URL.",
95@@ -91,3 +105,19 @@
96 c.Assert(err, gc.NotNil)
97 c.Check(err, gc.ErrorMatches, ".*cannot change name.*")
98 }
99+
100+func (*configSuite) TestValidateCannotChangeAgentName(c *gc.C) {
101+ baseAttrs := map[string]interface{}{
102+ "maas-server": "http://maas.testing.invalid/maas/",
103+ "maas-oauth": "consumer-key:resource-token:resource-secret",
104+ "maas-agent-name": "1234-5678",
105+ }
106+ oldCfg, err := newConfig(baseAttrs)
107+ c.Assert(err, gc.IsNil)
108+ newCfg, err := oldCfg.Apply(map[string]interface{}{
109+ "maas-agent-name": "9876-5432",
110+ })
111+ c.Assert(err, gc.IsNil)
112+ _, err = maasEnvironProvider{}.Validate(newCfg, oldCfg.Config)
113+ c.Assert(err, gc.ErrorMatches, "cannot change maas-agent-name")
114+}
115
116=== modified file 'provider/maas/environ.go'
117--- provider/maas/environ.go 2013-10-02 00:29:29 +0000
118+++ provider/maas/environ.go 2013-10-16 13:44:41 +0000
119@@ -170,12 +170,13 @@
120
121 // acquireNode allocates a node from the MAAS.
122 func (environ *maasEnviron) acquireNode(cons constraints.Value, possibleTools tools.List) (gomaasapi.MAASObject, *tools.Tools, error) {
123- constraintsParams := convertConstraints(cons)
124+ acquireParams := convertConstraints(cons)
125+ acquireParams.Add("agent_name", environ.ecfg().maasAgentName())
126 var result gomaasapi.JSONObject
127 var err error
128 for a := shortAttempt.Start(); a.Next(); {
129 client := environ.getMAASClient().GetSubObject("nodes/")
130- result, err = client.CallPost("acquire", constraintsParams)
131+ result, err = client.CallPost("acquire", acquireParams)
132 if err == nil {
133 break
134 }
135@@ -322,6 +323,7 @@
136 func (environ *maasEnviron) instances(ids []instance.Id) ([]instance.Instance, error) {
137 nodeListing := environ.getMAASClient().GetSubObject("nodes")
138 filter := getSystemIdValues(ids)
139+ filter.Add("agent_name", environ.ecfg().maasAgentName())
140 listNodeObjects, err := nodeListing.CallGet("list", filter)
141 if err != nil {
142 return nil, err
143
144=== modified file 'provider/maas/environ_test.go'
145--- provider/maas/environ_test.go 2013-10-01 12:03:33 +0000
146+++ provider/maas/environ_test.go 2013-10-16 13:44:41 +0000
147@@ -4,6 +4,7 @@
148 package maas
149
150 import (
151+ "bytes"
152 "encoding/base64"
153 "fmt"
154 "io/ioutil"
155@@ -26,7 +27,6 @@
156 "launchpad.net/juju-core/instance"
157 "launchpad.net/juju-core/juju/testing"
158 "launchpad.net/juju-core/provider/common"
159- coretesting "launchpad.net/juju-core/testing"
160 jc "launchpad.net/juju-core/testing/checkers"
161 "launchpad.net/juju-core/tools"
162 "launchpad.net/juju-core/utils"
163@@ -54,31 +54,12 @@
164 return ecfg.Config
165 }
166
167-// makeEnviron creates a functional maasEnviron for a test.
168-func (suite *environSuite) makeEnviron() *maasEnviron {
169- attrs := coretesting.FakeConfig().Merge(coretesting.Attrs{
170- "name": suite.environ.Name(),
171- "type": "maas",
172- "maas-oauth": "a:b:c",
173- "maas-server": suite.testMAASObject.TestServer.URL,
174- })
175- cfg, err := config.New(config.NoDefaults, attrs)
176- if err != nil {
177- panic(err)
178- }
179- env, err := NewEnviron(cfg)
180- if err != nil {
181- panic(err)
182- }
183- return env
184-}
185-
186 func (suite *environSuite) setupFakeProviderStateFile(c *gc.C) {
187 suite.testMAASObject.TestServer.NewFile(common.StateFile, []byte("test file content"))
188 }
189
190 func (suite *environSuite) setupFakeTools(c *gc.C) {
191- stor := NewStorage(suite.environ)
192+ stor := NewStorage(suite.makeEnviron())
193 envtesting.UploadFakeTools(c, stor)
194 }
195
196@@ -139,7 +120,7 @@
197 resourceURI, _ := node.GetField("resource_uri")
198 instanceIds := []instance.Id{instance.Id(resourceURI)}
199
200- instances, err := suite.environ.Instances(instanceIds)
201+ instances, err := suite.makeEnviron().Instances(instanceIds)
202
203 c.Check(err, gc.IsNil)
204 c.Check(len(instances), gc.Equals, 1)
205@@ -149,7 +130,7 @@
206 func (suite *environSuite) TestInstancesReturnsErrNoInstancesIfEmptyParameter(c *gc.C) {
207 input := `{"system_id": "test"}`
208 suite.testMAASObject.TestServer.NewNode(input)
209- instances, err := suite.environ.Instances([]instance.Id{})
210+ instances, err := suite.makeEnviron().Instances([]instance.Id{})
211
212 c.Check(err, gc.Equals, environs.ErrNoInstances)
213 c.Check(instances, gc.IsNil)
214@@ -158,14 +139,14 @@
215 func (suite *environSuite) TestInstancesReturnsErrNoInstancesIfNilParameter(c *gc.C) {
216 input := `{"system_id": "test"}`
217 suite.testMAASObject.TestServer.NewNode(input)
218- instances, err := suite.environ.Instances(nil)
219+ instances, err := suite.makeEnviron().Instances(nil)
220
221 c.Check(err, gc.Equals, environs.ErrNoInstances)
222 c.Check(instances, gc.IsNil)
223 }
224
225 func (suite *environSuite) TestInstancesReturnsErrNoInstancesIfNoneFound(c *gc.C) {
226- _, err := suite.environ.Instances([]instance.Id{"unknown"})
227+ _, err := suite.makeEnviron().Instances([]instance.Id{"unknown"})
228 c.Check(err, gc.Equals, environs.ErrNoInstances)
229 }
230
231@@ -174,7 +155,7 @@
232 node := suite.testMAASObject.TestServer.NewNode(input)
233 resourceURI, _ := node.GetField("resource_uri")
234
235- instances, err := suite.environ.AllInstances()
236+ instances, err := suite.makeEnviron().AllInstances()
237
238 c.Check(err, gc.IsNil)
239 c.Check(len(instances), gc.Equals, 1)
240@@ -182,7 +163,7 @@
241 }
242
243 func (suite *environSuite) TestAllInstancesReturnsEmptySliceIfNoInstance(c *gc.C) {
244- instances, err := suite.environ.AllInstances()
245+ instances, err := suite.makeEnviron().AllInstances()
246
247 c.Check(err, gc.IsNil)
248 c.Check(len(instances), gc.Equals, 0)
249@@ -198,7 +179,7 @@
250 instanceId2 := instance.Id("unknown systemID")
251 instanceIds := []instance.Id{instanceId1, instanceId2}
252
253- instances, err := suite.environ.Instances(instanceIds)
254+ instances, err := suite.makeEnviron().Instances(instanceIds)
255
256 c.Check(err, gc.Equals, environs.ErrPartialInstances)
257 c.Check(len(instances), gc.Equals, 1)
258@@ -298,7 +279,7 @@
259 }
260
261 func (suite *environSuite) TestAcquireNode(c *gc.C) {
262- stor := NewStorage(suite.environ)
263+ stor := NewStorage(suite.makeEnviron())
264 fakeTools := envtesting.MustUploadFakeToolsVersions(stor, version.Current)[0]
265 env := suite.makeEnviron()
266 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
267@@ -313,7 +294,7 @@
268 }
269
270 func (suite *environSuite) TestAcquireNodeTakesConstraintsIntoAccount(c *gc.C) {
271- stor := NewStorage(suite.environ)
272+ stor := NewStorage(suite.makeEnviron())
273 fakeTools := envtesting.MustUploadFakeToolsVersions(stor, version.Current)[0]
274 env := suite.makeEnviron()
275 suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
276@@ -329,6 +310,21 @@
277 c.Assert(nodeRequestValues[0].Get("mem"), gc.Equals, "1024")
278 }
279
280+func (suite *environSuite) TestAcquireNodePassedAgentName(c *gc.C) {
281+ stor := NewStorage(suite.makeEnviron())
282+ fakeTools := envtesting.MustUploadFakeToolsVersions(stor, version.Current)[0]
283+ env := suite.makeEnviron()
284+ suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
285+
286+ _, _, err := env.acquireNode(constraints.Value{}, tools.List{fakeTools})
287+
288+ c.Check(err, gc.IsNil)
289+ requestValues := suite.testMAASObject.TestServer.NodeOperationRequestValues()
290+ nodeRequestValues, found := requestValues["node0"]
291+ c.Assert(found, gc.Equals, true)
292+ c.Assert(nodeRequestValues[0].Get("agent_name"), gc.Equals, exampleAgentName)
293+}
294+
295 func (*environSuite) TestConvertConstraints(c *gc.C) {
296 var testValues = []struct {
297 constraints constraints.Value
298@@ -352,13 +348,13 @@
299 func (suite *environSuite) getInstance(systemId string) *maasInstance {
300 input := `{"system_id": "` + systemId + `"}`
301 node := suite.testMAASObject.TestServer.NewNode(input)
302- return &maasInstance{&node, suite.environ}
303+ return &maasInstance{&node, suite.makeEnviron()}
304 }
305
306 func (suite *environSuite) TestStopInstancesReturnsIfParameterEmpty(c *gc.C) {
307 suite.getInstance("test1")
308
309- err := suite.environ.StopInstances([]instance.Instance{})
310+ err := suite.makeEnviron().StopInstances([]instance.Instance{})
311 c.Check(err, gc.IsNil)
312 operations := suite.testMAASObject.TestServer.NodeOperations()
313 c.Check(operations, gc.DeepEquals, map[string][]string{})
314@@ -370,7 +366,7 @@
315 suite.getInstance("test3")
316 instances := []instance.Instance{instance1, instance2}
317
318- err := suite.environ.StopInstances(instances)
319+ err := suite.makeEnviron().StopInstances(instances)
320
321 c.Check(err, gc.IsNil)
322 operations := suite.testMAASObject.TestServer.NodeOperations()
323@@ -383,7 +379,7 @@
324 hostname := "test"
325 input := `{"system_id": "system_id", "hostname": "` + hostname + `"}`
326 node := suite.testMAASObject.TestServer.NewNode(input)
327- testInstance := &maasInstance{&node, suite.environ}
328+ testInstance := &maasInstance{&node, suite.makeEnviron()}
329 err := common.SaveState(
330 env.Storage(),
331 &common.BootstrapState{StateInstances: []instance.Id{testInstance.Id()}})
332@@ -480,7 +476,9 @@
333 // Add a dummy file to storage so we can use that to check the
334 // obtained source later.
335 data := makeRandomBytes(10)
336- suite.testMAASObject.TestServer.NewFile("filename", data)
337+ stor := NewStorage(env)
338+ err := stor.Put("filename", bytes.NewBuffer([]byte(data)), int64(len(data)))
339+ c.Assert(err, gc.IsNil)
340 sources, err := imagemetadata.GetMetadataSources(env)
341 c.Assert(err, gc.IsNil)
342 c.Assert(len(sources), gc.Equals, 2)
343@@ -495,7 +493,9 @@
344 // Add a dummy file to storage so we can use that to check the
345 // obtained source later.
346 data := makeRandomBytes(10)
347- suite.testMAASObject.TestServer.NewFile("tools/filename", data)
348+ stor := NewStorage(env)
349+ err := stor.Put("tools/filename", bytes.NewBuffer([]byte(data)), int64(len(data)))
350+ c.Assert(err, gc.IsNil)
351 sources, err := envtools.GetMetadataSources(env)
352 c.Assert(err, gc.IsNil)
353 c.Assert(len(sources), gc.Equals, 1)
354
355=== modified file 'provider/maas/environprovider.go'
356--- provider/maas/environprovider.go 2013-09-25 17:04:52 +0000
357+++ provider/maas/environprovider.go 2013-10-16 13:44:41 +0000
358@@ -4,6 +4,7 @@
359 package maas
360
361 import (
362+ "errors"
363 "os"
364
365 "launchpad.net/loggo"
366@@ -37,8 +38,24 @@
367 return env, nil
368 }
369
370+var errAgentNameAlreadySet = errors.New(
371+ "maas-agent-name is already set; this should not be set by hand")
372+
373 func (p maasEnvironProvider) Prepare(cfg *config.Config) (environs.Environ, error) {
374- // TODO any attributes to prepare?
375+ attrs := cfg.UnknownAttrs()
376+ oldName, found := attrs["maas-agent-name"]
377+ if found && oldName != "" {
378+ return nil, errAgentNameAlreadySet
379+ }
380+ uuid, err := utils.NewUUID()
381+ if err != nil {
382+ return nil, err
383+ }
384+ attrs["maas-agent-name"] = uuid.String()
385+ cfg, err = cfg.Apply(attrs)
386+ if err != nil {
387+ return nil, err
388+ }
389 return p.Open(cfg)
390 }
391
392
393=== modified file 'provider/maas/environprovider_test.go'
394--- provider/maas/environprovider_test.go 2013-09-25 17:04:52 +0000
395+++ provider/maas/environprovider_test.go 2013-10-16 13:44:41 +0000
396@@ -11,6 +11,8 @@
397
398 "launchpad.net/juju-core/environs/config"
399 "launchpad.net/juju-core/testing"
400+ jc "launchpad.net/juju-core/testing/checkers"
401+ "launchpad.net/juju-core/utils"
402 )
403
404 type EnvironProviderSuite struct {
405@@ -31,13 +33,52 @@
406 config, err := config.New(config.NoDefaults, attrs)
407 c.Assert(err, gc.IsNil)
408
409- secretAttrs, err := suite.environ.Provider().SecretAttrs(config)
410+ secretAttrs, err := suite.makeEnviron().Provider().SecretAttrs(config)
411 c.Assert(err, gc.IsNil)
412
413 expectedAttrs := map[string]string{"maas-oauth": oauth}
414 c.Check(secretAttrs, gc.DeepEquals, expectedAttrs)
415 }
416
417+func (suite *EnvironProviderSuite) TestUnknownAttrsContainAgentName(c *gc.C) {
418+ testJujuHome := c.MkDir()
419+ defer config.SetJujuHome(config.SetJujuHome(testJujuHome))
420+ attrs := testing.FakeConfig().Merge(testing.Attrs{
421+ "type": "maas",
422+ "maas-oauth": "aa:bb:cc",
423+ "maas-server": "http://maas.testing.invalid/maas/",
424+ })
425+ config, err := config.New(config.NoDefaults, attrs)
426+ c.Assert(err, gc.IsNil)
427+
428+ environ, err := suite.makeEnviron().Provider().Prepare(config)
429+ c.Assert(err, gc.IsNil)
430+
431+ preparedConfig := environ.Config()
432+ unknownAttrs := preparedConfig.UnknownAttrs()
433+
434+ uuid, ok := unknownAttrs["maas-agent-name"]
435+
436+ c.Assert(ok, jc.IsTrue)
437+ c.Assert(uuid, jc.Satisfies, utils.IsValidUUIDString)
438+}
439+
440+func (suite *EnvironProviderSuite) TestAgentNameShouldNotBeSetByHand(c *gc.C) {
441+ testJujuHome := c.MkDir()
442+ defer config.SetJujuHome(config.SetJujuHome(testJujuHome))
443+ attrs := testing.FakeConfig().Merge(testing.Attrs{
444+ "type": "maas",
445+ "maas-oauth": "aa:bb:cc",
446+ "maas-server": "http://maas.testing.invalid/maas/",
447+ "maas-agent-name": "foobar",
448+ })
449+ config, err := config.New(config.NoDefaults, attrs)
450+ c.Assert(err, gc.IsNil)
451+
452+ _, err = suite.makeEnviron().Provider().Prepare(config)
453+ c.Assert(err, gc.Equals, errAgentNameAlreadySet)
454+}
455+
456 // create a temporary file with the given content. The file will be cleaned
457 // up at the end of the test calling this method.
458 func createTempFile(c *gc.C, content []byte) string {
459@@ -65,7 +106,7 @@
460 _MAASInstanceFilename = filename
461 defer func() { _MAASInstanceFilename = old_MAASInstanceFilename }()
462
463- provider := suite.environ.Provider()
464+ provider := suite.makeEnviron().Provider()
465 publicAddress, err := provider.PublicAddress()
466 c.Assert(err, gc.IsNil)
467 c.Check(publicAddress, gc.Equals, hostname)
468@@ -85,7 +126,7 @@
469 })
470 config, err := config.New(config.NoDefaults, attrs)
471 c.Assert(err, gc.IsNil)
472- env, err := suite.environ.Provider().Open(config)
473+ env, err := suite.makeEnviron().Provider().Open(config)
474 // When Open() fails (i.e. returns a non-nil error), it returns an
475 // environs.Environ interface object with a nil value and a nil
476 // type.
477
478=== modified file 'provider/maas/instance_test.go'
479--- provider/maas/instance_test.go 2013-10-10 15:08:27 +0000
480+++ provider/maas/instance_test.go 2013-10-16 13:44:41 +0000
481@@ -21,7 +21,7 @@
482 jsonValue := `{"system_id": "system_id", "test": "test"}`
483 obj := s.testMAASObject.TestServer.NewNode(jsonValue)
484 resourceURI, _ := obj.GetField("resource_uri")
485- instance := maasInstance{&obj, s.environ}
486+ instance := maasInstance{&obj, s.makeEnviron()}
487
488 c.Check(string(instance.Id()), gc.Equals, resourceURI)
489 }
490@@ -29,19 +29,21 @@
491 func (s *instanceTest) TestString(c *gc.C) {
492 jsonValue := `{"hostname": "thethingintheplace", "system_id": "system_id", "test": "test"}`
493 obj := s.testMAASObject.TestServer.NewNode(jsonValue)
494- instance := &maasInstance{&obj, s.environ}
495+ instance := &maasInstance{&obj, s.makeEnviron()}
496 hostname, err := instance.DNSName()
497 c.Assert(err, gc.IsNil)
498 expected := hostname + ":" + string(instance.Id())
499 c.Assert(fmt.Sprint(instance), gc.Equals, expected)
500+}
501
502+func (s *instanceTest) TestStringWithoutHostname(c *gc.C) {
503 // For good measure, test what happens if we don't have a hostname.
504- jsonValue = `{"system_id": "system_id", "test": "test"}`
505- obj = s.testMAASObject.TestServer.NewNode(jsonValue)
506- instance = &maasInstance{&obj, s.environ}
507- hostname, err = instance.DNSName()
508+ jsonValue := `{"system_id": "system_id", "test": "test"}`
509+ obj := s.testMAASObject.TestServer.NewNode(jsonValue)
510+ instance := &maasInstance{&obj, s.makeEnviron()}
511+ _, err := instance.DNSName()
512 c.Assert(err, gc.NotNil)
513- expected = fmt.Sprintf("<DNSName failed: %q>", err) + ":" + string(instance.Id())
514+ expected := fmt.Sprintf("<DNSName failed: %q>", err) + ":" + string(instance.Id())
515 c.Assert(fmt.Sprint(instance), gc.Equals, expected)
516 }
517
518@@ -49,7 +51,7 @@
519 jsonValue := `{"system_id": "system_id", "test": "test"}`
520 obj := s.testMAASObject.TestServer.NewNode(jsonValue)
521 s.testMAASObject.TestServer.ChangeNode("system_id", "test2", "test2")
522- instance := maasInstance{&obj, s.environ}
523+ instance := maasInstance{&obj, s.makeEnviron()}
524
525 err := instance.refreshInstance()
526
527@@ -62,7 +64,7 @@
528 func (s *instanceTest) TestDNSName(c *gc.C) {
529 jsonValue := `{"hostname": "DNS name", "system_id": "system_id"}`
530 obj := s.testMAASObject.TestServer.NewNode(jsonValue)
531- instance := maasInstance{&obj, s.environ}
532+ instance := maasInstance{&obj, s.makeEnviron()}
533
534 dnsName, err := instance.DNSName()
535
536@@ -83,7 +85,7 @@
537 "ip_addresses": [ "1.2.3.4", "fe80::d806:dbff:fe23:1199" ]
538 }`
539 obj := s.testMAASObject.TestServer.NewNode(jsonValue)
540- inst := maasInstance{&obj, s.environ}
541+ inst := maasInstance{&obj, s.makeEnviron()}
542
543 expected := []instance.Address{
544 {Value: "testing.invalid", Type: instance.HostName, NetworkScope: instance.NetworkPublic},
545@@ -105,7 +107,7 @@
546 "system_id": "system_id"
547 }`
548 obj := s.testMAASObject.TestServer.NewNode(jsonValue)
549- inst := maasInstance{&obj, s.environ}
550+ inst := maasInstance{&obj, s.makeEnviron()}
551
552 addr, err := inst.Addresses()
553 c.Assert(err, gc.IsNil)
554@@ -121,7 +123,7 @@
555 "ip_addresses": "incompatible"
556 }`
557 obj := s.testMAASObject.TestServer.NewNode(jsonValue)
558- inst := maasInstance{&obj, s.environ}
559+ inst := maasInstance{&obj, s.makeEnviron()}
560
561 _, err := inst.Addresses()
562 c.Assert(err, gc.NotNil)
563@@ -134,7 +136,7 @@
564 "ip_addresses": [42]
565 }`
566 obj := s.testMAASObject.TestServer.NewNode(jsonValue)
567- inst := maasInstance{&obj, s.environ}
568+ inst := maasInstance{&obj, s.makeEnviron()}
569
570 _, err := inst.Addresses()
571 c.Assert(err, gc.NotNil)
572
573=== modified file 'provider/maas/maas_test.go'
574--- provider/maas/maas_test.go 2013-09-20 02:53:59 +0000
575+++ provider/maas/maas_test.go 2013-10-16 13:44:41 +0000
576@@ -9,7 +9,9 @@
577 gc "launchpad.net/gocheck"
578 "launchpad.net/gomaasapi"
579
580+ "launchpad.net/juju-core/environs/config"
581 envtesting "launchpad.net/juju-core/environs/testing"
582+ coretesting "launchpad.net/juju-core/testing"
583 "launchpad.net/juju-core/testing/testbase"
584 )
585
586@@ -20,7 +22,6 @@
587 type providerSuite struct {
588 testbase.LoggingSuite
589 envtesting.ToolsFixture
590- environ *maasEnviron
591 testMAASObject *gomaasapi.TestMAASObject
592 restoreTimeouts func()
593 }
594@@ -32,7 +33,6 @@
595 s.LoggingSuite.SetUpSuite(c)
596 TestMAASObject := gomaasapi.NewTestMAAS("1.0")
597 s.testMAASObject = TestMAASObject
598- s.environ = &maasEnviron{name: "test env", maasClientUnlocked: &TestMAASObject.MAASObject}
599 }
600
601 func (s *providerSuite) SetUpTest(c *gc.C) {
602@@ -51,3 +51,25 @@
603 s.restoreTimeouts()
604 s.LoggingSuite.TearDownSuite(c)
605 }
606+
607+const exampleAgentName = "dfb69555-0bc4-4d1f-85f2-4ee390974984"
608+
609+// makeEnviron creates a functional maasEnviron for a test.
610+func (suite *providerSuite) makeEnviron() *maasEnviron {
611+ attrs := coretesting.FakeConfig().Merge(coretesting.Attrs{
612+ "name": "test env",
613+ "type": "maas",
614+ "maas-oauth": "a:b:c",
615+ "maas-server": suite.testMAASObject.TestServer.URL,
616+ "maas-agent-name": exampleAgentName,
617+ })
618+ cfg, err := config.New(config.NoDefaults, attrs)
619+ if err != nil {
620+ panic(err)
621+ }
622+ env, err := NewEnviron(cfg)
623+ if err != nil {
624+ panic(err)
625+ }
626+ return env
627+}
628
629=== modified file 'provider/maas/storage.go'
630--- provider/maas/storage.go 2013-09-18 22:54:32 +0000
631+++ provider/maas/storage.go 2013-10-16 13:44:41 +0000
632@@ -11,6 +11,7 @@
633 "io/ioutil"
634 "net/url"
635 "sort"
636+ "strings"
637 "sync"
638
639 "launchpad.net/gomaasapi"
640@@ -88,8 +89,21 @@
641 return obj, nil
642 }
643
644+// All filenames need to be namespaced so they are private to this environment.
645+// This prevents different environments from interfering with each other.
646+// We're using the agent name UUID here.
647+func (stor *maasStorage) prefixWithPrivateNamespace(name string) string {
648+ env := stor.getSnapshot().environUnlocked
649+ prefix := env.ecfg().maasAgentName()
650+ if prefix != "" {
651+ return prefix + "-" + name
652+ }
653+ return name
654+}
655+
656 // Get is specified in the StorageReader interface.
657 func (stor *maasStorage) Get(name string) (io.ReadCloser, error) {
658+ name = stor.prefixWithPrivateNamespace(name)
659 fileObj, err := stor.retrieveFileObject(name)
660 if err != nil {
661 return nil, err
662@@ -108,6 +122,7 @@
663 // extractFilenames returns the filenames from a "list" operation on the
664 // MAAS API, sorted by name.
665 func (stor *maasStorage) extractFilenames(listResult gomaasapi.JSONObject) ([]string, error) {
666+ privatePrefix := stor.prefixWithPrivateNamespace("")
667 list, err := listResult.GetArray()
668 if err != nil {
669 return nil, err
670@@ -122,7 +137,8 @@
671 if err != nil {
672 return nil, err
673 }
674- result[index] = filename
675+ // When listing files we need to return them without our special prefix.
676+ result[index] = strings.TrimPrefix(filename, privatePrefix)
677 }
678 sort.Strings(result)
679 return result, nil
680@@ -130,6 +146,7 @@
681
682 // List is specified in the StorageReader interface.
683 func (stor *maasStorage) List(prefix string) ([]string, error) {
684+ prefix = stor.prefixWithPrivateNamespace(prefix)
685 params := make(url.Values)
686 params.Add("prefix", prefix)
687 snapshot := stor.getSnapshot()
688@@ -142,6 +159,7 @@
689
690 // URL is specified in the StorageReader interface.
691 func (stor *maasStorage) URL(name string) (string, error) {
692+ name = stor.prefixWithPrivateNamespace(name)
693 fileObj, err := stor.retrieveFileObject(name)
694 if err != nil {
695 return "", err
696@@ -174,6 +192,7 @@
697
698 // Put is specified in the StorageWriter interface.
699 func (stor *maasStorage) Put(name string, r io.Reader, length int64) error {
700+ name = stor.prefixWithPrivateNamespace(name)
701 data, err := ioutil.ReadAll(io.LimitReader(r, length))
702 if err != nil {
703 return err
704@@ -187,6 +206,7 @@
705
706 // Remove is specified in the StorageWriter interface.
707 func (stor *maasStorage) Remove(name string) error {
708+ name = stor.prefixWithPrivateNamespace(name)
709 // The only thing that can go wrong here, really, is that the file
710 // does not exist. But deletion is idempotent: deleting a file that
711 // is no longer there anyway is success, not failure.
712@@ -200,7 +220,7 @@
713 if err != nil {
714 return err
715 }
716- // Remove all the objects in parallel so that we incur less round-trips.
717+ // Remove all the objects in parallel so that we incur fewer round-trips.
718 // If we're in danger of having hundreds of objects,
719 // we'll want to change this to limit the number
720 // of concurrent operations.
721
722=== modified file 'provider/maas/storage_test.go'
723--- provider/maas/storage_test.go 2013-09-18 22:54:32 +0000
724+++ provider/maas/storage_test.go 2013-10-16 13:44:41 +0000
725@@ -29,8 +29,10 @@
726 // makeStorage creates a MAAS storage object for the running test.
727 func (s *storageSuite) makeStorage(name string) *maasStorage {
728 maasobj := s.testMAASObject.MAASObject
729- env := maasEnviron{name: name, maasClientUnlocked: &maasobj}
730- return NewStorage(&env).(*maasStorage)
731+ env := s.makeEnviron()
732+ env.name = name
733+ env.maasClientUnlocked = &maasobj
734+ return NewStorage(env).(*maasStorage)
735 }
736
737 // makeRandomBytes returns an array of arbitrary byte values.
738@@ -50,7 +52,10 @@
739 // Or don't, if you want consistent (and debuggable) results.
740 func (s *storageSuite) fakeStoredFile(stor storage.Storage, name string) gomaasapi.MAASObject {
741 data := makeRandomBytes(rand.Intn(10))
742- return s.testMAASObject.TestServer.NewFile(name, data)
743+ // The filename must be prefixed with the private namespace as we're
744+ // bypassing the Put() method that would normally do that.
745+ prefixFilename := stor.(*maasStorage).prefixWithPrivateNamespace("") + name
746+ return s.testMAASObject.TestServer.NewFile(prefixFilename, data)
747 }
748
749 func (s *storageSuite) TestGetSnapshotCreatesClone(c *gc.C) {
750@@ -93,7 +98,8 @@
751 fileContent, err := file.GetField("content")
752 c.Assert(err, gc.IsNil)
753
754- obj, err := stor.retrieveFileObject(filename)
755+ prefixFilename := stor.prefixWithPrivateNamespace(filename)
756+ obj, err := stor.retrieveFileObject(prefixFilename)
757 c.Assert(err, gc.IsNil)
758
759 uri, err := obj.GetField("anon_resource_uri")
760@@ -117,7 +123,8 @@
761 err := stor.Put(filename, bytes.NewReader(data), int64(len(data)))
762 c.Assert(err, gc.IsNil)
763
764- obj, err := stor.retrieveFileObject(filename)
765+ prefixFilename := stor.prefixWithPrivateNamespace(filename)
766+ obj, err := stor.retrieveFileObject(prefixFilename)
767 c.Assert(err, gc.IsNil)
768
769 base64Content, err := obj.GetField("content")
770@@ -144,20 +151,20 @@
771
772 func (s *storageSuite) TestGetReturnsNotFoundErrorIfNotFound(c *gc.C) {
773 const filename = "lost-data"
774- stor := NewStorage(s.environ)
775+ stor := NewStorage(s.makeEnviron())
776 _, err := storage.Get(stor, filename)
777 c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
778 }
779
780 func (s *storageSuite) TestListReturnsEmptyIfNoFilesStored(c *gc.C) {
781- stor := NewStorage(s.environ)
782+ stor := NewStorage(s.makeEnviron())
783 listing, err := storage.List(stor, "")
784 c.Assert(err, gc.IsNil)
785 c.Check(listing, gc.DeepEquals, []string{})
786 }
787
788 func (s *storageSuite) TestListReturnsAllFilesIfPrefixEmpty(c *gc.C) {
789- stor := NewStorage(s.environ)
790+ stor := NewStorage(s.makeEnviron())
791 files := []string{"1a", "2b", "3c"}
792 for _, name := range files {
793 s.fakeStoredFile(stor, name)
794@@ -169,7 +176,7 @@
795 }
796
797 func (s *storageSuite) TestListSortsResults(c *gc.C) {
798- stor := NewStorage(s.environ)
799+ stor := NewStorage(s.makeEnviron())
800 files := []string{"4d", "1a", "3c", "2b"}
801 for _, name := range files {
802 s.fakeStoredFile(stor, name)
803@@ -181,7 +188,7 @@
804 }
805
806 func (s *storageSuite) TestListReturnsNoFilesIfNoFilesMatchPrefix(c *gc.C) {
807- stor := NewStorage(s.environ)
808+ stor := NewStorage(s.makeEnviron())
809 s.fakeStoredFile(stor, "foo")
810
811 listing, err := storage.List(stor, "bar")
812@@ -190,7 +197,7 @@
813 }
814
815 func (s *storageSuite) TestListReturnsOnlyFilesWithMatchingPrefix(c *gc.C) {
816- stor := NewStorage(s.environ)
817+ stor := NewStorage(s.makeEnviron())
818 s.fakeStoredFile(stor, "abc")
819 s.fakeStoredFile(stor, "xyz")
820
821@@ -200,7 +207,7 @@
822 }
823
824 func (s *storageSuite) TestListMatchesPrefixOnly(c *gc.C) {
825- stor := NewStorage(s.environ)
826+ stor := NewStorage(s.makeEnviron())
827 s.fakeStoredFile(stor, "abc")
828 s.fakeStoredFile(stor, "xabc")
829
830@@ -210,7 +217,7 @@
831 }
832
833 func (s *storageSuite) TestListOperatesOnFlatNamespace(c *gc.C) {
834- stor := NewStorage(s.environ)
835+ stor := NewStorage(s.makeEnviron())
836 s.fakeStoredFile(stor, "a/b/c/d")
837
838 listing, err := storage.List(stor, "a/b")
839@@ -233,7 +240,7 @@
840
841 func (s *storageSuite) TestURLReturnsURLCorrespondingToFile(c *gc.C) {
842 const filename = "my-file.txt"
843- stor := NewStorage(s.environ).(*maasStorage)
844+ stor := NewStorage(s.makeEnviron()).(*maasStorage)
845 file := s.fakeStoredFile(stor, filename)
846 // The file contains an anon_resource_uri, which lacks a network part
847 // (but will probably contain a query part). anonURL will be the
848@@ -256,7 +263,7 @@
849 const filename = "broken-toaster.jpg"
850 contents := []byte("Contents here")
851 length := int64(len(contents))
852- stor := NewStorage(s.environ)
853+ stor := NewStorage(s.makeEnviron())
854
855 err := stor.Put(filename, bytes.NewReader(contents), length)
856
857@@ -271,7 +278,7 @@
858
859 func (s *storageSuite) TestPutOverwritesFile(c *gc.C) {
860 const filename = "foo.bar"
861- stor := NewStorage(s.environ)
862+ stor := NewStorage(s.makeEnviron())
863 s.fakeStoredFile(stor, filename)
864 newContents := []byte("Overwritten")
865
866@@ -292,7 +299,7 @@
867 const filename = "xyzzyz.2.xls"
868 const length = 5
869 contents := []byte("abcdefghijklmnopqrstuvwxyz")
870- stor := NewStorage(s.environ)
871+ stor := NewStorage(s.makeEnviron())
872
873 err := stor.Put(filename, bytes.NewReader(contents), length)
874 c.Assert(err, gc.IsNil)
875@@ -310,7 +317,7 @@
876 const filename = "a-file-which-is-mine"
877 oldContents := []byte("abcdefghijklmnopqrstuvwxyz")
878 newContents := []byte("xyz")
879- stor := NewStorage(s.environ)
880+ stor := NewStorage(s.makeEnviron())
881 err := stor.Put(filename, bytes.NewReader(oldContents), int64(len(oldContents)))
882 c.Assert(err, gc.IsNil)
883
884@@ -329,7 +336,7 @@
885
886 func (s *storageSuite) TestRemoveDeletesFile(c *gc.C) {
887 const filename = "doomed.txt"
888- stor := NewStorage(s.environ)
889+ stor := NewStorage(s.makeEnviron())
890 s.fakeStoredFile(stor, filename)
891
892 err := stor.Remove(filename)
893@@ -345,7 +352,7 @@
894
895 func (s *storageSuite) TestRemoveIsIdempotent(c *gc.C) {
896 const filename = "half-a-file"
897- stor := NewStorage(s.environ)
898+ stor := NewStorage(s.makeEnviron())
899 s.fakeStoredFile(stor, filename)
900
901 err := stor.Remove(filename)
902@@ -358,7 +365,7 @@
903 func (s *storageSuite) TestNamesMayHaveSlashes(c *gc.C) {
904 const filename = "name/with/slashes"
905 content := []byte("File contents")
906- stor := NewStorage(s.environ)
907+ stor := NewStorage(s.makeEnviron())
908
909 err := stor.Put(filename, bytes.NewReader(content), int64(len(content)))
910 c.Assert(err, gc.IsNil)
911@@ -390,3 +397,24 @@
912 c.Assert(err, gc.IsNil)
913 c.Assert(listing, gc.DeepEquals, []string{})
914 }
915+
916+func (s *storageSuite) TestprefixWithPrivateNamespacePrefixesWithAgentName(c *gc.C) {
917+ sstor := NewStorage(s.makeEnviron())
918+ stor := sstor.(*maasStorage)
919+ agentName := stor.environUnlocked.ecfg().maasAgentName()
920+ c.Assert(agentName, gc.Not(gc.Equals), "")
921+ expectedPrefix := agentName + "-"
922+ const name = "myname"
923+ expectedResult := expectedPrefix + name
924+ c.Assert(stor.prefixWithPrivateNamespace(name), gc.Equals, expectedResult)
925+}
926+
927+func (s *storageSuite) TesttprefixWithPrivateNamespaceIgnoresAgentName(c *gc.C) {
928+ sstor := NewStorage(s.makeEnviron())
929+ stor := sstor.(*maasStorage)
930+ ecfg := stor.environUnlocked.ecfg()
931+ ecfg.attrs["maas-agent-name"] = ""
932+
933+ const name = "myname"
934+ c.Assert(stor.prefixWithPrivateNamespace(name), gc.Equals, name)
935+}

Subscribers

People subscribed via source and target branches

to all changes: