From 59185c1f1d50eceeaa1ca5beebd3e85cd733911e Mon Sep 17 00:00:00 2001 From: Stefan Benz Date: Tue, 3 Mar 2020 15:15:24 +0100 Subject: [PATCH] fix(state): corrected state handling, so that only necessary internal states are used --- internal/bundle/bundle.go | 89 ++++++++++--------------- internal/bundle/bundle_test.go | 20 +++--- internal/crd/crd.go | 1 + internal/crd/v1beta1/crd.go | 9 ++- internal/gitcrd/v1beta1/gitcrd.go | 3 + internal/templator/helm/helm.go | 14 +--- internal/templator/helm/mutate.go | 11 ++- internal/templator/helm/mutatevalues.go | 11 +-- internal/templator/helm/presteps.go | 26 ++++---- internal/templator/helm/template.go | 62 ++++++----------- internal/templator/templator.go | 5 +- internal/templator/yaml/template.go | 35 ++++------ internal/templator/yaml/yaml.go | 9 +-- 13 files changed, 117 insertions(+), 178 deletions(-) diff --git a/internal/bundle/bundle.go b/internal/bundle/bundle.go index 927c268..a598031 100644 --- a/internal/bundle/bundle.go +++ b/internal/bundle/bundle.go @@ -29,7 +29,6 @@ type Bundle struct { HelmTemplator templator.Templator YamlTemplator templator.Templator monitor mntr.Monitor - status error } func New(conf *config.Config) *Bundle { @@ -44,26 +43,18 @@ func New(conf *config.Config) *Bundle { HelmTemplator: helmTemplator, YamlTemplator: yamlTemplator, Applications: apps, - status: nil, } return b } -func (b *Bundle) GetStatus() error { - return b.status -} +func (b *Bundle) CleanUp() error { -func (b *Bundle) CleanUp() *Bundle { - if b.GetStatus() != nil { - return b + err := b.HelmTemplator.CleanUp() + if err != nil { + return err } - b.status = b.HelmTemplator.CleanUp().GetStatus() - if b.GetStatus() != nil { - return b - } - b.status = b.YamlTemplator.CleanUp().GetStatus() - return b + return b.YamlTemplator.CleanUp() } func (b *Bundle) GetApplications() map[name.Application]application.Application { @@ -71,9 +62,6 @@ func (b *Bundle) GetApplications() map[name.Application]application.Application } func (b *Bundle) AddApplicationsByBundleName(name name.Bundle) error { - if err := b.GetStatus(); err != nil { - return err - } names := bundles.Get(name) if names == nil { @@ -82,72 +70,61 @@ func (b *Bundle) AddApplicationsByBundleName(name name.Bundle) error { bnew := b for _, name := range names { - bnew = bnew.AddApplicationByName(name) - if err := bnew.GetStatus(); err != nil { + if err := bnew.AddApplicationByName(name); err != nil { return err } } return nil } -func (b *Bundle) AddApplicationByName(appName name.Application) *Bundle { - if b.GetStatus() != nil { - return b - } +func (b *Bundle) AddApplicationByName(appName name.Application) error { app := application.New(b.monitor, appName) return b.AddApplication(app) } -func (b *Bundle) AddApplication(app application.Application) *Bundle { - if b.GetStatus() != nil { - return b - } +func (b *Bundle) AddApplication(app application.Application) error { if _, found := b.Applications[app.GetName()]; found { - b.status = errors.New("Application already in bundle") - return b + return errors.New("Application already in bundle") } b.Applications[app.GetName()] = app - return b + return nil } -func (b *Bundle) Reconcile(currentResourceList []*clientgo.Resource, spec *v1beta1.ToolsetSpec) *Bundle { - if b.status != nil { - return b - } +func (b *Bundle) Reconcile(currentResourceList []*clientgo.Resource, spec *v1beta1.ToolsetSpec) error { applicationCount := 0 // go through list of application until every application is reconciled // and this orderNumber by orderNumber (default is 1) for orderNumber := 0; applicationCount < len(b.Applications); orderNumber++ { var wg sync.WaitGroup + errList := make(map[name.Application]chan error, len(b.Applications)) for appName := range b.Applications { //if application has the same orderNumber as currently iterating the reconcile the application if application.GetOrderNumber(appName) == orderNumber { wg.Add(1) - go b.ReconcileApplication(currentResourceList, appName, spec, &wg) - if err := b.GetStatus(); err != nil { - b.status = err - return b - } + errChan := make(chan error) + go b.ReconcileApplication(currentResourceList, appName, spec, &wg, errChan) applicationCount++ + errList[appName] = errChan + } + } + for appName, errChan := range errList { + if err := <-errChan; err != nil { + return errors.Wrapf(err, "Error while reconciling application %s", appName.String()) } } wg.Wait() } - return b + return nil } -func (b *Bundle) ReconcileApplication(currentResourceList []*clientgo.Resource, appName name.Application, spec *v1beta1.ToolsetSpec, wg *sync.WaitGroup) *Bundle { +func (b *Bundle) ReconcileApplication(currentResourceList []*clientgo.Resource, appName name.Application, spec *v1beta1.ToolsetSpec, wg *sync.WaitGroup, errChan chan error) { defer wg.Done() - if b.status != nil { - return b - } - logFields := map[string]interface{}{ "application": appName, "action": "reconciling", @@ -156,9 +133,10 @@ func (b *Bundle) ReconcileApplication(currentResourceList []*clientgo.Resource, app, found := b.Applications[appName] if !found { - b.status = errors.New("Application not found") - monitor.Error(b.status) - return b + err := errors.New("Application not found") + monitor.Error(err) + errChan <- err + return } monitor.Info("Start") @@ -180,16 +158,21 @@ func (b *Bundle) ReconcileApplication(currentResourceList []*clientgo.Resource, _, usedHelm := app.(application.HelmApplication) if usedHelm { - b.status = b.HelmTemplator.Template(app, spec, resultFunc).GetStatus() - if b.status != nil { - return b + err := b.HelmTemplator.Template(app, spec, resultFunc) + if err != nil { + errChan <- err + return } } _, usedYaml := app.(application.YAMLApplication) if usedYaml { - b.status = b.YamlTemplator.Template(app, spec, resultFunc).GetStatus() + err := b.YamlTemplator.Template(app, spec, resultFunc) + if err != nil { + errChan <- err + return + } } monitor.Info("Done") - return b + errChan <- nil } diff --git a/internal/bundle/bundle_test.go b/internal/bundle/bundle_test.go index bbd39ea..48a411c 100644 --- a/internal/bundle/bundle_test.go +++ b/internal/bundle/bundle_test.go @@ -127,13 +127,13 @@ func TestBundle_AddApplication_AlreadyAdded(t *testing.T) { out, _ := yamlv3.Marshal(testHelperResource) app.SetDeploy(spec, true).SetGetYaml(spec, string(out)) - err := b.AddApplication(app.Application()).GetStatus() + err := b.AddApplication(app.Application()) assert.NoError(t, err) apps := b.GetApplications() assert.Equal(t, 1, len(apps)) - err2 := b.AddApplication(app.Application()).GetStatus() + err2 := b.AddApplication(app.Application()) assert.Error(t, err2) } @@ -153,8 +153,9 @@ func TestBundle_ReconcileApplication(t *testing.T) { var wg sync.WaitGroup wg.Add(1) - err := b.ReconcileApplication(resources, app.Application().GetName(), spec, &wg).GetStatus() - assert.NoError(t, err) + errChan := make(chan error) + go b.ReconcileApplication(resources, app.Application().GetName(), spec, &wg, errChan) + assert.NoError(t, <-errChan) } func TestBundle_ReconcileApplication_nonexistent(t *testing.T) { @@ -170,8 +171,9 @@ func TestBundle_ReconcileApplication_nonexistent(t *testing.T) { var wg sync.WaitGroup wg.Add(1) - err := b.ReconcileApplication(resources, app.Application().GetName(), nil, &wg).GetStatus() - assert.Error(t, err) + errChan := make(chan error) + go b.ReconcileApplication(resources, app.Application().GetName(), nil, &wg, errChan) + assert.Error(t, <-errChan) } func TestBundle_Reconcile(t *testing.T) { @@ -186,8 +188,7 @@ func TestBundle_Reconcile(t *testing.T) { resources := []*clientgo.Resource{} - b.Reconcile(resources, spec) - err := b.GetStatus() + err := b.Reconcile(resources, spec) assert.NoError(t, err) } @@ -196,7 +197,6 @@ func TestBundle_Reconcile_NoApplications(t *testing.T) { spec := &v1beta1.ToolsetSpec{} resources := []*clientgo.Resource{} - b.Reconcile(resources, spec) - err := b.GetStatus() + err := b.Reconcile(resources, spec) assert.NoError(t, err) } diff --git a/internal/crd/crd.go b/internal/crd/crd.go index 5cfeac7..44e7ae4 100644 --- a/internal/crd/crd.go +++ b/internal/crd/crd.go @@ -22,6 +22,7 @@ type Crd interface { Reconcile([]*clientgo.Resource, *toolsetsv1beta1.Toolset) CleanUp() GetStatus() error + SetBackStatus() } func New(conf *config.Config) (Crd, error) { diff --git a/internal/crd/v1beta1/crd.go b/internal/crd/v1beta1/crd.go index b415104..b927b05 100644 --- a/internal/crd/v1beta1/crd.go +++ b/internal/crd/v1beta1/crd.go @@ -29,12 +29,16 @@ func (c *Crd) GetStatus() error { return c.status } +func (c *Crd) SetBackStatus() { + c.status = nil +} + func (c *Crd) CleanUp() { if c.GetStatus() != nil { return } - c.status = c.bundle.CleanUp().GetStatus() + c.status = c.bundle.CleanUp() } func GetVersion() name.Version { @@ -94,6 +98,7 @@ func (c *Crd) Reconcile(currentResourceList []*clientgo.Resource, toolsetCRD *to if c.GetStatus() != nil { return } + logFields := map[string]interface{}{ "CRD": toolsetCRD.Name, "action": "reconciling", @@ -112,5 +117,5 @@ func (c *Crd) Reconcile(currentResourceList []*clientgo.Resource, toolsetCRD *to return } - c.status = c.bundle.Reconcile(currentResourceList, toolsetCRD.Spec).GetStatus() + c.status = c.bundle.Reconcile(currentResourceList, toolsetCRD.Spec) } diff --git a/internal/gitcrd/v1beta1/gitcrd.go b/internal/gitcrd/v1beta1/gitcrd.go index 5ddbfe2..0b25ab0 100644 --- a/internal/gitcrd/v1beta1/gitcrd.go +++ b/internal/gitcrd/v1beta1/gitcrd.go @@ -65,7 +65,9 @@ func New(conf *config.Config) (*GitCrd, error) { func (c *GitCrd) GetStatus() error { return c.status } + func (c *GitCrd) SetBackStatus() { + c.crd.SetBackStatus() c.status = nil } @@ -160,6 +162,7 @@ func (c *GitCrd) Reconcile(currentResourceList []*clientgo.Resource) { } } } + func (c *GitCrd) getCrdContent() (*toolsetsv1beta1.Toolset, error) { c.gitMutex.Lock() defer c.gitMutex.Unlock() diff --git a/internal/templator/helm/helm.go b/internal/templator/helm/helm.go index d8922fe..76d8e72 100644 --- a/internal/templator/helm/helm.go +++ b/internal/templator/helm/helm.go @@ -24,7 +24,6 @@ func GetPrio() name.Templator { type Helm struct { overlay string - status error monitor mntr.Monitor templatorDirectoryPath string } @@ -37,13 +36,8 @@ func New(monitor mntr.Monitor, overlay, templatorDirectoryPath string) templator } } -func (h *Helm) CleanUp() templator.Templator { - if h.status != nil { - return h - } - - h.status = os.RemoveAll(h.templatorDirectoryPath) - return h +func (h *Helm) CleanUp() error { + return os.RemoveAll(h.templatorDirectoryPath) } func (h *Helm) getResultsFileDirectory(appName name.Application, overlay, basePath string) string { @@ -54,10 +48,6 @@ func (h *Helm) GetResultsFilePath(appName name.Application, overlay, basePath st return filepath.Join(h.getResultsFileDirectory(appName, overlay, basePath), "results.yaml") } -func (h *Helm) GetStatus() error { - return h.status -} - func checkTemplatorInterface(templatorInterface interface{}) (templator.HelmApplication, error) { app, isTemplator := templatorInterface.(templator.HelmApplication) if !isTemplator { diff --git a/internal/templator/helm/mutate.go b/internal/templator/helm/mutate.go index ebf4ebf..ce7ccea 100644 --- a/internal/templator/helm/mutate.go +++ b/internal/templator/helm/mutate.go @@ -11,10 +11,7 @@ type TemplatorMutate interface { HelmMutate(mntr.Monitor, *v1beta1.ToolsetSpec, string) error } -func (h *Helm) mutate(app interface{}, spec *v1beta1.ToolsetSpec) templator.Templator { - if h.status != nil { - return h - } +func (h *Helm) mutate(app interface{}, spec *v1beta1.ToolsetSpec) error { mutate, ok := app.(TemplatorMutate) if ok { @@ -30,9 +27,9 @@ func (h *Helm) mutate(app interface{}, spec *v1beta1.ToolsetSpec) templator.Temp resultfilepath := h.GetResultsFilePath(mutate.GetName(), h.overlay, h.templatorDirectoryPath) if err := mutate.HelmMutate(mutateMonitor, spec, resultfilepath); err != nil { - h.status = err - return h + return err } } - return h + + return nil } diff --git a/internal/templator/helm/mutatevalues.go b/internal/templator/helm/mutatevalues.go index 3b9b0a2..3a44d08 100644 --- a/internal/templator/helm/mutatevalues.go +++ b/internal/templator/helm/mutatevalues.go @@ -11,11 +11,7 @@ type TemplatorMutateValues interface { HelmMutateValues(mntr.Monitor, *v1beta1.ToolsetSpec, string) error } -func (h *Helm) mutateValue(app interface{}, spec *v1beta1.ToolsetSpec, valuesAbsFilePath string) templator.Templator { - if h.status != nil { - return h - } - +func (h *Helm) mutateValue(app interface{}, spec *v1beta1.ToolsetSpec, valuesAbsFilePath string) error { mutate, ok := app.(TemplatorMutateValues) if ok { @@ -28,10 +24,9 @@ func (h *Helm) mutateValue(app interface{}, spec *v1beta1.ToolsetSpec, valuesAbs mutateMonitor.Debug("Mutate values") if err := mutate.HelmMutateValues(mutateMonitor, spec, valuesAbsFilePath); err != nil { - h.status = err - return h + return err } } - return h + return nil } diff --git a/internal/templator/helm/presteps.go b/internal/templator/helm/presteps.go index 55a6ddf..916fa04 100644 --- a/internal/templator/helm/presteps.go +++ b/internal/templator/helm/presteps.go @@ -13,10 +13,7 @@ type TemplatorPreSteps interface { HelmPreApplySteps(mntr.Monitor, *v1beta1.ToolsetSpec) ([]interface{}, error) } -func (h *Helm) preApplySteps(app interface{}, spec *v1beta1.ToolsetSpec) templator.Templator { - if h.status != nil { - return h - } +func (h *Helm) preApplySteps(app interface{}, spec *v1beta1.ToolsetSpec) error { pre, ok := app.(TemplatorPreSteps) if ok { @@ -30,25 +27,26 @@ func (h *Helm) preApplySteps(app interface{}, spec *v1beta1.ToolsetSpec) templat monitor.Debug("Pre-steps") resources, err := pre.HelmPreApplySteps(monitor, spec) if err != nil { - h.status = errors.Wrapf(err, "Error while processing pre-steps for application %s", pre.GetName().String()) - return h + return errors.Wrapf(err, "Error while processing pre-steps for application %s", pre.GetName().String()) } resultfilepath := h.GetResultsFilePath(pre.GetName(), h.overlay, h.templatorDirectoryPath) for i, resource := range resources { value, isString := resource.(string) + if isString { - h.status = helper.AddStringObjectToYaml(resultfilepath, value) + err := helper.AddStringObjectToYaml(resultfilepath, value) + if err != nil { + return errors.Wrapf(err, "Error while adding element %d to result-file", i) + } } else { - h.status = helper.AddStructToYaml(resultfilepath, resource) - } - - if h.status != nil { - h.status = errors.Wrapf(err, "Error while adding element %d to result-file", i) - return h + err = helper.AddStructToYaml(resultfilepath, resource) + if err != nil { + return errors.Wrapf(err, "Error while adding element %d to result-file", i) + } } } } - return h + return nil } diff --git a/internal/templator/helm/template.go b/internal/templator/helm/template.go index 47c1720..1c4c628 100644 --- a/internal/templator/helm/template.go +++ b/internal/templator/helm/template.go @@ -11,15 +11,10 @@ import ( "github.com/pkg/errors" ) -func (h *Helm) Template(appInterface interface{}, spec *v1beta1.ToolsetSpec, resultFunc func(resultFilePath, namespace string) error) templator.Templator { - if h.status != nil { - return h - } - +func (h *Helm) Template(appInterface interface{}, spec *v1beta1.ToolsetSpec, resultFunc func(resultFilePath, namespace string) error) error { app, err := checkTemplatorInterface(appInterface) if err != nil { - h.status = err - return h + return err } logFields := map[string]interface{}{ @@ -30,72 +25,61 @@ func (h *Helm) Template(appInterface interface{}, spec *v1beta1.ToolsetSpec, res monitor := h.monitor.WithFields(logFields) monitor.Debug("Deleting old results") - h.status = h.deleteResults(app) - if h.status != nil { - return h + err = h.deleteResults(app) + if err != nil { + return err } var resultAbsFilePath string resultfilepath := h.GetResultsFilePath(app.GetName(), h.overlay, h.templatorDirectoryPath) - resultAbsFilePath, h.status = filepath.Abs(resultfilepath) - if h.status != nil { - return h + resultAbsFilePath, err = filepath.Abs(resultfilepath) + if err != nil { + return err } valuesAbsFilePath, err := helper.GetAbsPath(h.templatorDirectoryPath, app.GetName().String(), h.overlay, "values.yaml") if err != nil { monitor.Error(err) - h.status = err - return h + return err } if err := h.prepareHelmTemplate(h.overlay, app, spec, valuesAbsFilePath); err != nil { - h.status = err monitor.Error(err) - return h + return err } - if err := h.mutateValue(app, spec, valuesAbsFilePath).GetStatus(); err != nil { - h.status = err + if err := h.mutateValue(app, spec, valuesAbsFilePath); err != nil { monitor.Error(err) - return h + return err } if err := h.runHelmTemplate(h.overlay, app, valuesAbsFilePath, resultAbsFilePath); err != nil { - h.status = err monitor.Error(err) - return h + return err } deleteKind := "Namespace" - h.status = helper.DeleteKindFromYaml(resultAbsFilePath, deleteKind) - if h.status != nil { - h.status = errors.Wrapf(h.status, "Error while trying to delete kind %s from results", deleteKind) - return h + err = helper.DeleteKindFromYaml(resultAbsFilePath, deleteKind) + if err != nil { + return errors.Wrapf(err, "Error while trying to delete kind %s from results", deleteKind) } // mutate templated results - if err := h.mutate(app, spec).GetStatus(); err != nil { - h.status = err - return h + if err := h.mutate(app, spec); err != nil { + return err } // pre apply steps - if err := h.preApplySteps(app, spec).GetStatus(); err != nil { - h.status = err - return h + if err := h.preApplySteps(app, spec); err != nil { + return err } // func to apply - h.status = resultFunc(resultAbsFilePath, app.GetNamespace()) - return h + return resultFunc(resultAbsFilePath, app.GetNamespace()) } func (h *Helm) prepareHelmTemplate(overlay string, app templator.HelmApplication, spec *v1beta1.ToolsetSpec, valuesAbsFilePath string) error { - if h.status != nil { - return h.status - } logFields := map[string]interface{}{ "application": app.GetName().String(), @@ -122,10 +106,6 @@ func (h *Helm) prepareHelmTemplate(overlay string, app templator.HelmApplication } func (h *Helm) runHelmTemplate(overlay string, app templator.HelmApplication, valuesAbsFilePath, resultAbsFilePath string) error { - if h.status != nil { - return h.status - } - logFields := map[string]interface{}{ "application": app.GetName().String(), "overlay": overlay, diff --git a/internal/templator/templator.go b/internal/templator/templator.go index 03a71e7..d56b952 100644 --- a/internal/templator/templator.go +++ b/internal/templator/templator.go @@ -8,10 +8,9 @@ import ( ) type Templator interface { - Template(interface{}, *v1beta1.ToolsetSpec, func(string, string) error) Templator + Template(interface{}, *v1beta1.ToolsetSpec, func(string, string) error) error GetResultsFilePath(name.Application, string, string) string - CleanUp() Templator - GetStatus() error + CleanUp() error } type BaseApplication interface { diff --git a/internal/templator/yaml/template.go b/internal/templator/yaml/template.go index c469268..c00f5d0 100644 --- a/internal/templator/yaml/template.go +++ b/internal/templator/yaml/template.go @@ -5,18 +5,12 @@ import ( "github.com/caos/boom/api/v1beta1" "github.com/caos/boom/internal/helper" - "github.com/caos/boom/internal/templator" ) -func (y *YAML) Template(appInterface interface{}, spec *v1beta1.ToolsetSpec, resultFunc func(string, string) error) templator.Templator { - if y.GetStatus() != nil { - return y - } - +func (y *YAML) Template(appInterface interface{}, spec *v1beta1.ToolsetSpec, resultFunc func(string, string) error) error { app, err := checkTemplatorInterface(appInterface) if err != nil { - y.status = err - return y + return err } yamlInterface := app.GetYaml(y.monitor, spec) @@ -25,29 +19,28 @@ func (y *YAML) Template(appInterface interface{}, spec *v1beta1.ToolsetSpec, res resultAbsFilePath, err := filepath.Abs(resultfilepath) if err != nil { - y.status = err - return y + return err } resultAbsFileDirectory, err := filepath.Abs(resultfiledirectory) if err != nil { - y.status = err - return y + return err } if err := helper.RecreatePath(resultAbsFileDirectory); err != nil { - y.status = err - return y + return err } if yamlStr, isString := yamlInterface.(string); isString { - y.status = helper.AddStringObjectToYaml(resultAbsFilePath, yamlStr) + err = helper.AddStringObjectToYaml(resultAbsFilePath, yamlStr) + if err != nil { + return err + } } else { - y.status = helper.AddStringObjectToYaml(resultAbsFilePath, yamlStr) - } - if y.GetStatus() != nil { - return y + err = helper.AddStringObjectToYaml(resultAbsFilePath, yamlStr) + if err != nil { + return err + } } - y.status = resultFunc(resultAbsFilePath, "") - return y + return resultFunc(resultAbsFilePath, "") } diff --git a/internal/templator/yaml/yaml.go b/internal/templator/yaml/yaml.go index de916c7..d45b72f 100644 --- a/internal/templator/yaml/yaml.go +++ b/internal/templator/yaml/yaml.go @@ -19,7 +19,6 @@ func GetName() name.Templator { type YAML struct { monitor mntr.Monitor - status error overlay string templatorDirectoryPath string } @@ -32,10 +31,6 @@ func New(monitor mntr.Monitor, overlay, templatorDirectoryPath string) *YAML { } } -func (y *YAML) GetStatus() error { - return y.status -} - func (y *YAML) getResultsFileDirectory(appName name.Application, overlay, basePath string) string { return filepath.Join(basePath, appName.String(), overlay, "results") } @@ -44,8 +39,8 @@ func (y *YAML) GetResultsFilePath(appName name.Application, overlay, basePath st return filepath.Join(y.getResultsFileDirectory(appName, overlay, basePath), "results.yaml") } -func (y *YAML) CleanUp() templator.Templator { - return y +func (y *YAML) CleanUp() error { + return nil } func checkTemplatorInterface(templatorInterface interface{}) (templator.YamlApplication, error) {