state;apiserver: Fix a race - lp bug #1067979

state/apiserver/client/client.go

 // Copyright 2013 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package client
 
 import (
         "fmt"
         "net/url"
         "os"
         "strings"
 
-        "launchpad.net/loggo"
+        "github.com/loggo/loggo"
 
         "launchpad.net/juju-core/charm"
         coreCloudinit "launchpad.net/juju-core/cloudinit"
         "launchpad.net/juju-core/cloudinit/sshinit"
         "launchpad.net/juju-core/environs"
         "launchpad.net/juju-core/environs/cloudinit"
         "launchpad.net/juju-core/environs/config"
         "launchpad.net/juju-core/errors"
         "launchpad.net/juju-core/instance"
         "launchpad.net/juju-core/juju"
@@ skipping 577 matching lines @@
 }
 
 // ProvisioningScript returns a shell script that, when run,
 // provisions a machine agent on the machine executing the script.
 func (c *Client) ProvisioningScript(args params.ProvisioningScriptParams) (params.ProvisioningScriptResult, error) {
         var result params.ProvisioningScriptResult
         mcfg, err := statecmd.MachineConfig(c.api.state, args.MachineId, args.Nonce, args.DataDir)
         if err != nil {
                 return result, err
         }
+        mcfg.DisablePackageCommands = args.DisablePackageCommands
         cloudcfg := coreCloudinit.New()
         if err := cloudinit.ConfigureJuju(mcfg, cloudcfg); err != nil {
                 return result, err
         }
         // ProvisioningScript is run on an existing machine;
         // we explicitly disable apt_upgrade so as not to
         // trample the machine's existing configuration.
         cloudcfg.SetAptUpgrade(false)
         result.Script, err = sshinit.ConfigureScript(cloudcfg)
         return result, err
@@ skipping 250 matching lines @@
         if _, err := archive.Seek(0, 0); err != nil {
                 return fmt.Errorf("cannot rewind charm archive: %v", err)
         }
 
         // Get the environment storage and upload the charm.
         env, err := environs.New(envConfig)
         if err != nil {
                 return fmt.Errorf("cannot access environment: %v", err)
         }
         storage := env.Storage()
-        archiveName, err := state.CharmArchiveName(charmURL.Name, charmURL.Revision)
+        archiveName, err := CharmArchiveName(charmURL.Name, charmURL.Revision)
         if err != nil {
                 return fmt.Errorf("cannot generate charm archive name: %v", err)
-        }
-        storageURL, err := storage.URL(archiveName)
-        if err != nil {
-                return fmt.Errorf("cannot get storage URL for charm: %v", err)
-        }
-        bundleURL, err := url.Parse(storageURL)
-        if err != nil {
-                return fmt.Errorf("cannot parse storage URL: %v", err)
         }
         if err := storage.Put(archiveName, archive, size); err != nil {
                 return fmt.Errorf("cannot upload charm to provider storage: %v", err)
+        }
+        storageURL, err := storage.URL(archiveName)
+        if err != nil {
+                return fmt.Errorf("cannot get storage URL for charm: %v", err)
+        }
+        bundleURL, err := url.Parse(storageURL)
+        if err != nil {
+                return fmt.Errorf("cannot parse storage URL: %v", err)
         }
 
         // Finally, update the charm data in state and mark it as no longer pending.
         _, err = c.api.state.UpdateUploadedCharm(downloadedCharm, charmURL, bundleURL, bundleSHA256)
         if err == state.ErrCharmRevisionAlreadyModified ||
                 state.IsCharmAlreadyUploadedError(err) {
                 // This is not an error, it just signifies somebody else
                 // managed to upload and update the charm in state before
                 // us. This means we have to delete what we just uploaded
                 // to storage.
                 if err := storage.Remove(archiveName); err != nil {
                         logger.Errorf("cannot remove duplicated charm from storage: %v", err)
                 }
                 return nil
         }
         return err
 }
+
+// CharmArchiveName returns a string that is suitable as a file name
+// in a storage URL. It is constructed from the charm name, revision
+// and a random UUID string.
+func CharmArchiveName(name string, revision int) (string, error) {
+        uuid, err := utils.NewUUID()
+        if err != nil {
+                return "", err
+        }
+        return charm.Quote(fmt.Sprintf("%s-%d-%s", name, revision, uuid)), nil
+}