From 7b4991fe276617f9a8c988d992fb3d56deeb6f92 Mon Sep 17 00:00:00 2001 From: Ville Aikas Date: Mon, 16 Nov 2015 08:50:02 -0800 Subject: [PATCH] Do not delete the deployment nor resources when a deployment fails --- manager/manager/manager.go | 7 ++++--- manager/manager/manager_test.go | 21 ++++++++++++++++++--- manager/manager/types.go | 19 ++++++++++--------- manager/repository/repository.go | 18 ++++++++++++++++++ 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/manager/manager/manager.go b/manager/manager/manager.go index f23026aab..2cb4858a3 100644 --- a/manager/manager/manager.go +++ b/manager/manager/manager.go @@ -113,15 +113,16 @@ func (m *manager) CreateDeployment(t *Template) (*Deployment, error) { return nil, err } - // TODO: Mark this as failed instead of deleting. if err := m.deployer.CreateConfiguration(et.Config); err != nil { - m.repository.DeleteDeployment(t.Name, true) + // Deployment failed, mark as deleted + log.Printf("CreateConfiguration failed: %v", err) + m.repository.SetDeploymentStatus(t.Name, FailedStatus) return nil, err } + m.repository.SetDeploymentStatus(t.Name, DeployedStatus) // Finally update the type instances for this deployment. m.addTypeInstances(t.Name, manifest.Name, manifest.Layout) - return m.repository.GetValidDeployment(t.Name) } diff --git a/manager/manager/manager_test.go b/manager/manager/manager_test.go index 84a9340ff..e3bc55155 100644 --- a/manager/manager/manager_test.go +++ b/manager/manager/manager_test.go @@ -122,6 +122,7 @@ type repositoryStub struct { TypeInstancesCleared bool GetTypeInstancesCalled bool ListTypesCalled bool + DeploymentStatuses []DeploymentStatus } func (repository *repositoryStub) reset() { @@ -134,6 +135,7 @@ func (repository *repositoryStub) reset() { repository.TypeInstancesCleared = false repository.GetTypeInstancesCalled = false repository.ListTypesCalled = false + repository.DeploymentStatuses = make([]DeploymentStatus, 0) } func newRepositoryStub() *repositoryStub { @@ -165,6 +167,11 @@ func (repository *repositoryStub) GetValidDeployment(d string) (*Deployment, err return &deploymentWithConfiguration, nil } +func (repository *repositoryStub) SetDeploymentStatus(name string, status DeploymentStatus) error { + repository.DeploymentStatuses = append(repository.DeploymentStatuses, status) + return nil +} + func (repository *repositoryStub) CreateDeployment(d string) (*Deployment, error) { repository.Created = append(repository.Created, d) return &deploymentWithConfiguration, nil @@ -294,6 +301,10 @@ func TestCreateDeployment(t *testing.T) { testDeployer.Created[0], configuration) } + if testRepository.DeploymentStatuses[0] != DeployedStatus { + t.Error("CreateDeployment success did not mark deployment as deployed") + } + if !testRepository.TypeInstancesCleared { t.Error("Repository did not clear type instances during creation") } @@ -314,9 +325,13 @@ func TestCreateDeploymentCreationFailure(t *testing.T) { testRepository.Created[0], template.Name) } - if testRepository.Deleted[0] != template.Name { - t.Errorf("Repository DeleteDeployment 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", + testRepository.Created[0]) + } + + if testRepository.DeploymentStatuses[0] != FailedStatus { + t.Error("CreateDeployment failure did not mark deployment as failed") } if !strings.HasPrefix(testRepository.ManifestAdd[template.Name].Name, "manifest-") { diff --git a/manager/manager/types.go b/manager/manager/types.go index 97af0697b..e9c95e728 100644 --- a/manager/manager/types.go +++ b/manager/manager/types.go @@ -47,6 +47,7 @@ type Repository interface { GetValidDeployment(name string) (*Deployment, error) CreateDeployment(name string) (*Deployment, error) DeleteDeployment(name string, forget bool) (*Deployment, error) + SetDeploymentStatus(name string, status DeploymentStatus) error // Manifests. AddManifest(deploymentName string, manifest *Manifest) error @@ -69,7 +70,7 @@ type Deployment struct { DeployedAt time.Time `json:"deployedAt,omitempty"` ModifiedAt time.Time `json:"modifiedAt,omitempty"` DeletedAt time.Time `json:"deletedAt,omitempty"` - Status deploymentStatus `json:"status,omitempty"` + Status DeploymentStatus `json:"status,omitempty"` Current *Configuration `json:"current,omitEmpty"` Manifests map[string]*Manifest `json:"manifests,omitempty"` } @@ -85,19 +86,19 @@ func NewManifest(deploymentName string, manifestName string) *Manifest { return &Manifest{Deployment: deploymentName, Name: manifestName} } -// deploymentStatus is an enumeration type for the status of a deployment. -type deploymentStatus string +// DeploymentStatus is an enumeration type for the status of a deployment. +type DeploymentStatus string // These constants implement the deploymentStatus enumeration type. const ( - CreatedStatus deploymentStatus = "Created" - DeletedStatus deploymentStatus = "Deleted" - DeployedStatus deploymentStatus = "Deployed" - FailedStatus deploymentStatus = "Failed" - ModifiedStatus deploymentStatus = "Modified" + CreatedStatus DeploymentStatus = "Created" + DeletedStatus DeploymentStatus = "Deleted" + DeployedStatus DeploymentStatus = "Deployed" + FailedStatus DeploymentStatus = "Failed" + ModifiedStatus DeploymentStatus = "Modified" ) -func (s deploymentStatus) String() string { +func (s DeploymentStatus) String() string { return string(s) } diff --git a/manager/repository/repository.go b/manager/repository/repository.go index 1c499fe03..665b48d98 100644 --- a/manager/repository/repository.go +++ b/manager/repository/repository.go @@ -84,6 +84,24 @@ func (r *mapBasedRepository) GetValidDeployment(name string) (*manager.Deploymen return d, nil } +// SetDeploymentStatus sets the DeploymentStatus of the deployment and updates ModifiedAt +func (r *mapBasedRepository) SetDeploymentStatus(name string, status manager.DeploymentStatus) error { + return func() error { + r.Lock() + defer r.Unlock() + + d, err := r.GetValidDeployment(name) + if err != nil { + return err + } + + d.Status = status + d.ModifiedAt = time.Now() + r.deployments[name] = *d + return nil + }() +} + // CreateDeployment creates a new deployment and stores it in the repository. func (r *mapBasedRepository) CreateDeployment(name string) (*manager.Deployment, error) { d, err := func() (*manager.Deployment, error) {