From a6c48fb2490deb98336f02f3ca7d0d430fa1f261 Mon Sep 17 00:00:00 2001 From: Brendan Melville Date: Thu, 3 Dec 2015 15:05:57 -0500 Subject: [PATCH 1/2] Deployment now has top-level error for deployment-level failures Deployment status is now deployment state, which contains both status and errors, which are set for deployment-level failure. Some common failures that will show up here are expansion, type resolution, and reference failures. --- common/types.go | 15 +++++++++++++-- manager/manager/manager.go | 27 +++++++++++++++++++-------- manager/manager/manager_test.go | 18 +++++++++++------- manager/repository/repository.go | 13 ++++++------- manager/repository/repository_test.go | 6 +++--- 5 files changed, 52 insertions(+), 27 deletions(-) diff --git a/common/types.go b/common/types.go index 13218dd31..63b1340f6 100644 --- a/common/types.go +++ b/common/types.go @@ -37,13 +37,24 @@ 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"` + State *DeploymentState `json:"state,omitempty"` LatestManifest string `json:"latestManifest,omitEmpty"` } // NewDeployment creates a new deployment. func NewDeployment(name string) *Deployment { - return &Deployment{Name: name, CreatedAt: time.Now(), Status: CreatedStatus} + return &Deployment{ + Name: name, + CreatedAt: time.Now(), + State: &DeploymentState{Status: CreatedStatus}, + } +} + +// DeploymentState describes the state of a resource. It is set to the terminal +// state depending on the outcome of an operation on the deployment. +type DeploymentState struct { + Status DeploymentStatus `json:"status,omitempty"` + Errors []string `json:"errors,omitempty"` } // DeploymentStatus is an enumeration type for the status of a deployment. diff --git a/manager/manager/manager.go b/manager/manager/manager.go index 553c6736a..53e14a089 100644 --- a/manager/manager/manager.go +++ b/manager/manager/manager.go @@ -102,13 +102,13 @@ func (m *manager) CreateDeployment(t *common.Template) (*common.Deployment, erro manifest, err := m.createManifest(t) if err != nil { log.Printf("Manifest creation failed: %v", err) - m.repository.SetDeploymentStatus(t.Name, common.FailedStatus) + m.repository.SetDeploymentState(t.Name, failState(err)) return nil, err } if err := m.repository.AddManifest(t.Name, manifest); err != nil { log.Printf("AddManifest failed %v", err) - m.repository.SetDeploymentStatus(t.Name, common.FailedStatus) + m.repository.SetDeploymentState(t.Name, failState(err)) return nil, err } @@ -116,7 +116,8 @@ func (m *manager) CreateDeployment(t *common.Template) (*common.Deployment, erro if err != nil { // Deployment failed, mark as failed log.Printf("CreateConfiguration failed: %v", err) - m.repository.SetDeploymentStatus(t.Name, common.FailedStatus) + m.repository.SetDeploymentState(t.Name, failState(err)) + // If we failed before being able to create some of the resources, then // return the failure as such. Otherwise, we're going to add the manifest // and hence resource specific errors down below. @@ -124,14 +125,14 @@ func (m *manager) CreateDeployment(t *common.Template) (*common.Deployment, erro return nil, err } } else { - m.repository.SetDeploymentStatus(t.Name, common.DeployedStatus) + m.repository.SetDeploymentState(t.Name, &common.DeploymentState{Status: common.DeployedStatus}) } // Update the manifest with the actual state of the reified resources manifest.ExpandedConfig = actualConfig if err := m.repository.SetManifest(t.Name, manifest); err != nil { log.Printf("SetManifest failed %v", err) - m.repository.SetDeploymentStatus(t.Name, common.FailedStatus) + m.repository.SetDeploymentState(t.Name, failState(err)) return nil, err } @@ -198,6 +199,7 @@ func (m *manager) DeleteDeployment(name string, forget bool) (*common.Deployment // If there's a latest manifest, delete the underlying resources. latest, err := m.repository.GetLatestManifest(name) if err != nil { + m.repository.SetDeploymentState(name, failState(err)) return nil, err } @@ -210,6 +212,7 @@ func (m *manager) DeleteDeployment(name string, forget bool) (*common.Deployment if _, err := m.deployer.DeleteConfiguration(latest.ExpandedConfig); err != nil { log.Printf("Failed to delete resources from the latest manifest: %v", err) + m.repository.SetDeploymentState(name, failState(err)) return nil, err } @@ -217,6 +220,7 @@ func (m *manager) DeleteDeployment(name string, forget bool) (*common.Deployment err = m.repository.AddManifest(name, &common.Manifest{Deployment: name, Name: generateManifestName()}) if err != nil { log.Printf("Failed to add empty manifest") + m.repository.SetDeploymentState(name, failState(err)) return nil, err } } @@ -243,20 +247,20 @@ func (m *manager) PutDeployment(name string, t *common.Template) (*common.Deploy manifest, err := m.createManifest(t) if err != nil { log.Printf("Manifest creation failed: %v", err) - m.repository.SetDeploymentStatus(name, common.FailedStatus) + m.repository.SetDeploymentState(name, failState(err)) return nil, err } actualConfig, err := m.deployer.PutConfiguration(manifest.ExpandedConfig) if err != nil { - m.repository.SetDeploymentStatus(name, common.FailedStatus) + m.repository.SetDeploymentState(name, failState(err)) return nil, err } manifest.ExpandedConfig = actualConfig err = m.repository.AddManifest(t.Name, manifest) if err != nil { - m.repository.SetDeploymentStatus(name, common.FailedStatus) + m.repository.SetDeploymentState(name, failState(err)) return nil, err } @@ -290,3 +294,10 @@ func (m *manager) ListInstances(typeName string) []*common.TypeInstance { func generateManifestName() string { return fmt.Sprintf("manifest-%d", time.Now().UTC().UnixNano()) } + +func failState(e error) *common.DeploymentState { + return &common.DeploymentState{ + Status: common.FailedStatus, + Errors: []string{e.Error()}, + } +} diff --git a/manager/manager/manager_test.go b/manager/manager/manager_test.go index acfee73f9..eeed183a7 100644 --- a/manager/manager/manager_test.go +++ b/manager/manager/manager_test.go @@ -134,7 +134,7 @@ type repositoryStub struct { TypeInstancesCleared bool GetTypeInstancesCalled bool ListTypesCalled bool - DeploymentStatuses []common.DeploymentStatus + DeploymentStates []*common.DeploymentState } func (repository *repositoryStub) reset() { @@ -148,7 +148,7 @@ func (repository *repositoryStub) reset() { repository.TypeInstancesCleared = false repository.GetTypeInstancesCalled = false repository.ListTypesCalled = false - repository.DeploymentStatuses = make([]common.DeploymentStatus, 0) + repository.DeploymentStates = []*common.DeploymentState{} } func newRepositoryStub() *repositoryStub { @@ -176,8 +176,8 @@ func (repository *repositoryStub) GetValidDeployment(d string) (*common.Deployme return &deployment, nil } -func (repository *repositoryStub) SetDeploymentStatus(name string, status common.DeploymentStatus) error { - repository.DeploymentStatuses = append(repository.DeploymentStatuses, status) +func (repository *repositoryStub) SetDeploymentState(name string, state *common.DeploymentState) error { + repository.DeploymentStates = append(repository.DeploymentStates, state) return nil } @@ -340,7 +340,7 @@ func TestCreateDeployment(t *testing.T) { testDeployer.Created[0], configuration) } - if testRepository.DeploymentStatuses[0] != common.DeployedStatus { + if testRepository.DeploymentStates[0].Status != common.DeployedStatus { t.Fatal("CreateDeployment success did not mark deployment as deployed") } @@ -369,7 +369,7 @@ func TestCreateDeploymentCreationFailure(t *testing.T) { testRepository.Created[0]) } - if testRepository.DeploymentStatuses[0] != common.FailedStatus { + if testRepository.DeploymentStates[0].Status != common.FailedStatus { t.Fatal("CreateDeployment failure did not mark deployment as failed") } @@ -399,7 +399,7 @@ func TestCreateDeploymentCreationResourceFailure(t *testing.T) { testRepository.Created[0]) } - if testRepository.DeploymentStatuses[0] != common.FailedStatus { + if testRepository.DeploymentStates[0].Status != common.FailedStatus { t.Fatal("CreateDeployment failure did not mark deployment as failed") } @@ -455,6 +455,10 @@ func TestDeleteDeploymentForget(t *testing.T) { if err != nil { t.Fatalf("DeleteDeployment failed with %v", err) } + if d.State.Status != common.DeletedStatus { + t.Fatalf("Expected DeletedStatus on deleted deployment") + } + // Make sure the resources were deleted through deployer. if len(testDeployer.Deleted) > 0 { c := testDeployer.Deleted[0] diff --git a/manager/repository/repository.go b/manager/repository/repository.go index 8abf6d84f..97cb864b2 100644 --- a/manager/repository/repository.go +++ b/manager/repository/repository.go @@ -34,7 +34,7 @@ type Repository interface { GetValidDeployment(name string) (*common.Deployment, error) CreateDeployment(name string) (*common.Deployment, error) DeleteDeployment(name string, forget bool) (*common.Deployment, error) - SetDeploymentStatus(name string, status common.DeploymentStatus) error + SetDeploymentState(name string, state *common.DeploymentState) error // Manifests. AddManifest(deploymentName string, manifest *common.Manifest) error @@ -103,15 +103,15 @@ func (r *mapBasedRepository) GetValidDeployment(name string) (*common.Deployment return nil, err } - if d.Status == common.DeletedStatus { + if d.State.Status == common.DeletedStatus { return nil, fmt.Errorf("deployment %s is deleted", name) } return d, nil } -// SetDeploymentStatus sets the DeploymentStatus of the deployment and updates ModifiedAt -func (r *mapBasedRepository) SetDeploymentStatus(name string, status common.DeploymentStatus) error { +// SetDeploymentState sets the DeploymentState of the deployment and updates ModifiedAt +func (r *mapBasedRepository) SetDeploymentState(name string, state *common.DeploymentState) error { return func() error { r.Lock() defer r.Unlock() @@ -121,7 +121,7 @@ func (r *mapBasedRepository) SetDeploymentStatus(name string, status common.Depl return err } - d.Status = status + d.State = state d.ModifiedAt = time.Now() r.deployments[name] = *d return nil @@ -140,7 +140,6 @@ func (r *mapBasedRepository) CreateDeployment(name string) (*common.Deployment, } d := common.NewDeployment(name) - d.Status = common.CreatedStatus d.DeployedAt = time.Now() r.deployments[name] = *d return d, nil @@ -214,7 +213,7 @@ func (r *mapBasedRepository) DeleteDeployment(name string, forget bool) (*common if !forget { d.DeletedAt = time.Now() - d.Status = common.DeletedStatus + d.State = &common.DeploymentState{Status: common.DeletedStatus} r.deployments[name] = *d } else { delete(r.deployments, name) diff --git a/manager/repository/repository_test.go b/manager/repository/repository_test.go index 4bb7067b6..f660e1bba 100644 --- a/manager/repository/repository_test.go +++ b/manager/repository/repository_test.go @@ -140,7 +140,7 @@ func TestRepositoryDeleteWorksWithNoLatestManifest(t *testing.T) { if err != nil { t.Fatalf("DeleteDeployment failed: %v", err) } - if dDeleted.Status != common.DeletedStatus { + if dDeleted.State.Status != common.DeletedStatus { t.Fatalf("Deployment Status is not deleted") } if _, err := r.ListManifests(deploymentName); err == nil { @@ -165,7 +165,7 @@ func TestRepositoryDeleteDeploymentWorksNoForget(t *testing.T) { if err != nil { t.Fatalf("DeleteDeployment failed: %v", err) } - if dDeleted.Status != common.DeletedStatus { + if dDeleted.State.Status != common.DeletedStatus { t.Fatalf("Deployment Status is not deleted") } } @@ -187,7 +187,7 @@ func TestRepositoryDeleteDeploymentWorksForget(t *testing.T) { if err != nil { t.Fatalf("DeleteDeployment failed: %v", err) } - if dDeleted.Status != common.CreatedStatus { + if dDeleted.State.Status != common.CreatedStatus { t.Fatalf("Deployment Status is not created") } } From 55ec074701d2f1de1fbeff2ef722e76d2a0c33e7 Mon Sep 17 00:00:00 2001 From: Brendan Melville Date: Thu, 3 Dec 2015 16:30:41 -0500 Subject: [PATCH 2/2] Broke guestbook. --- examples/guestbook/guestbook.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/examples/guestbook/guestbook.yaml b/examples/guestbook/guestbook.yaml index 073f01994..9a31d2489 100644 --- a/examples/guestbook/guestbook.yaml +++ b/examples/guestbook/guestbook.yaml @@ -7,9 +7,6 @@ resources: external_service: true replicas: 3 image: gcr.io/google_containers/example-guestbook-php-redis:v3 - env: - - name: redis-master - value: $(ref.redis-master.name) - name: redis type: github.com/kubernetes/application-dm-templates/storage/redis:v1 properties: null