Merge lp:~axwalk/juju-core/localstorage-to-httpstorage into lp:~go-bot/juju-core/trunk
- localstorage-to-httpstorage
- Merge into trunk
| Status: | Merged |
|---|---|
| Approved by: | Andrew Wilkins |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 1832 |
| Proposed branch: | lp:~axwalk/juju-core/localstorage-to-httpstorage |
| Merge into: | lp:~go-bot/juju-core/trunk |
| Diff against target: |
953 lines (+211/-212) 12 files modified
environs/bootstrap/bootstrap_test.go (+8/-15) environs/filestorage/filestorage.go (+31/-19) environs/filestorage/filestorage_test.go (+45/-11) environs/httpstorage/backend.go (+36/-76) environs/httpstorage/backend_test.go (+6/-3) environs/httpstorage/storage.go (+2/-1) environs/httpstorage/storage_test.go (+5/-5) environs/testing/storage.go (+7/-4) provider/azure/environ_test.go (+33/-49) provider/local/environ.go (+9/-4) provider/local/storage/worker.go (+14/-3) provider/state_test.go (+15/-22) |
| To merge this branch: | bzr merge lp:~axwalk/juju-core/localstorage-to-httpstorage |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Juju Engineering | Pending | ||
|
Review via email:
|
|||
Commit message
environs/
localstorage becomes httpstorage; it no longer
operates on the disk, but instead delegates to
another environs.Storage object.
This work goes together with changes to
environs/
write to temporary storage and atomically move,
in the same way that sshstorage has been coded.
Description of the change
environs/
localstorage becomes httpstorage; it no longer
operates on the disk, but instead delegates to
another environs.Storage object.
This work goes together with changes to
environs/
write to temporary storage and atomically move,
in the same way that sshstorage has been coded.
| Andrew Wilkins (axwalk) wrote : | # |
| Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
| Tim Penhey (thumper) wrote : | # |
A direct test for the glob change, and a helper function to create the
storage are needed.
https:/
File environs/
https:/
environs/
Why defer? There is no harm in calling Close right now.
https:/
environs/
isn't " < 0 " more readable and understandable?
https:/
File environs/
https:/
environs/
a 500 as it is
I argue with this status code.
5xx means it is our fault. 4xx means it is your fault. This is
certainly a bad request.
https:/
File environs/
https:/
environs/
httpstorage.
Given this is the second or third time I have seen these lines of code,
perhaps a testing helper function would be good.
| Andrew Wilkins (axwalk) wrote : | # |
https:/
File environs/
https:/
environs/
On 2013/09/18 04:26:28, thumper wrote:
> Why defer? There is no harm in calling Close right now.
Indeed. I'll keep the defer, but move it closer to Get/before ReadAll.
https:/
environs/
On 2013/09/18 04:26:28, thumper wrote:
> isn't " < 0 " more readable and understandable?
Done.
https:/
File environs/
https:/
environs/
a 500 as it is
On 2013/09/18 04:26:28, thumper wrote:
> I argue with this status code.
> 5xx means it is our fault. 4xx means it is your fault. This is
certainly a bad
> request.
Yeah, I'll change the implementation (of filestorage's Get) and put this
back. Thanks.
https:/
File environs/
https:/
environs/
httpstorage.
On 2013/09/18 04:26:28, thumper wrote:
> Given this is the second or third time I have seen these lines of
code, perhaps
> a testing helper function would be good.
This is a testing helper function ;)
I will modify the other code to use it where it makes sense.
| Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
| Tim Penhey (thumper) wrote : | # |
LGTM - although I'd prefer with the added testing helper method, this
isn't essential.
https:/
File environs/
https:/
environs/
On 2013/09/18 05:33:28, axw1 wrote:
> On 2013/09/18 04:26:28, thumper wrote:
> > Why defer? There is no harm in calling Close right now.
> Indeed. I'll keep the defer, but move it closer to Get/before ReadAll.
Good call. I missed the err just above.
| Andrew Wilkins (axwalk) wrote : | # |
On 2013/09/18 05:43:00, thumper wrote:
> LGTM - although I'd prefer with the added testing helper method, this
isn't
> essential.
Done. I've also updated the code to use AddCleanup.
https:/
> File environs/
https:/
> environs/
> On 2013/09/18 05:33:28, axw1 wrote:
> > On 2013/09/18 04:26:28, thumper wrote:
> > > Why defer? There is no harm in calling Close right now.
> >
> > Indeed. I'll keep the defer, but move it closer to Get/before
ReadAll.
> Good call. I missed the err just above.
| Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
| Go Bot (go-bot) wrote : | # |
The attempt to merge lp:~axwalk/juju-core/localstorage-to-httpstorage into lp:juju-core failed. Below is the output from the failed tests.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
? launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
? launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
ok launchpad.
-------
PANIC: service_
Preview Diff
| 1 | === modified file 'environs/bootstrap/bootstrap_test.go' |
| 2 | --- environs/bootstrap/bootstrap_test.go 2013-09-12 12:38:04 +0000 |
| 3 | +++ environs/bootstrap/bootstrap_test.go 2013-09-18 06:28:38 +0000 |
| 4 | @@ -13,7 +13,6 @@ |
| 5 | "launchpad.net/juju-core/environs" |
| 6 | "launchpad.net/juju-core/environs/bootstrap" |
| 7 | "launchpad.net/juju-core/environs/config" |
| 8 | - "launchpad.net/juju-core/environs/localstorage" |
| 9 | envtesting "launchpad.net/juju-core/environs/testing" |
| 10 | "launchpad.net/juju-core/provider/dummy" |
| 11 | coretesting "launchpad.net/juju-core/testing" |
| 12 | @@ -52,8 +51,7 @@ |
| 13 | |
| 14 | func (s *bootstrapSuite) TestBootstrapNeedsSettings(c *gc.C) { |
| 15 | env := newEnviron("bar", noKeysDefined) |
| 16 | - cleanup := setDummyStorage(c, env) |
| 17 | - defer cleanup() |
| 18 | + s.setDummyStorage(c, env) |
| 19 | fixEnv := func(key string, value interface{}) { |
| 20 | cfg, err := env.Config().Apply(map[string]interface{}{ |
| 21 | key: value, |
| 22 | @@ -87,8 +85,7 @@ |
| 23 | |
| 24 | func (s *bootstrapSuite) TestBootstrapEmptyConstraints(c *gc.C) { |
| 25 | env := newEnviron("foo", useDefaultKeys) |
| 26 | - cleanup := setDummyStorage(c, env) |
| 27 | - defer cleanup() |
| 28 | + s.setDummyStorage(c, env) |
| 29 | uploadTools(c, env) |
| 30 | err := bootstrap.Bootstrap(env, constraints.Value{}) |
| 31 | c.Assert(err, gc.IsNil) |
| 32 | @@ -98,8 +95,7 @@ |
| 33 | |
| 34 | func (s *bootstrapSuite) TestBootstrapSpecifiedConstraints(c *gc.C) { |
| 35 | env := newEnviron("foo", useDefaultKeys) |
| 36 | - cleanup := setDummyStorage(c, env) |
| 37 | - defer cleanup() |
| 38 | + s.setDummyStorage(c, env) |
| 39 | uploadTools(c, env) |
| 40 | cons := constraints.MustParse("cpu-cores=2 mem=4G") |
| 41 | err := bootstrap.Bootstrap(env, cons) |
| 42 | @@ -188,8 +184,7 @@ |
| 43 | |
| 44 | func (s *bootstrapSuite) TestBootstrapNeedsTools(c *gc.C) { |
| 45 | env := newEnviron("foo", useDefaultKeys) |
| 46 | - cleanup := setDummyStorage(c, env) |
| 47 | - defer cleanup() |
| 48 | + s.setDummyStorage(c, env) |
| 49 | envtesting.RemoveFakeTools(c, env.Storage()) |
| 50 | err := bootstrap.Bootstrap(env, constraints.Value{}) |
| 51 | c.Check(err, gc.ErrorMatches, "cannot find bootstrap tools: no tools available") |
| 52 | @@ -228,13 +223,11 @@ |
| 53 | // setDummyStorage injects the local provider's fake storage implementation |
| 54 | // into the given environment, so that tests can manipulate storage as if it |
| 55 | // were real. |
| 56 | -// Returns a cleanup function that must be called when done with the storage. |
| 57 | -func setDummyStorage(c *gc.C, env *bootstrapEnviron) func() { |
| 58 | - listener, err := localstorage.Serve("127.0.0.1:0", c.MkDir()) |
| 59 | - c.Assert(err, gc.IsNil) |
| 60 | - env.storage = localstorage.Client(listener.Addr().String()) |
| 61 | +func (s *bootstrapSuite) setDummyStorage(c *gc.C, env *bootstrapEnviron) { |
| 62 | + closer, storage, _ := envtesting.CreateLocalTestStorage(c) |
| 63 | + env.storage = storage |
| 64 | envtesting.UploadFakeTools(c, env.storage) |
| 65 | - return func() { listener.Close() } |
| 66 | + s.AddCleanup(func(c *gc.C) { closer.Close() }) |
| 67 | } |
| 68 | |
| 69 | func (e *bootstrapEnviron) Name() string { |
| 70 | |
| 71 | === modified file 'environs/filestorage/filestorage.go' |
| 72 | --- environs/filestorage/filestorage.go 2013-09-12 05:04:40 +0000 |
| 73 | +++ environs/filestorage/filestorage.go 2013-09-18 06:28:38 +0000 |
| 74 | @@ -10,8 +10,10 @@ |
| 75 | "os" |
| 76 | "path/filepath" |
| 77 | "sort" |
| 78 | + "strings" |
| 79 | |
| 80 | "launchpad.net/juju-core/environs" |
| 81 | + coreerrors "launchpad.net/juju-core/errors" |
| 82 | "launchpad.net/juju-core/utils" |
| 83 | ) |
| 84 | |
| 85 | @@ -42,6 +44,15 @@ |
| 86 | // Get implements environs.StorageReader.Get. |
| 87 | func (f *fileStorageReader) Get(name string) (io.ReadCloser, error) { |
| 88 | filename := f.fullPath(name) |
| 89 | + fi, err := os.Stat(filename) |
| 90 | + if err != nil { |
| 91 | + if os.IsNotExist(err) { |
| 92 | + err = coreerrors.NewNotFoundError(err, "") |
| 93 | + } |
| 94 | + return nil, err |
| 95 | + } else if fi.IsDir() { |
| 96 | + return nil, coreerrors.NotFoundf("no such file with name %q", name) |
| 97 | + } |
| 98 | file, err := os.Open(filename) |
| 99 | if err != nil { |
| 100 | return nil, err |
| 101 | @@ -51,26 +62,23 @@ |
| 102 | |
| 103 | // List implements environs.StorageReader.List. |
| 104 | func (f *fileStorageReader) List(prefix string) ([]string, error) { |
| 105 | - // Add one for the missing path separator. |
| 106 | - pathlen := len(f.path) + 1 |
| 107 | - pattern := filepath.Join(f.path, prefix+"*") |
| 108 | - matches, err := filepath.Glob(pattern) |
| 109 | - if err != nil { |
| 110 | - return nil, err |
| 111 | - } |
| 112 | - list := []string{} |
| 113 | - for _, match := range matches { |
| 114 | - fi, err := os.Stat(match) |
| 115 | + prefix = filepath.Join(f.path, prefix) |
| 116 | + dir := filepath.Dir(prefix) |
| 117 | + var names []string |
| 118 | + err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { |
| 119 | if err != nil { |
| 120 | - return nil, err |
| 121 | - } |
| 122 | - if !fi.Mode().IsDir() { |
| 123 | - filename := match[pathlen:] |
| 124 | - list = append(list, filename) |
| 125 | - } |
| 126 | + return err |
| 127 | + } |
| 128 | + if !info.IsDir() && strings.HasPrefix(path, prefix) { |
| 129 | + names = append(names, path[len(f.path)+1:]) |
| 130 | + } |
| 131 | + return nil |
| 132 | + }) |
| 133 | + if err != nil && !os.IsNotExist(err) { |
| 134 | + return nil, err |
| 135 | } |
| 136 | - sort.Strings(list) |
| 137 | - return list, nil |
| 138 | + sort.Strings(names) |
| 139 | + return names, nil |
| 140 | } |
| 141 | |
| 142 | // URL implements environs.StorageReader.URL. |
| 143 | @@ -110,7 +118,11 @@ |
| 144 | |
| 145 | func (f *fileStorageWriter) Remove(name string) error { |
| 146 | fullpath := f.fullPath(name) |
| 147 | - return os.Remove(fullpath) |
| 148 | + err := os.Remove(fullpath) |
| 149 | + if os.IsNotExist(err) { |
| 150 | + err = nil |
| 151 | + } |
| 152 | + return err |
| 153 | } |
| 154 | |
| 155 | func (f *fileStorageWriter) RemoveAll() error { |
| 156 | |
| 157 | === modified file 'environs/filestorage/filestorage_test.go' |
| 158 | --- environs/filestorage/filestorage_test.go 2013-09-12 05:04:40 +0000 |
| 159 | +++ environs/filestorage/filestorage_test.go 2013-09-18 06:28:38 +0000 |
| 160 | @@ -10,6 +10,7 @@ |
| 161 | import ( |
| 162 | "bytes" |
| 163 | "io/ioutil" |
| 164 | + "os" |
| 165 | "path/filepath" |
| 166 | "testing" |
| 167 | |
| 168 | @@ -17,6 +18,8 @@ |
| 169 | |
| 170 | "launchpad.net/juju-core/environs" |
| 171 | "launchpad.net/juju-core/environs/filestorage" |
| 172 | + coreerrors "launchpad.net/juju-core/errors" |
| 173 | + jc "launchpad.net/juju-core/testing/checkers" |
| 174 | ) |
| 175 | |
| 176 | func TestPackage(t *testing.T) { |
| 177 | @@ -40,8 +43,10 @@ |
| 178 | c.Assert(err, gc.IsNil) |
| 179 | } |
| 180 | |
| 181 | -func (s *filestorageSuite) createFile(c *gc.C) (fullpath string, data []byte) { |
| 182 | - fullpath = filepath.Join(s.dir, "test-file") |
| 183 | +func (s *filestorageSuite) createFile(c *gc.C, name string) (fullpath string, data []byte) { |
| 184 | + fullpath = filepath.Join(s.dir, name) |
| 185 | + dir := filepath.Dir(fullpath) |
| 186 | + c.Assert(os.MkdirAll(dir, 0755), gc.IsNil) |
| 187 | data = []byte{1, 2, 3, 4, 5} |
| 188 | err := ioutil.WriteFile(fullpath, data, 0644) |
| 189 | c.Assert(err, gc.IsNil) |
| 190 | @@ -49,15 +54,35 @@ |
| 191 | } |
| 192 | |
| 193 | func (s *filestorageSuite) TestList(c *gc.C) { |
| 194 | - expectedpath, _ := s.createFile(c) |
| 195 | - files, err := s.reader.List("test-") |
| 196 | - c.Assert(err, gc.IsNil) |
| 197 | - _, file := filepath.Split(expectedpath) |
| 198 | - c.Assert(files, gc.DeepEquals, []string{file}) |
| 199 | + names := []string{ |
| 200 | + "a/b/c", |
| 201 | + "a/bb", |
| 202 | + "a/c", |
| 203 | + "aa", |
| 204 | + "b/c/d", |
| 205 | + } |
| 206 | + for _, name := range names { |
| 207 | + s.createFile(c, name) |
| 208 | + } |
| 209 | + type test struct { |
| 210 | + prefix string |
| 211 | + expected []string |
| 212 | + } |
| 213 | + for i, test := range []test{ |
| 214 | + {"a", []string{"a/b/c", "a/bb", "a/c", "aa"}}, |
| 215 | + {"a/b", []string{"a/b/c", "a/bb"}}, |
| 216 | + {"a/b/c", []string{"a/b/c"}}, |
| 217 | + {"", names}, |
| 218 | + } { |
| 219 | + c.Logf("test %d: prefix=%q", i, test.prefix) |
| 220 | + files, err := s.reader.List(test.prefix) |
| 221 | + c.Assert(err, gc.IsNil) |
| 222 | + c.Assert(files, gc.DeepEquals, test.expected) |
| 223 | + } |
| 224 | } |
| 225 | |
| 226 | func (s *filestorageSuite) TestURL(c *gc.C) { |
| 227 | - expectedpath, _ := s.createFile(c) |
| 228 | + expectedpath, _ := s.createFile(c, "test-file") |
| 229 | _, file := filepath.Split(expectedpath) |
| 230 | url, err := s.reader.URL(file) |
| 231 | c.Assert(err, gc.IsNil) |
| 232 | @@ -65,7 +90,7 @@ |
| 233 | } |
| 234 | |
| 235 | func (s *filestorageSuite) TestGet(c *gc.C) { |
| 236 | - expectedpath, data := s.createFile(c) |
| 237 | + expectedpath, data := s.createFile(c, "test-file") |
| 238 | _, file := filepath.Split(expectedpath) |
| 239 | rc, err := s.reader.Get(file) |
| 240 | c.Assert(err, gc.IsNil) |
| 241 | @@ -74,6 +99,15 @@ |
| 242 | b, err := ioutil.ReadAll(rc) |
| 243 | c.Assert(err, gc.IsNil) |
| 244 | c.Assert(b, gc.DeepEquals, data) |
| 245 | + |
| 246 | + // Get on a non-existant path returns NotFoundError |
| 247 | + _, err = s.reader.Get("nowhere") |
| 248 | + c.Assert(err, jc.Satisfies, coreerrors.IsNotFoundError) |
| 249 | + |
| 250 | + // Get on a directory returns NotFoundError |
| 251 | + s.createFile(c, "dir/file") |
| 252 | + _, err = s.reader.Get("dir") |
| 253 | + c.Assert(err, jc.Satisfies, coreerrors.IsNotFoundError) |
| 254 | } |
| 255 | |
| 256 | func (s *filestorageSuite) TestPut(c *gc.C) { |
| 257 | @@ -86,7 +120,7 @@ |
| 258 | } |
| 259 | |
| 260 | func (s *filestorageSuite) TestRemove(c *gc.C) { |
| 261 | - expectedpath, _ := s.createFile(c) |
| 262 | + expectedpath, _ := s.createFile(c, "test-file") |
| 263 | _, file := filepath.Split(expectedpath) |
| 264 | err := s.writer.Remove(file) |
| 265 | c.Assert(err, gc.IsNil) |
| 266 | @@ -95,7 +129,7 @@ |
| 267 | } |
| 268 | |
| 269 | func (s *filestorageSuite) TestRemoveAll(c *gc.C) { |
| 270 | - expectedpath, _ := s.createFile(c) |
| 271 | + expectedpath, _ := s.createFile(c, "test-file") |
| 272 | err := s.writer.RemoveAll() |
| 273 | c.Assert(err, gc.IsNil) |
| 274 | _, err = ioutil.ReadFile(expectedpath) |
| 275 | |
| 276 | === renamed directory 'environs/localstorage' => 'environs/httpstorage' |
| 277 | === modified file 'environs/httpstorage/backend.go' |
| 278 | --- environs/localstorage/backend.go 2013-07-03 00:15:19 +0000 |
| 279 | +++ environs/httpstorage/backend.go 2013-09-18 06:28:38 +0000 |
| 280 | @@ -1,26 +1,24 @@ |
| 281 | // Copyright 2013 Canonical Ltd. |
| 282 | // Licensed under the AGPLv3, see LICENCE file for details. |
| 283 | |
| 284 | -package localstorage |
| 285 | +package httpstorage |
| 286 | |
| 287 | import ( |
| 288 | "fmt" |
| 289 | - "io" |
| 290 | "io/ioutil" |
| 291 | "net" |
| 292 | "net/http" |
| 293 | - "os" |
| 294 | - "path/filepath" |
| 295 | - "sort" |
| 296 | "strings" |
| 297 | + |
| 298 | + "launchpad.net/juju-core/environs" |
| 299 | ) |
| 300 | |
| 301 | -// storageBackend provides HTTP access to a defined path. The local |
| 302 | -// provider otimally would use a much simpler Storage, but this |
| 303 | -// code may be useful in storage-free environs. Here it requires |
| 304 | -// additional authentication work before it's viable. |
| 305 | +// TODO(axw) 2013-09-16 bug #1225916 |
| 306 | +// Implement authentication for modifying storage. |
| 307 | + |
| 308 | +// storageBackend provides HTTP access to a storage object. |
| 309 | type storageBackend struct { |
| 310 | - dir string |
| 311 | + backend environs.Storage |
| 312 | } |
| 313 | |
| 314 | // ServeHTTP handles the HTTP requests to the container. |
| 315 | @@ -43,9 +41,15 @@ |
| 316 | |
| 317 | // handleGet returns a storage file to the client. |
| 318 | func (s *storageBackend) handleGet(w http.ResponseWriter, req *http.Request) { |
| 319 | - data, err := ioutil.ReadFile(filepath.Join(s.dir, req.URL.Path)) |
| 320 | - if err != nil { |
| 321 | - http.Error(w, fmt.Sprintf("404 %v", err), http.StatusNotFound) |
| 322 | + readcloser, err := s.backend.Get(req.URL.Path[1:]) |
| 323 | + if err != nil { |
| 324 | + http.Error(w, fmt.Sprint(err), http.StatusNotFound) |
| 325 | + return |
| 326 | + } |
| 327 | + defer readcloser.Close() |
| 328 | + data, err := ioutil.ReadAll(readcloser) |
| 329 | + if err != nil { |
| 330 | + http.Error(w, fmt.Sprint(err), http.StatusInternalServerError) |
| 331 | return |
| 332 | } |
| 333 | w.Header().Set("Content-Type", "application/octet-stream") |
| 334 | @@ -54,62 +58,27 @@ |
| 335 | |
| 336 | // handleList returns the file names in the storage to the client. |
| 337 | func (s *storageBackend) handleList(w http.ResponseWriter, req *http.Request) { |
| 338 | - fp := filepath.Join(s.dir, req.URL.Path) |
| 339 | - dir, prefix := filepath.Split(fp) |
| 340 | - names, err := readDirs(dir, prefix[:len(prefix)-1], len(s.dir)+1) |
| 341 | + prefix := req.URL.Path |
| 342 | + prefix = prefix[1 : len(prefix)-1] // drop the leading '/' and trailing '*' |
| 343 | + names, err := s.backend.List(prefix) |
| 344 | if err != nil { |
| 345 | - http.Error(w, fmt.Sprintf("404 %v", err), http.StatusNotFound) |
| 346 | + http.Error(w, fmt.Sprint(err), http.StatusInternalServerError) |
| 347 | return |
| 348 | } |
| 349 | - sort.Strings(names) |
| 350 | data := []byte(strings.Join(names, "\n")) |
| 351 | w.Header().Set("Content-Type", "application/octet-stream") |
| 352 | w.Write(data) |
| 353 | } |
| 354 | |
| 355 | -// readDirs reads the directory hierarchy and compares the found |
| 356 | -// names with the given prefix. |
| 357 | -func readDirs(dir, prefix string, start int) ([]string, error) { |
| 358 | - names := []string{} |
| 359 | - fis, err := ioutil.ReadDir(dir) |
| 360 | - if err != nil { |
| 361 | - return nil, err |
| 362 | - } |
| 363 | - for _, fi := range fis { |
| 364 | - name := fi.Name() |
| 365 | - if strings.HasPrefix(name, prefix) { |
| 366 | - if fi.IsDir() { |
| 367 | - dnames, err := readDirs(filepath.Join(dir, name), prefix, start) |
| 368 | - if err != nil { |
| 369 | - return nil, err |
| 370 | - } |
| 371 | - names = append(names, dnames...) |
| 372 | - continue |
| 373 | - } |
| 374 | - fullname := filepath.Join(dir, name)[start:] |
| 375 | - names = append(names, fullname) |
| 376 | - } |
| 377 | - } |
| 378 | - return names, nil |
| 379 | -} |
| 380 | - |
| 381 | // handlePut stores data from the client in the storage. |
| 382 | func (s *storageBackend) handlePut(w http.ResponseWriter, req *http.Request) { |
| 383 | - fp := filepath.Join(s.dir, req.URL.Path) |
| 384 | - dir, _ := filepath.Split(fp) |
| 385 | - err := os.MkdirAll(dir, 0777) |
| 386 | - if err != nil { |
| 387 | - http.Error(w, fmt.Sprintf("500 %v", err), http.StatusInternalServerError) |
| 388 | - return |
| 389 | - } |
| 390 | - out, err := os.Create(fp) |
| 391 | - if err != nil { |
| 392 | - http.Error(w, fmt.Sprintf("500 %v", err), http.StatusInternalServerError) |
| 393 | - return |
| 394 | - } |
| 395 | - defer out.Close() |
| 396 | - if _, err := io.Copy(out, req.Body); err != nil { |
| 397 | - http.Error(w, fmt.Sprintf("500 %v", err), http.StatusInternalServerError) |
| 398 | + if req.ContentLength < 0 { |
| 399 | + http.Error(w, "missing or invalid Content-Length header", http.StatusInternalServerError) |
| 400 | + return |
| 401 | + } |
| 402 | + err := s.backend.Put(req.URL.Path[1:], req.Body, req.ContentLength) |
| 403 | + if err != nil { |
| 404 | + http.Error(w, fmt.Sprint(err), http.StatusInternalServerError) |
| 405 | return |
| 406 | } |
| 407 | w.WriteHeader(http.StatusCreated) |
| 408 | @@ -117,28 +86,19 @@ |
| 409 | |
| 410 | // handleDelete removes a file from the storage. |
| 411 | func (s *storageBackend) handleDelete(w http.ResponseWriter, req *http.Request) { |
| 412 | - fp := filepath.Join(s.dir, req.URL.Path) |
| 413 | - if err := os.Remove(fp); err != nil && !os.IsNotExist(err) { |
| 414 | - http.Error(w, fmt.Sprintf("500 %v", err), http.StatusInternalServerError) |
| 415 | + err := s.backend.Remove(req.URL.Path[1:]) |
| 416 | + if err != nil { |
| 417 | + http.Error(w, fmt.Sprint(err), http.StatusInternalServerError) |
| 418 | return |
| 419 | } |
| 420 | w.WriteHeader(http.StatusOK) |
| 421 | } |
| 422 | |
| 423 | -// Serve runs a storage server on the given network address, storing |
| 424 | -// data under the given directory. It returns the network listener. |
| 425 | -// This can then be attached to with Client. |
| 426 | -func Serve(addr, dir string) (net.Listener, error) { |
| 427 | - backend := &storageBackend{ |
| 428 | - dir: dir, |
| 429 | - } |
| 430 | - info, err := os.Stat(dir) |
| 431 | - if err != nil { |
| 432 | - return nil, fmt.Errorf("cannot stat directory: %v", err) |
| 433 | - } |
| 434 | - if !info.IsDir() { |
| 435 | - return nil, fmt.Errorf("%q is not a directory", dir) |
| 436 | - } |
| 437 | +// Serve runs a storage server on the given network address, relaying |
| 438 | +// requests to the given storage implementation. It returns the network |
| 439 | +// listener. This can then be attached to with Client. |
| 440 | +func Serve(addr string, storage environs.Storage) (net.Listener, error) { |
| 441 | + backend := &storageBackend{backend: storage} |
| 442 | listener, err := net.Listen("tcp", addr) |
| 443 | if err != nil { |
| 444 | return nil, fmt.Errorf("cannot start listener: %v", err) |
| 445 | |
| 446 | === modified file 'environs/httpstorage/backend_test.go' |
| 447 | --- environs/localstorage/backend_test.go 2013-09-13 14:48:13 +0000 |
| 448 | +++ environs/httpstorage/backend_test.go 2013-09-18 06:28:38 +0000 |
| 449 | @@ -1,7 +1,7 @@ |
| 450 | // Copyright 2013 Canonical Ltd. |
| 451 | // Licensed under the AGPLv3, see LICENCE file for details. |
| 452 | |
| 453 | -package localstorage_test |
| 454 | +package httpstorage_test |
| 455 | |
| 456 | import ( |
| 457 | "bytes" |
| 458 | @@ -16,7 +16,8 @@ |
| 459 | |
| 460 | gc "launchpad.net/gocheck" |
| 461 | |
| 462 | - "launchpad.net/juju-core/environs/localstorage" |
| 463 | + "launchpad.net/juju-core/environs/filestorage" |
| 464 | + "launchpad.net/juju-core/environs/httpstorage" |
| 465 | "launchpad.net/juju-core/testing" |
| 466 | ) |
| 467 | |
| 468 | @@ -35,7 +36,9 @@ |
| 469 | // a base URL for the server and the directory path. |
| 470 | func startServer(c *gc.C) (listener net.Listener, url, dataDir string) { |
| 471 | dataDir = c.MkDir() |
| 472 | - listener, err := localstorage.Serve("localhost:0", dataDir) |
| 473 | + embedded, err := filestorage.NewFileStorageWriter(dataDir) |
| 474 | + c.Assert(err, gc.IsNil) |
| 475 | + listener, err = httpstorage.Serve("localhost:0", embedded) |
| 476 | c.Assert(err, gc.IsNil) |
| 477 | return listener, fmt.Sprintf("http://%s/", listener.Addr()), dataDir |
| 478 | } |
| 479 | |
| 480 | === modified file 'environs/httpstorage/storage.go' |
| 481 | --- environs/localstorage/storage.go 2013-09-02 04:11:11 +0000 |
| 482 | +++ environs/httpstorage/storage.go 2013-09-18 06:28:38 +0000 |
| 483 | @@ -1,7 +1,7 @@ |
| 484 | // Copyright 2013 Canonical Ltd. |
| 485 | // Licensed under the AGPLv3, see LICENCE file for details. |
| 486 | |
| 487 | -package localstorage |
| 488 | +package httpstorage |
| 489 | |
| 490 | import ( |
| 491 | "fmt" |
| 492 | @@ -112,6 +112,7 @@ |
| 493 | return err |
| 494 | } |
| 495 | req.Header.Set("Content-Type", "application/octet-stream") |
| 496 | + req.ContentLength = length |
| 497 | resp, err := http.DefaultClient.Do(req) |
| 498 | if err != nil { |
| 499 | return err |
| 500 | |
| 501 | === modified file 'environs/httpstorage/storage_test.go' |
| 502 | --- environs/localstorage/storage_test.go 2013-08-29 01:46:55 +0000 |
| 503 | +++ environs/httpstorage/storage_test.go 2013-09-18 06:28:38 +0000 |
| 504 | @@ -1,7 +1,7 @@ |
| 505 | // Copyright 2013 Canonical Ltd. |
| 506 | // Licensed under the AGPLv3, see LICENCE file for details. |
| 507 | |
| 508 | -package localstorage_test |
| 509 | +package httpstorage_test |
| 510 | |
| 511 | import ( |
| 512 | "bytes" |
| 513 | @@ -13,7 +13,7 @@ |
| 514 | gc "launchpad.net/gocheck" |
| 515 | |
| 516 | "launchpad.net/juju-core/environs" |
| 517 | - "launchpad.net/juju-core/environs/localstorage" |
| 518 | + "launchpad.net/juju-core/environs/httpstorage" |
| 519 | "launchpad.net/juju-core/errors" |
| 520 | jc "launchpad.net/juju-core/testing/checkers" |
| 521 | ) |
| 522 | @@ -25,7 +25,7 @@ |
| 523 | func (s *storageSuite) TestList(c *gc.C) { |
| 524 | listener, _, _ := startServer(c) |
| 525 | defer listener.Close() |
| 526 | - storage := localstorage.Client(listener.Addr().String()) |
| 527 | + storage := httpstorage.Client(listener.Addr().String()) |
| 528 | names, err := storage.List("a/b/c") |
| 529 | c.Assert(err, gc.IsNil) |
| 530 | c.Assert(names, gc.HasLen, 0) |
| 531 | @@ -37,7 +37,7 @@ |
| 532 | listener, _, _ := startServer(c) |
| 533 | defer listener.Close() |
| 534 | |
| 535 | - storage := localstorage.Client(listener.Addr().String()) |
| 536 | + storage := httpstorage.Client(listener.Addr().String()) |
| 537 | names := []string{ |
| 538 | "aa", |
| 539 | "zzz/aa", |
| 540 | @@ -51,7 +51,7 @@ |
| 541 | checkList(c, storage, "a", []string{"aa"}) |
| 542 | checkList(c, storage, "zzz/", []string{"zzz/aa", "zzz/bb"}) |
| 543 | |
| 544 | - storage2 := localstorage.Client(listener.Addr().String()) |
| 545 | + storage2 := httpstorage.Client(listener.Addr().String()) |
| 546 | for _, name := range names { |
| 547 | checkFileHasContents(c, storage2, name, []byte(name)) |
| 548 | } |
| 549 | |
| 550 | === modified file 'environs/testing/storage.go' |
| 551 | --- environs/testing/storage.go 2013-09-17 05:29:35 +0000 |
| 552 | +++ environs/testing/storage.go 2013-09-18 06:28:38 +0000 |
| 553 | @@ -17,7 +17,8 @@ |
| 554 | gc "launchpad.net/gocheck" |
| 555 | |
| 556 | "launchpad.net/juju-core/environs" |
| 557 | - "launchpad.net/juju-core/environs/localstorage" |
| 558 | + "launchpad.net/juju-core/environs/filestorage" |
| 559 | + "launchpad.net/juju-core/environs/httpstorage" |
| 560 | "launchpad.net/juju-core/environs/tools" |
| 561 | "launchpad.net/juju-core/version" |
| 562 | ) |
| 563 | @@ -27,9 +28,11 @@ |
| 564 | // directory. |
| 565 | func CreateLocalTestStorage(c *gc.C) (closer io.Closer, storage environs.Storage, dataDir string) { |
| 566 | dataDir = c.MkDir() |
| 567 | - listener, err := localstorage.Serve("localhost:0", dataDir) |
| 568 | - c.Assert(err, gc.IsNil) |
| 569 | - storage = localstorage.Client(listener.Addr().String()) |
| 570 | + underlying, err := filestorage.NewFileStorageWriter(dataDir) |
| 571 | + c.Assert(err, gc.IsNil) |
| 572 | + listener, err := httpstorage.Serve("localhost:0", underlying) |
| 573 | + c.Assert(err, gc.IsNil) |
| 574 | + storage = httpstorage.Client(listener.Addr().String()) |
| 575 | closer = listener |
| 576 | return |
| 577 | } |
| 578 | |
| 579 | === modified file 'provider/azure/environ_test.go' |
| 580 | --- provider/azure/environ_test.go 2013-09-12 14:16:48 +0000 |
| 581 | +++ provider/azure/environ_test.go 2013-09-18 06:28:38 +0000 |
| 582 | @@ -23,8 +23,8 @@ |
| 583 | "launchpad.net/juju-core/environs" |
| 584 | "launchpad.net/juju-core/environs/config" |
| 585 | "launchpad.net/juju-core/environs/imagemetadata" |
| 586 | - "launchpad.net/juju-core/environs/localstorage" |
| 587 | "launchpad.net/juju-core/environs/simplestreams" |
| 588 | + envtesting "launchpad.net/juju-core/environs/testing" |
| 589 | "launchpad.net/juju-core/environs/tools" |
| 590 | "launchpad.net/juju-core/errors" |
| 591 | "launchpad.net/juju-core/instance" |
| 592 | @@ -54,12 +54,10 @@ |
| 593 | // setDummyStorage injects the local provider's fake storage implementation |
| 594 | // into the given environment, so that tests can manipulate storage as if it |
| 595 | // were real. |
| 596 | -// Returns a cleanup function that must be called when done with the storage. |
| 597 | -func setDummyStorage(c *gc.C, env *azureEnviron) func() { |
| 598 | - listener, err := localstorage.Serve("127.0.0.1:0", c.MkDir()) |
| 599 | - c.Assert(err, gc.IsNil) |
| 600 | - env.storage = localstorage.Client(listener.Addr().String()) |
| 601 | - return func() { listener.Close() } |
| 602 | +func (s *environSuite) setDummyStorage(c *gc.C, env *azureEnviron) { |
| 603 | + closer, storage, _ := envtesting.CreateLocalTestStorage(c) |
| 604 | + env.storage = storage |
| 605 | + s.AddCleanup(func(c *gc.C) { closer.Close() }) |
| 606 | } |
| 607 | |
| 608 | func (*environSuite) TestGetSnapshot(c *gc.C) { |
| 609 | @@ -469,22 +467,20 @@ |
| 610 | c.Check(env.storageAccountKey, gc.Equals, "") |
| 611 | } |
| 612 | |
| 613 | -func (*environSuite) TestStateInfoFailsIfNoStateInstances(c *gc.C) { |
| 614 | +func (s *environSuite) TestStateInfoFailsIfNoStateInstances(c *gc.C) { |
| 615 | env := makeEnviron(c) |
| 616 | - cleanup := setDummyStorage(c, env) |
| 617 | - defer cleanup() |
| 618 | + s.setDummyStorage(c, env) |
| 619 | _, _, err := env.StateInfo() |
| 620 | c.Check(err, jc.Satisfies, errors.IsNotBootstrapped) |
| 621 | } |
| 622 | |
| 623 | -func (*environSuite) TestStateInfo(c *gc.C) { |
| 624 | +func (s *environSuite) TestStateInfo(c *gc.C) { |
| 625 | instanceID := "my-instance" |
| 626 | patchWithServiceListResponse(c, []gwacl.HostedServiceDescriptor{{ |
| 627 | ServiceName: instanceID, |
| 628 | }}) |
| 629 | env := makeEnviron(c) |
| 630 | - cleanup := setDummyStorage(c, env) |
| 631 | - defer cleanup() |
| 632 | + s.setDummyStorage(c, env) |
| 633 | err := provider.SaveState( |
| 634 | env.Storage(), |
| 635 | &provider.BootstrapState{StateInstances: []instance.Id{instance.Id(instanceID)}}) |
| 636 | @@ -705,15 +701,13 @@ |
| 637 | return service1, service1Desc |
| 638 | } |
| 639 | |
| 640 | -func setServiceDeletionConcurrency(nbGoroutines int) func() { |
| 641 | - oldMaxConcurrentDeletes := maxConcurrentDeletes |
| 642 | - maxConcurrentDeletes = nbGoroutines |
| 643 | - return func() { maxConcurrentDeletes = oldMaxConcurrentDeletes } |
| 644 | +func (s *environSuite) setServiceDeletionConcurrency(nbGoroutines int) { |
| 645 | + restore := jc.Set(&maxConcurrentDeletes, nbGoroutines) |
| 646 | + s.AddCleanup(func(*gc.C) { restore() }) |
| 647 | } |
| 648 | |
| 649 | -func (*environSuite) TestStopInstancesDestroysMachines(c *gc.C) { |
| 650 | - cleanup := setServiceDeletionConcurrency(3) |
| 651 | - defer cleanup() |
| 652 | +func (s *environSuite) TestStopInstancesDestroysMachines(c *gc.C) { |
| 653 | + s.setServiceDeletionConcurrency(3) |
| 654 | service1Name := "service1" |
| 655 | service1, service1Desc := makeAzureService(service1Name) |
| 656 | service2Name := "service2" |
| 657 | @@ -739,9 +733,8 @@ |
| 658 | assertOneRequestMatches(c, *requests, "DELETE", ".*"+service2Name+".*") |
| 659 | } |
| 660 | |
| 661 | -func (*environSuite) TestStopInstancesWhenStoppingMachinesFails(c *gc.C) { |
| 662 | - cleanup := setServiceDeletionConcurrency(3) |
| 663 | - defer cleanup() |
| 664 | +func (s *environSuite) TestStopInstancesWhenStoppingMachinesFails(c *gc.C) { |
| 665 | + s.setServiceDeletionConcurrency(3) |
| 666 | responses := []gwacl.DispatcherResponse{ |
| 667 | gwacl.NewDispatcherResponse(nil, http.StatusConflict, nil), |
| 668 | } |
| 669 | @@ -767,9 +760,8 @@ |
| 670 | assertOneRequestMatches(c, *requests, "DELETE", ".*") |
| 671 | } |
| 672 | |
| 673 | -func (*environSuite) TestStopInstancesWithLimitedConcurrency(c *gc.C) { |
| 674 | - cleanup := setServiceDeletionConcurrency(3) |
| 675 | - defer cleanup() |
| 676 | +func (s *environSuite) TestStopInstancesWithLimitedConcurrency(c *gc.C) { |
| 677 | + s.setServiceDeletionConcurrency(3) |
| 678 | services := []*gwacl.HostedService{} |
| 679 | serviceDescs := []gwacl.HostedServiceDescriptor{} |
| 680 | for i := 0; i < 10; i++ { |
| 681 | @@ -788,9 +780,8 @@ |
| 682 | c.Check(len(*requests), gc.Equals, len(services)*2) |
| 683 | } |
| 684 | |
| 685 | -func (*environSuite) TestStopInstancesWithZeroInstance(c *gc.C) { |
| 686 | - cleanup := setServiceDeletionConcurrency(3) |
| 687 | - defer cleanup() |
| 688 | +func (s *environSuite) TestStopInstancesWithZeroInstance(c *gc.C) { |
| 689 | + s.setServiceDeletionConcurrency(3) |
| 690 | env := makeEnviron(c) |
| 691 | instances := []instance.Instance{} |
| 692 | |
| 693 | @@ -818,10 +809,9 @@ |
| 694 | return cleanupResponses |
| 695 | } |
| 696 | |
| 697 | -func (*environSuite) TestDestroyDoesNotCleanStorageIfError(c *gc.C) { |
| 698 | +func (s *environSuite) TestDestroyDoesNotCleanStorageIfError(c *gc.C) { |
| 699 | env := makeEnviron(c) |
| 700 | - cleanup := setDummyStorage(c, env) |
| 701 | - defer cleanup() |
| 702 | + s.setDummyStorage(c, env) |
| 703 | // Populate storage. |
| 704 | err := provider.SaveState( |
| 705 | env.Storage(), |
| 706 | @@ -840,10 +830,9 @@ |
| 707 | c.Check(files, gc.HasLen, 1) |
| 708 | } |
| 709 | |
| 710 | -func (*environSuite) TestDestroyCleansUpStorage(c *gc.C) { |
| 711 | +func (s *environSuite) TestDestroyCleansUpStorage(c *gc.C) { |
| 712 | env := makeEnviron(c) |
| 713 | - cleanup := setDummyStorage(c, env) |
| 714 | - defer cleanup() |
| 715 | + s.setDummyStorage(c, env) |
| 716 | // Populate storage. |
| 717 | err := provider.SaveState( |
| 718 | env.Storage(), |
| 719 | @@ -864,10 +853,9 @@ |
| 720 | c.Check(files, gc.HasLen, 0) |
| 721 | } |
| 722 | |
| 723 | -func (*environSuite) TestDestroyDeletesVirtualNetworkAndAffinityGroup(c *gc.C) { |
| 724 | +func (s *environSuite) TestDestroyDeletesVirtualNetworkAndAffinityGroup(c *gc.C) { |
| 725 | env := makeEnviron(c) |
| 726 | - cleanup := setDummyStorage(c, env) |
| 727 | - defer cleanup() |
| 728 | + s.setDummyStorage(c, env) |
| 729 | services := []gwacl.HostedServiceDescriptor{} |
| 730 | responses := getAzureServiceListResponse(c, services) |
| 731 | // Prepare a configuration with a single virtual network. |
| 732 | @@ -933,12 +921,10 @@ |
| 733 | c.Error(fmt.Sprintf("none of the requests matches: Method=%v, URL pattern=%v", method, urlPattern)) |
| 734 | } |
| 735 | |
| 736 | -func (*environSuite) TestDestroyStopsAllInstances(c *gc.C) { |
| 737 | - cleanup1 := setServiceDeletionConcurrency(3) |
| 738 | - defer cleanup1() |
| 739 | +func (s *environSuite) TestDestroyStopsAllInstances(c *gc.C) { |
| 740 | + s.setServiceDeletionConcurrency(3) |
| 741 | env := makeEnviron(c) |
| 742 | - cleanup2 := setDummyStorage(c, env) |
| 743 | - defer cleanup2() |
| 744 | + s.setDummyStorage(c, env) |
| 745 | |
| 746 | // Simulate 2 instances corresponding to two Azure services. |
| 747 | prefix := env.getEnvPrefix() |
| 748 | @@ -1300,10 +1286,9 @@ |
| 749 | c.Assert(retrieved, gc.DeepEquals, content) |
| 750 | } |
| 751 | |
| 752 | -func (*environSuite) TestGetImageMetadataSources(c *gc.C) { |
| 753 | +func (s *environSuite) TestGetImageMetadataSources(c *gc.C) { |
| 754 | env := makeEnviron(c) |
| 755 | - cleanup := setDummyStorage(c, env) |
| 756 | - defer cleanup() |
| 757 | + s.setDummyStorage(c, env) |
| 758 | |
| 759 | data := []byte{1, 2, 3, 4} |
| 760 | env.Storage().Put("filename", bytes.NewReader(data), int64(len(data))) |
| 761 | @@ -1320,10 +1305,9 @@ |
| 762 | c.Assert(url, gc.Equals, imagemetadata.DefaultBaseURL+"/") |
| 763 | } |
| 764 | |
| 765 | -func (*environSuite) TestGetToolsMetadataSources(c *gc.C) { |
| 766 | +func (s *environSuite) TestGetToolsMetadataSources(c *gc.C) { |
| 767 | env := makeEnviron(c) |
| 768 | - cleanup := setDummyStorage(c, env) |
| 769 | - defer cleanup() |
| 770 | + s.setDummyStorage(c, env) |
| 771 | |
| 772 | data := []byte{1, 2, 3, 4} |
| 773 | env.Storage().Put("tools/filename", bytes.NewReader(data), int64(len(data))) |
| 774 | |
| 775 | === modified file 'provider/local/environ.go' |
| 776 | --- provider/local/environ.go 2013-09-16 22:02:24 +0000 |
| 777 | +++ provider/local/environ.go 2013-09-18 06:28:38 +0000 |
| 778 | @@ -21,7 +21,8 @@ |
| 779 | "launchpad.net/juju-core/environs/bootstrap" |
| 780 | "launchpad.net/juju-core/environs/cloudinit" |
| 781 | "launchpad.net/juju-core/environs/config" |
| 782 | - "launchpad.net/juju-core/environs/localstorage" |
| 783 | + "launchpad.net/juju-core/environs/filestorage" |
| 784 | + "launchpad.net/juju-core/environs/httpstorage" |
| 785 | "launchpad.net/juju-core/instance" |
| 786 | "launchpad.net/juju-core/juju/osenv" |
| 787 | "launchpad.net/juju-core/names" |
| 788 | @@ -164,7 +165,11 @@ |
| 789 | } else if !info.Mode().IsDir() { |
| 790 | return nil, fmt.Errorf("%q exists but is not a directory (and it needs to be)", dir) |
| 791 | } |
| 792 | - return localstorage.Serve(address, dir) |
| 793 | + storage, err := filestorage.NewFileStorageWriter(dir) |
| 794 | + if err != nil { |
| 795 | + return nil, err |
| 796 | + } |
| 797 | + return httpstorage.Serve(address, storage) |
| 798 | } |
| 799 | |
| 800 | // SetConfig is specified in the Environ interface. |
| 801 | @@ -319,12 +324,12 @@ |
| 802 | |
| 803 | // Storage is specified in the Environ interface. |
| 804 | func (env *localEnviron) Storage() environs.Storage { |
| 805 | - return localstorage.Client(env.config.storageAddr()) |
| 806 | + return httpstorage.Client(env.config.storageAddr()) |
| 807 | } |
| 808 | |
| 809 | // PublicStorage is specified in the Environ interface. |
| 810 | func (env *localEnviron) PublicStorage() environs.StorageReader { |
| 811 | - return localstorage.Client(env.config.sharedStorageAddr()) |
| 812 | + return httpstorage.Client(env.config.sharedStorageAddr()) |
| 813 | } |
| 814 | |
| 815 | // Destroy is specified in the Environ interface. |
| 816 | |
| 817 | === modified file 'provider/local/storage/worker.go' |
| 818 | --- provider/local/storage/worker.go 2013-08-30 04:07:04 +0000 |
| 819 | +++ provider/local/storage/worker.go 2013-09-18 06:28:38 +0000 |
| 820 | @@ -5,7 +5,8 @@ |
| 821 | "launchpad.net/tomb" |
| 822 | |
| 823 | "launchpad.net/juju-core/agent" |
| 824 | - "launchpad.net/juju-core/environs/localstorage" |
| 825 | + "launchpad.net/juju-core/environs/filestorage" |
| 826 | + "launchpad.net/juju-core/environs/httpstorage" |
| 827 | "launchpad.net/juju-core/worker" |
| 828 | ) |
| 829 | |
| 830 | @@ -40,7 +41,12 @@ |
| 831 | storageAddr := s.config.Value(agent.StorageAddr) |
| 832 | logger.Infof("serving %s on %s", storageDir, storageAddr) |
| 833 | |
| 834 | - storageListener, err := localstorage.Serve(storageAddr, storageDir) |
| 835 | + storage, err := filestorage.NewFileStorageWriter(storageDir) |
| 836 | + if err != nil { |
| 837 | + logger.Errorf("error with local storage: %v", err) |
| 838 | + return err |
| 839 | + } |
| 840 | + storageListener, err := httpstorage.Serve(storageAddr, storage) |
| 841 | if err != nil { |
| 842 | logger.Errorf("error with local storage: %v", err) |
| 843 | return err |
| 844 | @@ -51,7 +57,12 @@ |
| 845 | sharedStorageAddr := s.config.Value(agent.SharedStorageAddr) |
| 846 | logger.Infof("serving %s on %s", sharedStorageDir, sharedStorageAddr) |
| 847 | |
| 848 | - sharedStorageListener, err := localstorage.Serve(sharedStorageAddr, sharedStorageDir) |
| 849 | + sharedStorage, err := filestorage.NewFileStorageWriter(sharedStorageDir) |
| 850 | + if err != nil { |
| 851 | + logger.Errorf("error with local storage: %v", err) |
| 852 | + return err |
| 853 | + } |
| 854 | + sharedStorageListener, err := httpstorage.Serve(sharedStorageAddr, sharedStorage) |
| 855 | if err != nil { |
| 856 | logger.Errorf("error with local storage: %v", err) |
| 857 | return err |
| 858 | |
| 859 | === modified file 'provider/state_test.go' |
| 860 | --- provider/state_test.go 2013-09-12 12:38:04 +0000 |
| 861 | +++ provider/state_test.go 2013-09-18 06:28:38 +0000 |
| 862 | @@ -12,7 +12,7 @@ |
| 863 | |
| 864 | "launchpad.net/juju-core/environs" |
| 865 | "launchpad.net/juju-core/environs/config" |
| 866 | - "launchpad.net/juju-core/environs/localstorage" |
| 867 | + envtesting "launchpad.net/juju-core/environs/testing" |
| 868 | "launchpad.net/juju-core/errors" |
| 869 | "launchpad.net/juju-core/instance" |
| 870 | "launchpad.net/juju-core/provider" |
| 871 | @@ -20,23 +20,21 @@ |
| 872 | jc "launchpad.net/juju-core/testing/checkers" |
| 873 | ) |
| 874 | |
| 875 | -type StateSuite struct{} |
| 876 | +type StateSuite struct { |
| 877 | + testing.LoggingSuite |
| 878 | +} |
| 879 | |
| 880 | var _ = gc.Suite(&StateSuite{}) |
| 881 | |
| 882 | // makeDummyStorage creates a local storage. |
| 883 | -// Returns a cleanup function that must be called when done with the storage. |
| 884 | -func makeDummyStorage(c *gc.C) (environs.Storage, func()) { |
| 885 | - listener, err := localstorage.Serve("127.0.0.1:0", c.MkDir()) |
| 886 | - c.Assert(err, gc.IsNil) |
| 887 | - storage := localstorage.Client(listener.Addr().String()) |
| 888 | - cleanup := func() { listener.Close() } |
| 889 | - return storage, cleanup |
| 890 | +func (suite *StateSuite) makeDummyStorage(c *gc.C) environs.Storage { |
| 891 | + closer, storage, _ := envtesting.CreateLocalTestStorage(c) |
| 892 | + suite.AddCleanup(func(*gc.C) { closer.Close() }) |
| 893 | + return storage |
| 894 | } |
| 895 | |
| 896 | -func (*StateSuite) TestCreateStateFileWritesEmptyStateFile(c *gc.C) { |
| 897 | - storage, cleanup := makeDummyStorage(c) |
| 898 | - defer cleanup() |
| 899 | +func (suite *StateSuite) TestCreateStateFileWritesEmptyStateFile(c *gc.C) { |
| 900 | + storage := suite.makeDummyStorage(c) |
| 901 | |
| 902 | url, err := provider.CreateStateFile(storage) |
| 903 | c.Assert(err, gc.IsNil) |
| 904 | @@ -53,8 +51,7 @@ |
| 905 | } |
| 906 | |
| 907 | func (suite *StateSuite) TestSaveStateWritesStateFile(c *gc.C) { |
| 908 | - storage, cleanup := makeDummyStorage(c) |
| 909 | - defer cleanup() |
| 910 | + storage := suite.makeDummyStorage(c) |
| 911 | arch := "amd64" |
| 912 | state := provider.BootstrapState{ |
| 913 | StateInstances: []instance.Id{instance.Id("an-instance-id")}, |
| 914 | @@ -85,8 +82,7 @@ |
| 915 | } |
| 916 | |
| 917 | func (suite *StateSuite) TestLoadStateReadsStateFile(c *gc.C) { |
| 918 | - storage, cleanup := makeDummyStorage(c) |
| 919 | - defer cleanup() |
| 920 | + storage := suite.makeDummyStorage(c) |
| 921 | state := suite.setUpSavedState(c, storage) |
| 922 | storedState, err := provider.LoadState(storage) |
| 923 | c.Assert(err, gc.IsNil) |
| 924 | @@ -94,8 +90,7 @@ |
| 925 | } |
| 926 | |
| 927 | func (suite *StateSuite) TestLoadStateFromURLReadsStateFile(c *gc.C) { |
| 928 | - storage, cleanup := makeDummyStorage(c) |
| 929 | - defer cleanup() |
| 930 | + storage := suite.makeDummyStorage(c) |
| 931 | state := suite.setUpSavedState(c, storage) |
| 932 | url, err := storage.URL(provider.StateFile) |
| 933 | c.Assert(err, gc.IsNil) |
| 934 | @@ -105,8 +100,7 @@ |
| 935 | } |
| 936 | |
| 937 | func (suite *StateSuite) TestLoadStateMissingFile(c *gc.C) { |
| 938 | - storage, cleanup := makeDummyStorage(c) |
| 939 | - defer cleanup() |
| 940 | + storage := suite.makeDummyStorage(c) |
| 941 | |
| 942 | _, err := provider.LoadState(storage) |
| 943 | |
| 944 | @@ -114,8 +108,7 @@ |
| 945 | } |
| 946 | |
| 947 | func (suite *StateSuite) TestLoadStateIntegratesWithSaveState(c *gc.C) { |
| 948 | - storage, cleanup := makeDummyStorage(c) |
| 949 | - defer cleanup() |
| 950 | + storage := suite.makeDummyStorage(c) |
| 951 | arch := "amd64" |
| 952 | state := provider.BootstrapState{ |
| 953 | StateInstances: []instance.Id{instance.Id("an-instance-id")}, |
Reviewers: mp+185958_
Message:
Please take a look.
Description:
environs/
localstorage becomes httpstorage; it no longer
operates on the disk, but instead delegates to
another environs.Storage object.
This work goes together with changes to
environs/
write to temporary storage and atomically move,
in the same way that sshstorage has been coded.
https:/
(do not edit description out of merge proposal)
Please review this at https:/
Affected files (+129, -133 lines):
A [revision details]
M cmd/jujud/
M environs/
M environs/
M environs/
M environs/
M environs/
M environs/
M environs/
M provider/
M provider/
M provider/
M provider/