← Back to branch summary

~jameinel/juju-core/scale-testing

« back to all changes in this revision

Viewing changes to state/apiserver/client/client.go

[r=dimitern],[bug=1067979] state;apiserver: Fix a race - lp bug #1067979

This introduces some changes to how charm store
charms are added through the API (in state and
to provider storage). Now PrepareStoreCharmUpload
is called before trying to download the charm,
repackage it and upload it to storage, in order
to reserve a charm URL in state with pending
state. Added a test that demonstrates multiple
concurrent deployments of the same charm does
not cause the race issues, like mentioned in
the bug.

A few drive-by fixes brought up during review:
* Added ReadSHA256 and ReadFileSHA256 helpers in
  utils, and changed most places where hashes
  are calculated to use them.
* Charms are now uploaded to storage with a
  randomly generated archive names with the
  format "<charm-name>-<revision>-<uuid>".
  This allows multiple concurrent uploads
  to happen safely, and at the end AddCharm
  in the API checks to see if the charm info
  is already updated in state and if so, deletes
  the duplicated upload.
* Added GetEnvironStorage helper to environs/testing.
* Fixed potential compatibility issues with older
  versions and the recently added PendingUpload
  and Placeholder fields of the charm document.

Also tested multiple concurrent deployments with
the local provider manually and updated the bug
accordingly.

https://codereview.appspot.com/53210044/

R=fwereade

Show diffs side-by-side

added added

removed removed

Lines of Context:
state/apiserver/client/client.go
4
4
package client
5
5
 
6
6
import (
7
 
        "crypto/sha256"
8
 
        "encoding/hex"
9
7
        "fmt"
10
 
        "io"
11
8
        "net/url"
12
9
        "os"
13
10
        "strings"
29
26
        "launchpad.net/juju-core/state/api/params"
30
27
        "launchpad.net/juju-core/state/apiserver/common"
31
28
        "launchpad.net/juju-core/state/statecmd"
 
29
        "launchpad.net/juju-core/utils"
32
30
)
33
31
 
34
32
var logger = loggo.GetLogger("juju.state.apiserver.client")
836
834
                return fmt.Errorf("charm URL must include revision")
837
835
        }
838
836
 
839
 
        // First check the charm is not already in state.
840
 
        if _, err := c.api.state.Charm(charmURL); err == nil {
 
837
        // First, check if a pending or a real charm exists in state.
 
838
        stateCharm, err := c.api.state.PrepareStoreCharmUpload(charmURL)
 
839
        if err == nil && stateCharm.IsUploaded() {
 
840
                // Charm already in state (it was uploaded already).
841
841
                return nil
 
842
        } else if err != nil {
 
843
                return err
842
844
        }
843
845
 
844
846
        // Get the charm and its information from the store.
862
864
                return fmt.Errorf("cannot read downloaded charm: %v", err)
863
865
        }
864
866
        defer archive.Close()
865
 
        hash := sha256.New()
866
 
        size, err := io.Copy(hash, archive)
 
867
        bundleSHA256, size, err := utils.ReadSHA256(archive)
867
868
        if err != nil {
868
869
                return fmt.Errorf("cannot calculate SHA256 hash of charm: %v", err)
869
870
        }
870
 
        bundleSHA256 := hex.EncodeToString(hash.Sum(nil))
871
871
        if _, err := archive.Seek(0, 0); err != nil {
872
872
                return fmt.Errorf("cannot rewind charm archive: %v", err)
873
873
        }
878
878
                return fmt.Errorf("cannot access environment: %v", err)
879
879
        }
880
880
        storage := env.Storage()
881
 
        name := charm.Quote(charmURL.String())
882
 
        err = storage.Put(name, archive, size)
 
881
        archiveName, err := CharmArchiveName(charmURL.Name, charmURL.Revision)
883
882
        if err != nil {
 
883
                return fmt.Errorf("cannot generate charm archive name: %v", err)
 
884
        }
 
885
        if err := storage.Put(archiveName, archive, size); err != nil {
884
886
                return fmt.Errorf("cannot upload charm to provider storage: %v", err)
885
887
        }
886
 
        storageURL, err := storage.URL(name)
 
888
        storageURL, err := storage.URL(archiveName)
887
889
        if err != nil {
888
890
                return fmt.Errorf("cannot get storage URL for charm: %v", err)
889
891
        }
892
894
                return fmt.Errorf("cannot parse storage URL: %v", err)
893
895
        }
894
896
 
895
 
        // Finally, add the charm to state.
896
 
        _, err = c.api.state.AddCharm(downloadedCharm, charmURL, bundleURL, bundleSHA256)
 
897
        // Finally, update the charm data in state and mark it as no longer pending.
 
898
        _, err = c.api.state.UpdateUploadedCharm(downloadedCharm, charmURL, bundleURL, bundleSHA256)
 
899
        if err == state.ErrCharmRevisionAlreadyModified ||
 
900
                state.IsCharmAlreadyUploadedError(err) {
 
901
                // This is not an error, it just signifies somebody else
 
902
                // managed to upload and update the charm in state before
 
903
                // us. This means we have to delete what we just uploaded
 
904
                // to storage.
 
905
                if err := storage.Remove(archiveName); err != nil {
 
906
                        logger.Errorf("cannot remove duplicated charm from storage: %v", err)
 
907
                }
 
908
                return nil
 
909
        }
897
910
        return err
898
911
}
 
912
 
 
913
// CharmArchiveName returns a string that is suitable as a file name
 
914
// in a storage URL. It is constructed from the charm name, revision
 
915
// and a random UUID string.
 
916
func CharmArchiveName(name string, revision int) (string, error) {
 
917
        uuid, err := utils.NewUUID()
 
918
        if err != nil {
 
919
                return "", err
 
920
        }
 
921
        return charm.Quote(fmt.Sprintf("%s-%d-%s", name, revision, uuid)), nil
 
922
}