Merge pull request #131 from crunch-accounting/bugfix/forceupdate

Force update fails to delete release and tries to create a new one

cmd/apply.go

@@ -66,8 +66,9 @@ var addCmd = &cobra.Command{
  return err
  }
 
- if err = executor.Apply(desired, current); err != nil {
- logrus.WithFields(logrus.Fields{"error": err}).Error("Applying desired state failed")
+ result, err := executor.Apply(desired, current)
+ if err != nil {
+ logrus.WithFields(logrus.Fields{"error": err, "result": result}).Error("Applying desired state failed")
  return err
  }
 

pkg/landscaper/executor.go

@@ -13,7 +13,7 @@ import (
 
 // Executor is responsible for applying a desired landscape to the actual landscape
 type Executor interface {
- Apply(Components, Components) error
+ Apply(Components, Components) (map[string][]string, error)
 
  CreateComponent(*Component) error
  UpdateComponent(*Component) error
@@ -68,61 +68,67 @@ func (e *executor) gatherForcedUpdates(current, update Components) (map[string]b
 }
 
 // Apply transforms the current state into the desired state
-func (e *executor) Apply(desired, current Components) error {
+func (e *executor) Apply(desired, current Components) (map[string][]string, error) {
+ result := make(map[string][]string)
+ result["create"] = []string{}
+ result["update"] = []string{}
+ result["delete"] = []string{}
+
  create, update, delete := diff(desired, current)
 
  // some to-be-updated components need a delete + create instead
  needForcedUpdate, err := e.gatherForcedUpdates(current, update)
  if err != nil {
- return err
+ return result, err
  }
 
  // delete+create pairs will never work in dry run since the dry-run "deleted" component will exist in create
  if !e.dryRun {
  create, update, delete = integrateForcedUpdates(current, create, update, delete, needForcedUpdate)
  }
 
- logrus.WithFields(logrus.Fields{"create": len(create), "update": len(update), "delete": len(delete)}).Info("Apply desired state")
-
  for _, cmp := range delete {
- _, cmpForcedUpdate := needForcedUpdate[cmp.Name]
+ cmpForcedUpdate := needForcedUpdate[cmp.Name]
  if e.stageEnabled("delete") || (e.stageEnabled("update") && cmpForcedUpdate) {
  logrus.Infof("Delete: %s", cmp.Name)
  if err := e.DeleteComponent(cmp); err != nil {
  logrus.WithFields(logrus.Fields{"error": err, "component": cmp}).Error("DeleteComponent failed")
- return err
+ return result, err
  }
+ result["delete"] = append(result["delete"], cmp.Name)
  }
  }
 
  if e.stageEnabled("update") {
  for _, cmp := range update {
  if err := logDifferences(logrus.Infof, "Update: "+cmp.Name, current[cmp.Name], cmp); err != nil {
- return err
+ return result, err
  }
  if err := e.UpdateComponent(cmp); err != nil {
  logrus.WithFields(logrus.Fields{"error": err, "component": cmp}).Error("UpdateComponent failed")
- return err
+ return result, err
  }
+ result["update"] = append(result["update"], cmp.Name)
  }
  }
 
  for _, cmp := range create {
- _, cmpForcedUpdate := needForcedUpdate[cmp.Name]
+ cmpForcedUpdate := needForcedUpdate[cmp.Name]
  if e.stageEnabled("create") || (e.stageEnabled("update") && cmpForcedUpdate) {
  if err := logDifferences(logrus.Infof, "Create: "+cmp.Name, nil, cmp); err != nil {
- return err
+ return result, err
  }
 
  if err := e.CreateComponent(cmp); err != nil {
  logrus.WithFields(logrus.Fields{"error": err, "component": cmp}).Error("CreateComponent failed")
- return err
+ return result, err
  }
+ result["create"] = append(result["create"], cmp.Name)
  }
  }
 
- logrus.WithFields(logrus.Fields{"created": len(create), "updated": len(update), "deleted": len(delete)}).Info("Applied desired state successfully")
- return nil
+ logrus.WithFields(logrus.Fields{"created": len(result["create"]), "updated": len(result["update"]), "deleted": len(result["delete"])}).Info("Applied desired state successfully")
+ return result, nil
 }
 
 func (e *executor) stageEnabled(stage string) bool {

pkg/landscaper/executor_test.go

@@ -87,24 +87,29 @@ func TestExecutorApply(t *testing.T) {
  },
  }
 
- err := NewExecutor(helmMock, chartLoadMock, secretsMock, false, false, waitTimeout, disabledStages).Apply(des, cur)
+ result, err := NewExecutor(helmMock, chartLoadMock, secretsMock, false, false, waitTimeout, disabledStages).Apply(des, cur)
  require.NoError(t, err)
-
+ require.Equal(t, len(result["create"]), 1)
+ require.Equal(t, len(result["update"]), 1)
+ require.Equal(t, len(result["delete"]), 1)
+ require.Equal(t, result["create"][0], nu.Name)
+ require.Equal(t, result["update"][0], updiff.Name)
+ require.Equal(t, result["delete"][0], rem.Name)
 }
 
-func TestExecutorApplyWithForcedUpdatesAndDeleteCreateDisable(t *testing.T) {
+func TestExecutorApplyWithForcedUpdatesAndDeleteCreateDisableWhenExistingSecretValueChanges(t *testing.T) {
  chartPath := "/opt/store/whatever/path/"
 
  nu := newTestComponent("new-one")
- nu.Namespace = "recognizable-new-one"
  rem := newTestComponent("busted-one")
  up := newTestComponent("updated-one")
+ up.Namespace = "recognizable-new-one"
  updiff := newTestComponent("updated-one")
- updiff.Configuration["FlushSize"] = 4
- updiff.SecretNames = SecretNames{"newSecret": "somethingNew"}
+ updiff.Namespace = up.Namespace
 
  updiff.SecretValues = SecretValues{
- "newSecret": []byte("somethingNew"),
+ "TestSecret1": []byte("secret value 1"),
+ "TestSecret2": []byte("new secret value 2"),
  }
 
  des := Components{nu.Name: nu, updiff.Name: updiff}
@@ -118,12 +123,12 @@ func TestExecutorApplyWithForcedUpdatesAndDeleteCreateDisable(t *testing.T) {
  },
  deleteRelease: func(rlsName string, opts ...helm.DeleteOption) (*services.UninstallReleaseResponse, error) {
  t.Logf("deleteRelease %#v", rlsName)
- require.Equal(t, rlsName, "busted-one")
+ require.Equal(t, rlsName, "updated-one")
  return nil, nil
  },
  updateRelease: func(rlsName string, chStr string, opts ...helm.UpdateOption) (*services.UpdateReleaseResponse, error) {
  t.Logf("updateRelease %#v %#v %#v", rlsName, chStr, opts)
- require.Equal(t, rlsName, "updated-one")
+ require.Equal(t, rlsName, "this-should-never-been-called")
  return nil, nil
  }}
  chartLoadMock := MockChartLoader(func(chartRef string) (*chart.Chart, string, error) {
@@ -141,9 +146,13 @@ func TestExecutorApplyWithForcedUpdatesAndDeleteCreateDisable(t *testing.T) {
 
  createDeleteDisabled := []string{"create", "delete"}
 
- err := NewExecutor(helmMock, chartLoadMock, secretsMock, false, false, waitTimeout, createDeleteDisabled).Apply(des, cur)
+ result, err := NewExecutor(helmMock, chartLoadMock, secretsMock, false, false, waitTimeout, createDeleteDisabled).Apply(des, cur)
  require.NoError(t, err)
-
+ require.Equal(t, len(result["create"]), 1)
+ require.Equal(t, len(result["update"]), 0)
+ require.Equal(t, len(result["delete"]), 1)
+ require.Equal(t, result["create"][0], updiff.Name)
+ require.Equal(t, result["delete"][0], updiff.Name)
 }
 
 func TestExecutorCreate(t *testing.T) {
@@ -311,8 +320,11 @@ func TestExecutorApplyWithDisabledStages(t *testing.T) {
  },
  }
 
- err := NewExecutor(helmMock, chartLoadMock, secretsMock, false, false, waitTimeout, []string{"delete", "create", "update"}).Apply(des, cur)
+ result, err := NewExecutor(helmMock, chartLoadMock, secretsMock, false, false, waitTimeout, []string{"delete", "create", "update"}).Apply(des, cur)
  require.NoError(t, err)
+ require.Equal(t, len(result["create"]), 0)
+ require.Equal(t, len(result["update"]), 0)
+ require.Equal(t, len(result["delete"]), 0)
 }
 
 func newTestComponent(name string) *Component {