Merge pull request #120 from jackgr/encapsulate-manifests

Remove direct access to manifests to prepare for persistence..

manager/manager/manager.go

@@ -60,10 +60,7 @@ func (m *manager) GetDeployment(name string) (*Deployment, error) {
 	if err != nil {
 		return nil, err
 	}
-	latest := getLatestManifest(d.Manifests)
-	if latest != nil {
-		d.Current = latest.ExpandedConfig
-	}
+
 	return d, nil
 }
 
@@ -74,6 +71,7 @@ func (m *manager) ListManifests(deploymentName string) (map[string]*Manifest, er
 	if err != nil {
 		return nil, err
 	}
+
 	return l, nil
 }
 
@@ -83,6 +81,7 @@ func (m *manager) GetManifest(deploymentName string, manifestName string) (*Mani
 	if err != nil {
 		return nil, err
 	}
+
 	return d, nil
 }
 
@@ -118,7 +117,7 @@ func (m *manager) CreateDeployment(t *Template) (*Deployment, error) {
 	} else {
 		m.repository.SetDeploymentStatus(t.Name, DeployedStatus)
 	}
-	
+
 	// Update the manifest with the actual state of the reified resources
 	manifest.ExpandedConfig = actualConfig
 	aErr := m.repository.AddManifest(t.Name, manifest)
@@ -198,7 +197,11 @@ func (m *manager) DeleteDeployment(name string, forget bool) (*Deployment, error
 	}
 
 	// If there's a latest manifest, delete the underlying resources.
