From 38b4e3463ab93c1800a8fbbb73533b1b7291cce1 Mon Sep 17 00:00:00 2001 From: olevchyk Date: Fri, 28 Sep 2018 17:30:52 +0200 Subject: [PATCH 1/5] Retry to RetryNotify to catch AlreadyExistsException --- main.go | 14 +++++++++++++- provider/aws/aws.go | 2 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index 2222b38..597f1d0 100644 --- a/main.go +++ b/main.go @@ -287,7 +287,19 @@ func enterProvider(wg *sync.WaitGroup, p provider.Provider, mergerCH <-chan []st createFunc := func() error { return p.Create(output) } - err = backoff.Retry(createFunc, retry) + notify := func(err error, t time.Duration) { + log.Errorf("%v at time: %v", err, t) + if awsErr, ok := err.(awserr.Error); ok { + if awsErr.Code() == "AlreadyExistsException" { + log.Debugf("got AlreadyExistsException on Stack creation, trying to update it") + err = p.Update(output) + if err != nil { + log.Error(err) + } + } + } + } + err = backoff.RetryNotify(createFunc, retry, notify) if err != nil { log.Error(err) } diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 9d1dd00..a8ab122 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -113,7 +113,7 @@ func (p *AwsProvider) Create(nets []string) error { stackID, err := p.createCFStack(nets, &spec) if err != nil { - return errors.Wrap(err, "failed to create CF stack") + return err } log.Infof("%s: Created CF Stack %s", p, stackID) return nil From 471c7951d06f4a188d3433a82b3cb84e37bc885e Mon Sep 17 00:00:00 2001 From: olevchyk Date: Mon, 1 Oct 2018 10:36:19 +0200 Subject: [PATCH 2/5] 2nd Create function to use createNotify as well --- main.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/main.go b/main.go index 597f1d0..32d254e 100644 --- a/main.go +++ b/main.go @@ -272,6 +272,18 @@ func enterProvider(wg *sync.WaitGroup, p provider.Provider, mergerCH <-chan []st } var err error + createNotify := func(err error, t time.Duration) { + log.Errorf("%v at time: %v", err, t) + if awsErr, ok := err.(awserr.Error); ok { + if awsErr.Code() == "AlreadyExistsException" { + log.Debugf("got AlreadyExistsException on Stack creation, trying to update it") + err = p.Update(output) + if err != nil { + log.Error(err) + } + } + } + } if len(input) == 0 { // not caused by faulty value in CIDR string if !sameValues(resultCache, output) { resultCache = output @@ -287,19 +299,7 @@ func enterProvider(wg *sync.WaitGroup, p provider.Provider, mergerCH <-chan []st createFunc := func() error { return p.Create(output) } - notify := func(err error, t time.Duration) { - log.Errorf("%v at time: %v", err, t) - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == "AlreadyExistsException" { - log.Debugf("got AlreadyExistsException on Stack creation, trying to update it") - err = p.Update(output) - if err != nil { - log.Error(err) - } - } - } - } - err = backoff.RetryNotify(createFunc, retry, notify) + err = backoff.RetryNotify(createFunc, retry, createNotify) if err != nil { log.Error(err) } @@ -315,7 +315,7 @@ func enterProvider(wg *sync.WaitGroup, p provider.Provider, mergerCH <-chan []st createFunc := func() error { return p.Create(output) } - err = backoff.Retry(createFunc, retry) + err = backoff.RetryNotify(createFunc, retry, createNotify) if err != nil { log.Error(err) } From ee439b4e1052cda248515a109a917a0e6a734549 Mon Sep 17 00:00:00 2001 From: olevchyk Date: Mon, 1 Oct 2018 15:05:41 +0200 Subject: [PATCH 3/5] review comments --- main.go | 58 +++++++++++++++++++++++++------------------- main_test.go | 2 +- provider/aws/aws.go | 14 ++++++++++- provider/error.go | 25 +++++++++++++++++++ provider/provider.go | 18 -------------- 5 files changed, 72 insertions(+), 45 deletions(-) create mode 100644 provider/error.go diff --git a/main.go b/main.go index 32d254e..7664cb6 100644 --- a/main.go +++ b/main.go @@ -6,18 +6,18 @@ import ( "os" "os/signal" "sort" - "strings" "sync" "syscall" "time" "github.com/cenkalti/backoff" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/szuecs/kube-static-egress-controller/kube" - provider "github.com/szuecs/kube-static-egress-controller/provider" + "github.com/szuecs/kube-static-egress-controller/provider" + "github.com/szuecs/kube-static-egress-controller/provider/aws" + "github.com/szuecs/kube-static-egress-controller/provider/noop" kingpin "gopkg.in/alecthomas/kingpin.v2" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" @@ -64,6 +64,18 @@ func NewConfig() *Config { return &Config{} } +func newProvider(dry bool, name string, natCidrBlocks, availabilityZones []string, StackTerminationProtection bool) provider.Provider { + switch name { + case aws.ProviderName: + return aws.NewAwsProvider(dry, natCidrBlocks, availabilityZones, StackTerminationProtection) + case noop.ProviderName: + return noop.NewNoopProvider() + default: + log.Fatalf("Unkown provider: %s", name) + } + return nil +} + func allLogLevelsAsStrings() []string { var levels []string for _, level := range log.AllLevels { @@ -125,7 +137,7 @@ func main() { log.SetLevel(ll) log.Debugf("config: %+v", cfg) - p := provider.NewProvider(cfg.DryRun, cfg.Provider, cfg.NatCidrBlocks, cfg.AvailabilityZones, cfg.StackTerminationProtection) + p := newProvider(cfg.DryRun, cfg.Provider, cfg.NatCidrBlocks, cfg.AvailabilityZones, cfg.StackTerminationProtection) run(newKubeClient(), p) } @@ -270,18 +282,16 @@ func enterProvider(wg *sync.WaitGroup, p provider.Provider, mergerCH <-chan []st bootstrap = false continue } - var err error createNotify := func(err error, t time.Duration) { - log.Errorf("%v at time: %v", err, t) - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == "AlreadyExistsException" { - log.Debugf("got AlreadyExistsException on Stack creation, trying to update it") - err = p.Update(output) - if err != nil { - log.Error(err) - } + switch errors.Cause(err).(type) { + case *provider.AlreadyExistsError: + err = p.Update(output) + if err != nil { + log.Error(err) } + default: + log.Errorf("updating stack err: %v at time: %v", err, t) } } if len(input) == 0 { // not caused by faulty value in CIDR string @@ -307,19 +317,17 @@ func enterProvider(wg *sync.WaitGroup, p provider.Provider, mergerCH <-chan []st err = p.Update(output) // create if stack does not exist, but we have targets if err != nil { - switch e := errors.Cause(err).(type) { - case awserr.Error: - log.Infof("%s | %s | %s", e.Code(), e.Message(), e.OrigErr()) - if strings.Contains(e.Message(), "does not exist") { - err = p.Create(output) - createFunc := func() error { - return p.Create(output) - } - err = backoff.RetryNotify(createFunc, retry, createNotify) - if err != nil { - log.Error(err) - } + switch errors.Cause(err).(type) { + case *provider.DoesNotExistError: + createFunc := func() error { + return p.Create(output) + } + err = backoff.RetryNotify(createFunc, retry, createNotify) + if err != nil { + log.Error(err) } + default: + log.Errorf("updating stack err: %v", err) } } } diff --git a/main_test.go b/main_test.go index 18717a3..0f988d9 100644 --- a/main_test.go +++ b/main_test.go @@ -169,7 +169,7 @@ func Test_enterProvider(t *testing.T) { { name: "enterProvider noop quit test", wg: sync.WaitGroup{}, - p: provider.NewProvider(true, noop.ProviderName, []string{}, []string{}, false), + p: newProvider(true, noop.ProviderName, []string{}, []string{}, false), mergerCH: make(chan []string), quitCH: make(chan struct{}), timeout: 3 * time.Second, diff --git a/provider/aws/aws.go b/provider/aws/aws.go index a8ab122..14a08fe 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/client" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/cloudformation" @@ -16,6 +17,7 @@ import ( "github.com/linki/instrumented_http" "github.com/pkg/errors" log "github.com/sirupsen/logrus" + "github.com/szuecs/kube-static-egress-controller/provider" ) const ( @@ -113,7 +115,7 @@ func (p *AwsProvider) Create(nets []string) error { stackID, err := p.createCFStack(nets, &spec) if err != nil { - return err + return errors.Wrap(err, "failed to create CF stack") } log.Infof("%s: Created CF Stack %s", p, stackID) return nil @@ -304,6 +306,11 @@ func (p *AwsProvider) updateCFStack(nets []string, spec *stackSpec) (string, err resp, err := p.cloudformation.UpdateStack(params) if err != nil { + if awsErr, ok := err.(awserr.Error); ok { + if awsErr.Code() == "AlreadyExistsException" { + err = provider.NewAlreadyExistsError(fmt.Sprintf("%s AlreadyExists", stackName)) + } + } return spec.name, err } return aws.StringValue(resp.StackId), nil @@ -335,6 +342,11 @@ func (p *AwsProvider) createCFStack(nets []string, spec *stackSpec) (string, err resp, err := p.cloudformation.CreateStack(params) log.Debugf("Stackoutput: %+v", resp) if err != nil { + if awsErr, ok := err.(awserr.Error); ok { + if strings.Contains(awsErr.Message(), "does not exist") { + err = provider.NewDoesNotExistError(fmt.Sprintf("%s does not exist", stackName)) + } + } return spec.name, err } return aws.StringValue(resp.StackId), nil diff --git a/provider/error.go b/provider/error.go new file mode 100644 index 0000000..84b8b39 --- /dev/null +++ b/provider/error.go @@ -0,0 +1,25 @@ +package provider + +type AlreadyExistsError struct { + Msg string +} + +type DoesNotExistError struct { + Msg string +} + +func (error *AlreadyExistsError) Error() string { + return error.Msg +} + +func (error *DoesNotExistError) Error() string { + return error.Msg +} + +func NewAlreadyExistsError(text string) error { + return &AlreadyExistsError{text} +} + +func NewDoesNotExistError(text string) error { + return &DoesNotExistError{text} +} diff --git a/provider/provider.go b/provider/provider.go index 8c751f1..df5edfb 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -1,26 +1,8 @@ package provider -import ( - log "github.com/sirupsen/logrus" - "github.com/szuecs/kube-static-egress-controller/provider/aws" - "github.com/szuecs/kube-static-egress-controller/provider/noop" -) - type Provider interface { Create([]string) error Update([]string) error Delete() error String() string } - -func NewProvider(dry bool, name string, natCidrBlocks, availabilityZones []string, StackTerminationProtection bool) Provider { - switch name { - case aws.ProviderName: - return aws.NewAwsProvider(dry, natCidrBlocks, availabilityZones, StackTerminationProtection) - case noop.ProviderName: - return noop.NewNoopProvider() - default: - log.Fatalf("Unkown provider: %s", name) - } - return nil -} From 08ee4f06ac1a69907744ff86d9d89a0ff524ee2b Mon Sep 17 00:00:00 2001 From: olevchyk Date: Mon, 1 Oct 2018 18:14:15 +0200 Subject: [PATCH 4/5] applied suggestions from review comments --- main.go | 2 -- provider/error.go | 16 ++++++++-------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/main.go b/main.go index 7664cb6..faad665 100644 --- a/main.go +++ b/main.go @@ -290,8 +290,6 @@ func enterProvider(wg *sync.WaitGroup, p provider.Provider, mergerCH <-chan []st if err != nil { log.Error(err) } - default: - log.Errorf("updating stack err: %v at time: %v", err, t) } } if len(input) == 0 { // not caused by faulty value in CIDR string diff --git a/provider/error.go b/provider/error.go index 84b8b39..867b31a 100644 --- a/provider/error.go +++ b/provider/error.go @@ -1,25 +1,25 @@ package provider type AlreadyExistsError struct { - Msg string + msg string } type DoesNotExistError struct { - Msg string + msg string } func (error *AlreadyExistsError) Error() string { - return error.Msg + return error.msg } func (error *DoesNotExistError) Error() string { - return error.Msg + return error.msg } -func NewAlreadyExistsError(text string) error { - return &AlreadyExistsError{text} +func NewAlreadyExistsError(msg string) error { + return &AlreadyExistsError{msg} } -func NewDoesNotExistError(text string) error { - return &DoesNotExistError{text} +func NewDoesNotExistError(msg string) error { + return &DoesNotExistError{msg} } From 334c8abed4763334a23bbbb344ef26b7020695a8 Mon Sep 17 00:00:00 2001 From: olevchyk Date: Mon, 1 Oct 2018 18:18:52 +0200 Subject: [PATCH 5/5] logging message fix --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index faad665..9075cf0 100644 --- a/main.go +++ b/main.go @@ -325,7 +325,7 @@ func enterProvider(wg *sync.WaitGroup, p provider.Provider, mergerCH <-chan []st log.Error(err) } default: - log.Errorf("updating stack err: %v", err) + log.Errorf("Failed to update stack: %v", err) } } }