From 8dad3365b5357e828f2e916b5d646e8e71b6c588 Mon Sep 17 00:00:00 2001 From: jackgr Date: Sun, 22 Nov 2015 21:02:13 -0800 Subject: [PATCH] Remove direct access to manifests to prepare for persistence.. --- manager/manager/manager.go | 31 ++--- manager/manager/manager_test.go | 183 +++++++++++++------------- manager/manager/types.go | 25 ++-- manager/repository/repository.go | 79 +++++++++-- manager/repository/repository_test.go | 149 +++++++-------------- 5 files changed, 224 insertions(+), 243 deletions(-) diff --git a/manager/manager/manager.go b/manager/manager/manager.go index c115d182d..6c218f0c8 100644 --- a/manager/manager/manager.go +++ b/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 -} diff --git a/manager/manager/manager_test.go b/manager/manager/manager_test.go index 1a9e47b44..2b6eb7aa0 100644 --- a/manager/manager/manager_test.go +++ b/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.") } } diff --git a/manager/manager/types.go b/manager/manager/types.go index 1fde509ae..031892711 100644 --- a/manager/manager/types.go +++ b/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 diff --git a/manager/repository/repository.go b/manager/repository/repository.go index 665b48d98..1812c0ecc 100644 --- a/manager/repository/repository.go +++ b/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. diff --git a/manager/repository/repository_test.go b/manager/repository/repository_test.go index 2d6f828d5..47f9af313 100644 --- a/manager/repository/repository_test.go +++ b/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") } }