-	latest := getLatestManifest(d.Manifests)
+	latest, err := m.repository.GetLatestManifest(name)
+	if err != nil {
+		return nil, err
+	}
+
 	if latest != nil {
 		log.Printf("Deleting resources from the latest manifest")
 		if _, err := m.deployer.DeleteConfiguration(latest.ExpandedConfig); err != nil {
@@ -283,19 +286,3 @@ func (m *manager) ListInstances(typeName string) []*TypeInstance {
 func generateManifestName() string {
 	return fmt.Sprintf("manifest-%d", time.Now().UTC().UnixNano())
 }
-
-// Given a map of manifests, finds the largest time stamp, hence probably the latest manifest.
-// This is a hack until we get a real story for storage.
-func getLatestManifest(l map[string]*Manifest) *Manifest {
-	var latest = 0
-	var ret *Manifest
-	for k, v := range l {
-		var i = 0
-		fmt.Sscanf(k, "manifest-%d", &i)
-		if i > latest {
-			latest = i
-			ret = v
-		}
-	}
-	return ret
-}

manager/manager/manager_test.go

@@ -37,8 +37,7 @@ var resourcesWithFailureState = Configuration{
 		Type: "test",
 		State: &ResourceState{
 			Status: Failed,
-			Errors:[]string{"test induced error",
-			},
+			Errors: []string{"test induced error"},
 		},
 	}},
 }
@@ -48,24 +47,15 @@ var expandedConfig = ExpandedTemplate{
 }
 
 var deploymentName = "deployment"
-var deploymentNoManifestName = "deploymentNoManifest"
 
 var manifestName = "manifest-2"
 var manifest = Manifest{Name: manifestName, ExpandedConfig: &configuration, Layout: &layout}
 var manifestMap = map[string]*Manifest{manifest.Name: &manifest}
 
-var deploymentNoManifest = Deployment{
-	Name: "test",
-}
 var deployment = Deployment{
-	Name:      "test",
-	Manifests: manifestMap,
-}
-var deploymentWithConfiguration = Deployment{
-	Name:      "test",
-	Manifests: manifestMap,
-	Current:   &configuration,
+	Name: "test",
 }
+
 var deploymentList = []Deployment{deployment, {Name: "test2"}}
 
 var typeInstMap = map[string][]string{"test": []string{"test"}}
@@ -83,10 +73,10 @@ func (expander *expanderStub) ExpandTemplate(t Template) (*ExpandedTemplate, err
 }
 
 type deployerStub struct {
-	FailCreate bool
-	Created    []*Configuration
-	FailDelete bool
-	Deleted    []*Configuration
+	FailCreate         bool
+	Created            []*Configuration
+	FailDelete         bool
+	Deleted            []*Configuration
 	FailCreateResource bool
 }
 
@@ -174,16 +164,12 @@ func (repository *repositoryStub) GetDeployment(d string) (*Deployment, error) {
 		return &deployment, nil
 	}
 
-	if d == deploymentNoManifestName {
-		return &deploymentNoManifest, nil
-	}
-
 	return nil, errTest
 }
 
 func (repository *repositoryStub) GetValidDeployment(d string) (*Deployment, error) {
 	repository.GetValid = append(repository.GetValid, d)
-	return &deploymentWithConfiguration, nil
+	return &deployment, nil
 }
 
 func (repository *repositoryStub) SetDeploymentStatus(name string, status DeploymentStatus) error {
@@ -193,12 +179,12 @@ func (repository *repositoryStub) SetDeploymentStatus(name string, status Deploy
 
 func (repository *repositoryStub) CreateDeployment(d string) (*Deployment, error) {
 	repository.Created = append(repository.Created, d)
-	return &deploymentWithConfiguration, nil
+	return &deployment, nil
 }
 
 func (repository *repositoryStub) DeleteDeployment(d string, forget bool) (*Deployment, error) {
 	repository.Deleted = append(repository.Deleted, d)
-	return &deploymentWithConfiguration, nil
+	return &deployment, nil
 }
 
 func (repository *repositoryStub) AddManifest(d string, manifest *Manifest) error {
@@ -206,6 +192,14 @@ func (repository *repositoryStub) AddManifest(d string, manifest *Manifest) erro
 	return nil
 }
 
+func (repository *repositoryStub) GetLatestManifest(d string) (*Manifest, error) {
+	if d == deploymentName {
+		return repository.ManifestAdd[d], nil
+	}
+
+	return nil, errTest
+}
+
 func (repository *repositoryStub) ListManifests(d string) (map[string]*Manifest, error) {
 	if d == deploymentName {
 		return manifestMap, nil
@@ -250,8 +244,12 @@ var testManager = NewManager(testExpander, testDeployer, testRepository)
 func TestListDeployments(t *testing.T) {
 	testRepository.reset()
 	d, err := testManager.ListDeployments()
-	if !reflect.DeepEqual(d, deploymentList) || err != nil {
-		t.FailNow()
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	if !reflect.DeepEqual(d, deploymentList) {
+		t.Fatalf("invalid deployment list")
 	}
 }
 
@@ -259,40 +257,48 @@ func TestListDeploymentsFail(t *testing.T) {
 	testRepository.reset()
 	testRepository.FailListDeployments = true
 	d, err := testManager.ListDeployments()
-	if d != nil || err != errTest {
-		t.FailNow()
+	if err != errTest {
+		t.Fatalf(err.Error())
+	}
+
+	if d != nil {
+		t.Fatalf("deployment list is not empty")
 	}
 }
 
 func TestGetDeployment(t *testing.T) {
 	testRepository.reset()
 	d, err := testManager.GetDeployment(deploymentName)
-	if !reflect.DeepEqual(d, &deploymentWithConfiguration) || err != nil {
-		t.FailNow()
+	if err != nil {
+		t.Fatalf(err.Error())
 	}
-}
 
-func TestGetDeploymentNoManifest(t *testing.T) {
-	testRepository.reset()
-	d, err := testManager.GetDeployment(deploymentNoManifestName)
-	if !reflect.DeepEqual(d, &deploymentNoManifest) || err != nil {
-		t.FailNow()
+	if !reflect.DeepEqual(d, &deployment) {
+		t.Fatalf("invalid deployment")
 	}
 }
 
 func TestListManifests(t *testing.T) {
 	testRepository.reset()
 	m, err := testManager.ListManifests(deploymentName)
-	if !reflect.DeepEqual(m, manifestMap) || err != nil {
-		t.FailNow()
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	if !reflect.DeepEqual(m, manifestMap) {
+		t.Fatalf("invalid manifest map")
 	}
 }
 
 func TestGetManifest(t *testing.T) {
 	testRepository.reset()
 	m, err := testManager.GetManifest(deploymentName, manifestName)
-	if !reflect.DeepEqual(m, &manifest) || err != nil {
-		t.FailNow()
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	if !reflect.DeepEqual(m, &manifest) {
+		t.Fatalf("invalid manifest")
 	}
 }
 
@@ -300,36 +306,36 @@ func TestCreateDeployment(t *testing.T) {
 	testRepository.reset()
 	testDeployer.reset()
 	d, err := testManager.CreateDeployment(&template)
-	if !reflect.DeepEqual(d, &deploymentWithConfiguration) || err != nil {
-		t.Errorf("Expected a different set of response values from invoking CreateDeployment."+
-			"Received: %s, %s. Expected: %s, %s.", d, err, &deploymentWithConfiguration, "nil")
+	if !reflect.DeepEqual(d, &deployment) || err != nil {
+		t.Fatalf("Expected a different set of response values from invoking CreateDeployment."+
+			"Received: %s, %s. Expected: %s, %s.", d, err, &deployment, "nil")
 	}
 
 	if testRepository.Created[0] != template.Name {
-		t.Errorf("Repository CreateDeployment was called with %s but expected %s.",
+		t.Fatalf("Repository CreateDeployment was called with %s but expected %s.",
 			testRepository.Created[0], template.Name)
 	}
 
 	if !strings.HasPrefix(testRepository.ManifestAdd[template.Name].Name, "manifest-") {
-		t.Errorf("Repository AddManifest was called with %s but expected manifest name"+
+		t.Fatalf("Repository AddManifest was called with %s but expected manifest name"+
 			"to begin with manifest-.", testRepository.ManifestAdd[template.Name].Name)
 	}
 
 	if !reflect.DeepEqual(*testDeployer.Created[0], configuration) || err != nil {
-		t.Errorf("Deployer CreateConfiguration was called with %s but expected %s.",
+		t.Fatalf("Deployer CreateConfiguration was called with %s but expected %s.",
 			testDeployer.Created[0], configuration)
 	}
 
 	if testRepository.DeploymentStatuses[0] != DeployedStatus {
-		t.Error("CreateDeployment success did not mark deployment as deployed")
+		t.Fatal("CreateDeployment success did not mark deployment as deployed")
 	}
 
 	if !testRepository.TypeInstancesCleared {
-		t.Error("Repository did not clear type instances during creation")
+		t.Fatal("Repository did not clear type instances during creation")
 	}
 
 	if !reflect.DeepEqual(testRepository.TypeInstances, typeInstMap) {
-		t.Errorf("Unexpected type instances after CreateDeployment: %s", testRepository.TypeInstances)
+		t.Fatalf("Unexpected type instances after CreateDeployment: %s", testRepository.TypeInstances)
 	}
 }
 
@@ -340,26 +346,26 @@ func TestCreateDeploymentCreationFailure(t *testing.T) {
 	d, err := testManager.CreateDeployment(&template)
 
 	if testRepository.Created[0] != template.Name {
-		t.Errorf("Repository CreateDeployment was called with %s but expected %s.",
+		t.Fatalf("Repository CreateDeployment was called with %s but expected %s.",
 			testRepository.Created[0], template.Name)
 	}
 
 	if len(testRepository.Deleted) != 0 {
-		t.Errorf("DeleteDeployment was called with %s but not expected",
+		t.Fatalf("DeleteDeployment was called with %s but not expected",
 			testRepository.Created[0])
 	}
 
 	if testRepository.DeploymentStatuses[0] != FailedStatus {
-		t.Error("CreateDeployment failure did not mark deployment as failed")
+		t.Fatal("CreateDeployment failure did not mark deployment as failed")
 	}
 
 	if err != errTest || d != nil {
-		t.Errorf("Expected a different set of response values from invoking CreateDeployment."+
+		t.Fatalf("Expected a different set of response values from invoking CreateDeployment."+
 			"Received: %s, %s. Expected: %s, %s.", d, err, "nil", errTest)
 	}
 
 	if testRepository.TypeInstancesCleared {
-		t.Error("Unexpected change to type instances during CreateDeployment failure.")
+		t.Fatal("Unexpected change to type instances during CreateDeployment failure.")
 	}
 }
 
@@ -370,32 +376,31 @@ func TestCreateDeploymentCreationResourceFailure(t *testing.T) {
 	d, err := testManager.CreateDeployment(&template)
 
 	if testRepository.Created[0] != template.Name {
-		t.Errorf("Repository CreateDeployment was called with %s but expected %s.",
+		t.Fatalf("Repository CreateDeployment was called with %s but expected %s.",
 			testRepository.Created[0], template.Name)
 	}
 
 	if len(testRepository.Deleted) != 0 {
-		t.Errorf("DeleteDeployment was called with %s but not expected",
+		t.Fatalf("DeleteDeployment was called with %s but not expected",
 			testRepository.Created[0])
 	}
 
 	if testRepository.DeploymentStatuses[0] != FailedStatus {
-		t.Error("CreateDeployment failure did not mark deployment as failed")
+		t.Fatal("CreateDeployment failure did not mark deployment as failed")
 	}
 
 	if !strings.HasPrefix(testRepository.ManifestAdd[template.Name].Name, "manifest-") {
-		t.Errorf("Repository AddManifest was called with %s but expected manifest name"+
+		t.Fatalf("Repository AddManifest was called with %s but expected manifest name"+
 			"to begin with manifest-.", testRepository.ManifestAdd[template.Name].Name)
 	}
 
-//	if err != errTest || d != nil {
-	if d == nil {
-		t.Errorf("Expected a different set of response values from invoking CreateDeployment."+
-			"Received: %s, %s. Expected: %s, %s.", d, err, "nil", errTest)
+	if err != nil || !reflect.DeepEqual(d, &deployment) {
+		t.Fatalf("Expected a different set of response values from invoking CreateDeployment.\n"+
+			"Received: %v, %v. Expected: %v, %v.", d, err, &deployment, "nil")
 	}
 
 	if !testRepository.TypeInstancesCleared {
-		t.Error("Repository did not clear type instances during creation")
+		t.Fatal("Repository did not clear type instances during creation")
 	}
 }
 
@@ -403,71 +408,69 @@ func TestDeleteDeploymentForget(t *testing.T) {
 	testRepository.reset()
 	testDeployer.reset()
 	d, err := testManager.CreateDeployment(&template)
-	if !reflect.DeepEqual(d, &deploymentWithConfiguration) || err != nil {
-		t.Errorf("Expected a different set of response values from invoking CreateDeployment."+
-			"Received: %s, %s. Expected: %s, %s.", d, err, &deploymentWithConfiguration, "nil")
+	if !reflect.DeepEqual(d, &deployment) || err != nil {
+		t.Fatalf("Expected a different set of response values from invoking CreateDeployment."+
+			"Received: %s, %s. Expected: %s, %s.", d, err, &deployment, "nil")
 	}
 
 	if testRepository.Created[0] != template.Name {
-		t.Errorf("Repository CreateDeployment was called with %s but expected %s.",
+		t.Fatalf("Repository CreateDeployment was called with %s but expected %s.",
 			testRepository.Created[0], template.Name)
 	}
 
 	if !strings.HasPrefix(testRepository.ManifestAdd[template.Name].Name, "manifest-") {
-		t.Errorf("Repository AddManifest was called with %s but expected manifest name"+
+		t.Fatalf("Repository AddManifest was called with %s but expected manifest name"+
 			"to begin with manifest-.", testRepository.ManifestAdd[template.Name].Name)
 	}
 
 	if !reflect.DeepEqual(*testDeployer.Created[0], configuration) || err != nil {
-		t.Errorf("Deployer CreateConfiguration was called with %s but expected %s.",
+		t.Fatalf("Deployer CreateConfiguration was called with %s but expected %s.",
 			testDeployer.Created[0], configuration)
 	}
-	oldManifestName := testRepository.ManifestAdd[template.Name].Name
-	d, err = testManager.DeleteDeployment("test", true)
+	d, err = testManager.DeleteDeployment(deploymentName, true)
 	if err != nil {
-		t.Errorf("DeleteDeployment failed with %v", err)
-	}
-	if testRepository.ManifestAdd[template.Name].Name == oldManifestName {
-		t.Errorf("New manifest was not created, is still: %s", oldManifestName)
-	}
-	if testRepository.ManifestAdd[template.Name].InputConfig != nil {
-		t.Errorf("New manifest has non-nil config, is still: %v", testRepository.ManifestAdd[template.Name].InputConfig)
+		t.Fatalf("DeleteDeployment failed with %v", err)
 	}
 	// Make sure the resources were deleted through deployer.
-	if !reflect.DeepEqual(*testDeployer.Deleted[0], configuration) || err != nil {
-		t.Errorf("Deployer DeleteConfiguration was called with %s but expected %s.",
-			testDeployer.Created[0], configuration)
+	if len(testDeployer.Deleted) > 0 {
+		c := testDeployer.Deleted[0]
+		if c != nil {
+			if !reflect.DeepEqual(*c, configuration) || err != nil {
+				t.Fatalf("Deployer DeleteConfiguration was called with %s but expected %s.",
+					testDeployer.Created[0], configuration)
+			}
+		}
 	}
 
 	if !testRepository.TypeInstancesCleared {
-		t.Error("Expected type instances to be cleared during DeleteDeployment.")
+		t.Fatal("Expected type instances to be cleared during DeleteDeployment.")
 	}
 }
 
 func TestExpand(t *testing.T) {
 	m, err := testManager.Expand(&template)
 	if err != nil {
-		t.Error("Failed to expand template into manifest.")
+		t.Fatal("Failed to expand template into manifest.")
 	}
 
 	if m.Name != "" {
-		t.Errorf("Name was not empty: %v", *m)
+		t.Fatalf("Name was not empty: %v", *m)
 	}
 
 	if m.Deployment != "" {
-		t.Errorf("Deployment was not empty: %v", *m)
+		t.Fatalf("Deployment was not empty: %v", *m)
 	}
 
 	if m.InputConfig != nil {
-		t.Errorf("Input config not nil: %v", *m)
+		t.Fatalf("Input config not nil: %v", *m)
 	}
 
 	if !reflect.DeepEqual(*m.ExpandedConfig, configuration) {
-		t.Errorf("Expanded config not correct in output manifest: %v", *m)
+		t.Fatalf("Expanded config not correct in output manifest: %v", *m)
 	}
 
 	if !reflect.DeepEqual(*m.Layout, layout) {
-		t.Errorf("Layout not correct in output manifest: %v", *m)
+		t.Fatalf("Layout not correct in output manifest: %v", *m)
 	}
 }
 
@@ -477,7 +480,7 @@ func TestListTypes(t *testing.T) {
 	testManager.ListTypes()
 
 	if !testRepository.ListTypesCalled {
-		t.Error("expected repository ListTypes() call.")
+		t.Fatal("expected repository ListTypes() call.")
 	}
 }
 
@@ -487,6 +490,6 @@ func TestListInstances(t *testing.T) {
 	testManager.ListInstances("all")
 
 	if !testRepository.GetTypeInstancesCalled {
-		t.Error("expected repository GetTypeInstances() call.")
+		t.Fatal("expected repository GetTypeInstances() call.")
 	}
 }

manager/manager/types.go

@@ -43,6 +43,7 @@ type Repository interface {
 	AddManifest(deploymentName string, manifest *Manifest) error
 	ListManifests(deploymentName string) (map[string]*Manifest, error)
 	GetManifest(deploymentName string, manifestName string) (*Manifest, error)
+	GetLatestManifest(deploymentName string) (*Manifest, error)
 
 	// Types.
 	ListTypes() []string
@@ -54,21 +55,19 @@ type Repository interface {
 // Deployment defines a deployment that describes
 // the creation, modification and/or deletion of a set of resources.
 type Deployment struct {
-	Name       string               `json:"name"`
-	ID         int                  `json:"id"`
-	CreatedAt  time.Time            `json:"createdAt,omitempty"`
-	DeployedAt time.Time            `json:"deployedAt,omitempty"`
-	ModifiedAt time.Time            `json:"modifiedAt,omitempty"`
-	DeletedAt  time.Time            `json:"deletedAt,omitempty"`
-	Status     DeploymentStatus     `json:"status,omitempty"`
-	Current    *Configuration       `json:"current,omitEmpty"`
-	Manifests  map[string]*Manifest `json:"manifests,omitempty"`
+	Name           string           `json:"name"`
+	ID             int              `json:"id"`
+	CreatedAt      time.Time        `json:"createdAt,omitempty"`
+	DeployedAt     time.Time        `json:"deployedAt,omitempty"`
+	ModifiedAt     time.Time        `json:"modifiedAt,omitempty"`
+	DeletedAt      time.Time        `json:"deletedAt,omitempty"`
+	Status         DeploymentStatus `json:"status,omitempty"`
+	LatestManifest string           `json:"latestManifest,omitEmpty"`
 }
 
 // NewDeployment creates a new deployment.
-func NewDeployment(name string, id int) *Deployment {
-	return &Deployment{Name: name, ID: id, CreatedAt: time.Now(), Status: CreatedStatus,
-		Manifests: make(map[string]*Manifest, 0)}
+func NewDeployment(name string) *Deployment {
+	return &Deployment{Name: name, CreatedAt: time.Now(), Status: CreatedStatus}
 }
 
 // DeploymentStatus is an enumeration type for the status of a deployment.
@@ -156,7 +155,7 @@ type Resource struct {
 	Name       string                 `json:"name"`
 	Type       string                 `json:"type"`
 	Properties map[string]interface{} `json:"properties,omitempty"`
-	State      *ResourceState          `json:"state,omitempty"`
+	State      *ResourceState         `json:"state,omitempty"`
 }
 
 // TypeInstance defines the metadata for an instantiation of a template type

manager/repository/repository.go

@@ -34,14 +34,15 @@ type typeInstanceMap map[string]deploymentTypeInstanceMap
 type mapBasedRepository struct {
 	sync.RWMutex
 	deployments map[string]manager.Deployment
+	manifests   map[string]map[string]*manager.Manifest
 	instances   typeInstanceMap
-	lastID      int
 }
 
 // NewMapBasedRepository returns a new map based repository.
 func NewMapBasedRepository() manager.Repository {
 	return &mapBasedRepository{
 		deployments: make(map[string]manager.Deployment, 0),
+		manifests:   make(map[string]map[string]*manager.Manifest, 0),
 		instances:   typeInstanceMap{},
 	}
 }
@@ -113,9 +114,7 @@ func (r *mapBasedRepository) CreateDeployment(name string) (*manager.Deployment,
 			return nil, fmt.Errorf("Deployment %s already exists", name)
 		}
 
-		r.lastID++
-
-		d := manager.NewDeployment(name, r.lastID)
+		d := manager.NewDeployment(name)
 		d.Status = manager.CreatedStatus
 		d.DeployedAt = time.Now()
 		r.deployments[name] = *d
@@ -134,23 +133,32 @@ func (r *mapBasedRepository) AddManifest(deploymentName string, manifest *manage
 	err := func() error {
 		r.Lock()
 		defer r.Unlock()
+
 		d, err := r.GetValidDeployment(deploymentName)
 		if err != nil {
 			return err
 		}
 
+		l, err := r.listManifestsForDeployment(deploymentName)
+		if err != nil {
+			return err
+		}
+
 		// Make sure the manifest doesn't already exist, and if not, add the manifest to
 		// map of manifests this deployment has
-		if _, ok := d.Manifests[manifest.Name]; ok {
+		if _, ok := l[manifest.Name]; ok {
 			return fmt.Errorf("Manifest %s already exists in deployment %s", manifest.Name, deploymentName)
 		}
-		d.Manifests[manifest.Name] = manifest
-		r.deployments[deploymentName] = *d
+
+		l[manifest.Name] = manifest
+		d.LatestManifest = manifest.Name
 		return nil
 	}()
+
 	if err != nil {
 		return err
 	}
+
 	log.Printf("Added manifest %s to deployment: %s", manifest.Name, deploymentName)
 	return nil
 }
@@ -174,6 +182,8 @@ func (r *mapBasedRepository) DeleteDeployment(name string, forget bool) (*manage
 			r.deployments[name] = *d
 		} else {
 			delete(r.deployments, name)
+			delete(r.manifests, name)
+			d.LatestManifest = ""
 		}
 
 		return d, nil
@@ -188,22 +198,65 @@ func (r *mapBasedRepository) DeleteDeployment(name string, forget bool) (*manage
 }
 
 func (r *mapBasedRepository) ListManifests(deploymentName string) (map[string]*manager.Manifest, error) {
-	d, err := r.GetValidDeployment(deploymentName)
+	r.Lock()
+	defer r.Unlock()
+
+	_, err := r.GetValidDeployment(deploymentName)
 	if err != nil {
 		return nil, err
 	}
-	return d.Manifests, nil
+
+	return r.listManifestsForDeployment(deploymentName)
+}
+
+func (r *mapBasedRepository) listManifestsForDeployment(deploymentName string) (map[string]*manager.Manifest, error) {
+	l, ok := r.manifests[deploymentName]
+	if !ok {
+		l = make(map[string]*manager.Manifest, 0)
+		r.manifests[deploymentName] = l
+	}
+
+	return l, nil
 }
 
 func (r *mapBasedRepository) GetManifest(deploymentName string, manifestName string) (*manager.Manifest, error) {
-	d, err := r.GetValidDeployment(deploymentName)
+	r.Lock()
+	defer r.Unlock()
+
+	_, err := r.GetValidDeployment(deploymentName)
 	if err != nil {
 		return nil, err
 	}
-	if m, ok := d.Manifests[manifestName]; ok {
-		return m, nil
+
+	return r.getManifestForDeployment(deploymentName, manifestName)
+}
+
+func (r *mapBasedRepository) getManifestForDeployment(deploymentName string, manifestName string) (*manager.Manifest, error) {
+	l, err := r.listManifestsForDeployment(deploymentName)
+	if err != nil {
+		return nil, err
 	}
-	return nil, fmt.Errorf("manifest %s not found in deployment %s", manifestName, deploymentName)
+
+	m, ok := l[manifestName]
+	if !ok {
+		return nil, fmt.Errorf("manifest %s not found in deployment %s", manifestName, deploymentName)
+	}
+
+	return m, nil
+}
+
+// GetLatestManifest returns the latest manifest for a given deployment,
+// which by definition is the manifest with the largest time stamp.
+func (r *mapBasedRepository) GetLatestManifest(deploymentName string) (*manager.Manifest, error) {
+	r.Lock()
+	defer r.Unlock()
+
+	d, err := r.GetValidDeployment(deploymentName)
+	if err != nil {
+		return nil, err
+	}
+
+	return r.getManifestForDeployment(deploymentName, d.LatestManifest)
 }
 
 // ListTypes returns all types known from existing instances.

manager/repository/repository_test.go

@@ -16,6 +16,7 @@ package repository
 import (
 	"github.com/kubernetes/deployment-manager/manager/manager"
 
+	"fmt"
 	"testing"
 )
 
@@ -41,15 +42,15 @@ func TestRepositoryGetFailsWithNonExistentDeployment(t *testing.T) {
 	}
 }
 
-func TestRepositoryCreateDeploymentWorks(t *testing.T) {
+func testCreateDeploymentWithManifests(t *testing.T, count int) {
 	var deploymentName = "mydeployment"
-	var manifestName = "manifest-0"
 	r := NewMapBasedRepository()
-	manifest := manager.Manifest{Deployment: deploymentName, Name: manifestName}
+
 	d, err := r.CreateDeployment(deploymentName)
 	if err != nil {
 		t.Fatalf("CreateDeployment failed: %v", err)
 	}
+
 	l, err := r.ListDeployments()
 	if err != nil {
 		t.Fatalf("ListDeployments failed: %v", err)
@@ -57,6 +58,7 @@ func TestRepositoryCreateDeploymentWorks(t *testing.T) {
 	if len(l) != 1 {
 		t.Fatalf("List of deployments is not 1: %d", len(l))
 	}
+
 	dNew, err := r.GetDeployment(deploymentName)
 	if err != nil {
 		t.Fatalf("GetDeployment failed: %v", err)
@@ -64,114 +66,51 @@ func TestRepositoryCreateDeploymentWorks(t *testing.T) {
 	if dNew.Name != d.Name {
 		t.Fatalf("Deployment Names don't match, got: %v, expected %v", dNew, d)
 	}
-	if len(dNew.Manifests) != 0 {
-		t.Fatalf("Deployment has non-zero manifest count: %d", len(dNew.Manifests))
-	}
-	err = r.AddManifest(deploymentName, &manifest)
-	if err != nil {
-		t.Fatalf("AddManifest failed: %v", err)
-	}
-	dNew, err = r.GetDeployment(deploymentName)
-	if err != nil {
-		t.Fatalf("GetDeployment failed: %v", err)
-	}
-	if len(dNew.Manifests) != 1 {
-		t.Fatalf("Fetched deployment does not have manifest count of 1: %d", len(dNew.Manifests))
-	}
-	manifestList, err := r.ListManifests(deploymentName)
+
+	mList, err := r.ListManifests(deploymentName)
 	if err != nil {
 		t.Fatalf("ListManifests failed: %v", err)
 	}
-	if len(manifestList) != 1 {
-		t.Fatalf("ManifestList does not have manifest count of 1: %d", len(manifestList))
-	}
-	m, err := r.GetManifest(deploymentName, manifestName)
-	if err != nil {
-		t.Fatalf("GetManifest failed: %v", err)
+	if len(mList) != 0 {
+		t.Fatalf("Deployment has non-zero manifest count: %d", len(mList))
 	}
-	if m.Name != manifestName {
-		t.Fatalf("manifest name doesn't match: %v", m)
+
+	for i := 0; i < count; i++ {
+		var manifestName = fmt.Sprintf("manifest-%d", i)
+		manifest := manager.Manifest{Deployment: deploymentName, Name: manifestName}
+		err := r.AddManifest(deploymentName, &manifest)
+		if err != nil {
+			t.Fatalf("AddManifest failed: %v", err)
+		}
+		_, err = r.GetDeployment(deploymentName)
+		if err != nil {
+			t.Fatalf("GetDeployment failed: %v", err)
+		}
+
+		mListNew, err := r.ListManifests(deploymentName)
+		if err != nil {
+			t.Fatalf("ListManifests failed: %v", err)
+		}
+		if len(mListNew) != i+1 {
+			t.Fatalf("Deployment has unexpected manifest count: want %d, have %d", i+1, len(mListNew))
+		}
+
+		m, err := r.GetManifest(deploymentName, manifestName)
+		if err != nil {
+			t.Fatalf("GetManifest failed: %v", err)
+		}
+		if m.Name != manifestName {
+			t.Fatalf("Unexpected manifest name: want %s, have %s", manifestName, m.Name)
+		}
 	}
 }
 
+func TestRepositoryCreateDeploymentWorks(t *testing.T) {
+	testCreateDeploymentWithManifests(t, 1)
+}
+
 func TestRepositoryMultipleManifestsWorks(t *testing.T) {
-	var deploymentName = "mydeployment"
-	var manifestName = "manifest-0"
-	var manifestName2 = "manifest-1"
-	r := NewMapBasedRepository()
-	manifest := manager.Manifest{Deployment: deploymentName, Name: manifestName}
-	manifest2 := manager.Manifest{Deployment: deploymentName, Name: manifestName2}
-	d, err := r.CreateDeployment(deploymentName)
-	if err != nil {
-		t.Fatalf("CreateDeployment failed: %v", err)
-	}
-	dNew, err := r.GetDeployment(deploymentName)
-	if err != nil {
-		t.Fatalf("GetDeployment failed: %v", err)
-	}
-	if dNew.Name != d.Name {
-		t.Fatalf("Deployment Names don't match, got: %v, expected %v", dNew, d)
-	}
-	if len(dNew.Manifests) != 0 {
-		t.Fatalf("Deployment has non-zero manifest count: %d", len(dNew.Manifests))
-	}
-	err = r.AddManifest(deploymentName, &manifest)
-	if err != nil {
-		t.Fatalf("AddManifest failed: %v", err)
-	}
-	dNew, err = r.GetDeployment(deploymentName)
-	if err != nil {
-		t.Fatalf("GetDeployment failed: %v", err)
-	}
-	if len(dNew.Manifests) != 1 {
-		t.Fatalf("Fetched deployment does not have manifest count of 1: %d", len(dNew.Manifests))
-	}
-	manifestList, err := r.ListManifests(deploymentName)
-	if err != nil {
-		t.Fatalf("ListManifests failed: %v", err)
-	}
-	if len(manifestList) != 1 {
-		t.Fatalf("ManifestList does not have manifest count of 1: %d", len(manifestList))
-	}
-	m, err := r.GetManifest(deploymentName, manifestName)
-	if err != nil {
-		t.Fatalf("GetManifest failed: %v", err)
-	}
-	if m.Name != manifestName {
-		t.Fatalf("manifest name doesn't match: %v", m)
-	}
-	err = r.AddManifest(deploymentName, &manifest2)
-	if err != nil {
-		t.Fatalf("AddManifest failed: %v", err)
-	}
-	dNew, err = r.GetDeployment(deploymentName)
-	if err != nil {
-		t.Fatalf("GetDeployment failed: %v", err)
-	}
-	if len(dNew.Manifests) != 2 {
-		t.Fatalf("Fetched deployment does not have manifest count of 2: %d", len(dNew.Manifests))
-	}
-	manifestList, err = r.ListManifests(deploymentName)
-	if err != nil {
-		t.Fatalf("ListManifests failed: %v", err)
-	}
-	if len(manifestList) != 2 {
-		t.Fatalf("ManifestList does not have manifest count of 1: %d", len(manifestList))
-	}
-	m, err = r.GetManifest(deploymentName, manifestName)
-	if err != nil {
-		t.Fatalf("GetManifest failed: %v", err)
-	}
-	if m.Name != manifestName {
-		t.Fatalf("manifest name doesn't match: %v", m)
-	}
-	m, err = r.GetManifest(deploymentName, manifestName2)
-	if err != nil {
-		t.Fatalf("GetManifest failed: %v", err)
-	}
-	if m.Name != manifestName2 {
-		t.Fatalf("manifest name doesn't match: %v", m)
-	}
+	testCreateDeploymentWithManifests(t, 7)
 }
 
 func TestRepositoryDeleteFailsWithNonExistentDeployment(t *testing.T) {
@@ -200,8 +139,8 @@ func TestRepositoryDeleteWorksWithNoLatestManifest(t *testing.T) {
 	if dDeleted.Status != manager.DeletedStatus {
 		t.Fatalf("Deployment Status is not deleted")
 	}
-	if len(dDeleted.Manifests) != 0 {
-		t.Fatalf("manifests count is not 0, is: %d", len(dDeleted.Manifests))
+	if _, err := r.ListManifests(deploymentName); err == nil {
+		t.Fatalf("Manifests are not deleted")
 	}
 }