Skip to content

Commit

Permalink
fixed stuttering issues introduced by new alerting http client
Browse files Browse the repository at this point in the history
  • Loading branch information
alexandreLamarre committed Oct 21, 2022
1 parent bfe5b30 commit 59e393d
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 269 deletions.
34 changes: 25 additions & 9 deletions pkg/alerting/backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"net/http"
"net/url"
"path"
"reflect"
"strings"
"time"

Expand All @@ -18,6 +17,7 @@ import (
"github.com/rancher/opni/pkg/logger"
"github.com/tidwall/gjson"
"go.uber.org/zap"
"golang.org/x/exp/slices"
)

const (
Expand Down Expand Up @@ -70,6 +70,18 @@ func WithRetrier(retrier backoffv2.Policy) AlertManagerApiOption {
}
}

func WithDefaultRetrier() AlertManagerApiOption {
return func(o *AlertManagerApiOptions) {
b := backoffv2.Exponential(
backoffv2.WithMinInterval(time.Second*2),
backoffv2.WithMaxInterval(time.Second*5),
backoffv2.WithMaxRetries(4),
backoffv2.WithMultiplier(1.2),
)
o.backoff = &b
}
}

func WithExpectClosure(expectClosure func(*http.Response) error) AlertManagerApiOption {
return func(o *AlertManagerApiOptions) {
o.expectClosure = expectClosure
Expand Down Expand Up @@ -359,7 +371,15 @@ func NewExpectStatusOk() func(*http.Response) error {
}
}

// FIXME: there has to be a way to do this that will work
func NewExpectStatusCodes(expectedCodes []int) func(*http.Response) error {
return func(resp *http.Response) error {
if !slices.Contains(expectedCodes, resp.StatusCode) {
return fmt.Errorf("unexpected status code: %d", resp.StatusCode)
}
return nil
}
}

func NewExpectConfigEqual(expectedConfig string) func(*http.Response) error {
// newConfig := newConfig
return func(resp *http.Response) error {
Expand Down Expand Up @@ -390,14 +410,10 @@ func NewExpectConfigEqual(expectedConfig string) func(*http.Response) error {
} else {
lg.Debug("%v", r2)
}
// cannot compare entire structs since AlertManager does invisble maintenance on the config
if !reflect.DeepEqual(r1.Receivers, r2.Receivers) {
return fmt.Errorf("current alertmanager receivers differ from expected receivers")
}
if !reflect.DeepEqual(r1.Route, r2.Route) {
return fmt.Errorf("current alertmanager route differ from expected route")
if r1 != r2 { // this comparison is good enough for our purposes
return nil
}
return nil
return fmt.Errorf("config.original not equal to expected config")
}
}

Expand Down
251 changes: 0 additions & 251 deletions pkg/alerting/routing/endpoint_test.go

This file was deleted.

3 changes: 3 additions & 0 deletions plugins/alerting/pkg/alerting/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ func (p *Plugin) TriggerAlerts(ctx context.Context, req *alertingv1alpha.Trigger
if err != nil {
return nil, err
}
// FIXME: submitting this during a reload can lead to a context.Cancel
// on this post operation, however its unclear if this would lead to actual
// problems in this function
apiNode := backend.NewAlertManagerPostAlertClient(
availableEndpoint,
ctx,
Expand Down
11 changes: 8 additions & 3 deletions plugins/alerting/pkg/alerting/api_conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ func (p *Plugin) AlertConditionStatus(ctx context.Context, ref *corev1.Reference
ctx,
backend.WithLogger(lg),
backend.WithExpectClosure(func(resp *http.Response) error {

if resp.StatusCode != http.StatusOK {
return fmt.Errorf("unexpected status code %d", resp.StatusCode)
}
b, err := ioutil.ReadAll(resp.Body)
if err != nil {
lg.Errorf("failed to read body : %s", err)
Expand Down Expand Up @@ -294,7 +296,7 @@ func (p *Plugin) ActivateSilence(ctx context.Context, req *alertingv1alpha.Silen
backend.WithLogger(p.Logger),
backend.WithPostSilenceBody(req.ConditionId.Id, req.Duration.AsDuration(), silenceID),
backend.WithExpectClosure(func(resp *http.Response) error {
if resp.StatusCode != 200 {
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("failed to create silence : %s", resp.Status)
}
return json.NewDecoder(resp.Body).Decode(respSilence)
Expand Down Expand Up @@ -342,7 +344,10 @@ func (p *Plugin) DeactivateSilence(ctx context.Context, req *corev1.Reference) (
apiNode := backend.NewAlertManagerDeleteSilenceClient(
availableEndpoint,
existing.Silence.SilenceId,
ctx)
ctx,
backend.WithLogger(p.Logger),
backend.WithExpectClosure(backend.NewExpectStatusOk()),
)
err = apiNode.DoRequest()
if err != nil {
p.Logger.Errorf("failed to delete silence : %s", err)
Expand Down
Loading

0 comments on commit 59e393d

Please sign in to comment.