Skip to content

Commit

Permalink
Add a threshold for the ratio warning to not notify on small violatio…
Browse files Browse the repository at this point in the history
…ns (#109)
  • Loading branch information
bastjan authored Jul 29, 2024
1 parent 0e32b8a commit f953439
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 14 deletions.
7 changes: 7 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/appuio/appuio-cloud-agent/limits"
"go.uber.org/multierr"
"gopkg.in/inf.v0"
"k8s.io/apimachinery/pkg/api/resource"
"sigs.k8s.io/yaml"
)
Expand All @@ -27,6 +28,12 @@ type Config struct {
// MemoryPerCoreLimits is the fair use limit of memory usage per CPU core
// It is possible to select limits by node selector labels
MemoryPerCoreLimits limits.Limits
// MemoryPerCoreWarnThreshold is the threshold at which a warning is emitted if the memory per core limit is exceeded.
// Should be a decimal number resembling a percentage (e.g. "0.8" for 80%), represented as a string.
// The limit is multiplied by the optional threshold before comparison.
// A threshold of 0.95 would mean that the warnings are emitted if the ratio is below 95% of the limit.
// Thus adding a leniency of 5% to the limit.
MemoryPerCoreWarnThreshold *inf.Dec

// Privileged* is a list of the given type allowed to bypass restrictions.
// Wildcards are supported (e.g. "system:serviceaccount:default:*" or "cluster-*-operator").
Expand Down
31 changes: 31 additions & 0 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/inf.v0"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -82,3 +83,33 @@ func Test_Config_MemoryPerCoreLimits(t *testing.T) {
})
}
}

func Test_Config_MemoryPerCoreWarnThreshold(t *testing.T) {
tc := []struct {
yaml string
expected *inf.Dec
}{
{
yaml: `MemoryPerCoreWarnThreshold: "0.95"`,
expected: inf.NewDec(95, 2),
}, {
expected: nil,
},
}

for _, tC := range tc {
t.Run(tC.yaml, func(t *testing.T) {
t.Parallel()

tmp := t.TempDir()
configPath := filepath.Join(tmp, "config.yaml")
require.NoError(t, os.WriteFile(configPath, []byte(tC.yaml), 0o644))

c, warnings, err := ConfigFromFile(configPath)
assert.Len(t, warnings, 0)
require.NoError(t, err)

assert.Equal(t, tC.expected, c.MemoryPerCoreWarnThreshold)
})
}
}
8 changes: 5 additions & 3 deletions controllers/ratio_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/appuio/appuio-cloud-agent/limits"
"github.com/appuio/appuio-cloud-agent/ratio"
"gopkg.in/inf.v0"
corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/tools/record"

Expand All @@ -24,8 +25,9 @@ type RatioReconciler struct {
Recorder record.EventRecorder
Scheme *runtime.Scheme

Ratio ratioFetcher
RatioLimits limits.Limits
Ratio ratioFetcher
RatioLimits limits.Limits
RatioWarnThreshold *inf.Dec
}

type ratioFetcher interface {
Expand Down Expand Up @@ -62,7 +64,7 @@ func (r *RatioReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
continue
}

if ratio.Below(*limit) {
if ratio.Below(*limit, r.RatioWarnThreshold) {
l.Info("recording warn event: ratio too low")

if err := r.warnPod(ctx, req.Name, req.Namespace, ratio, nodeSel, limit); err != nil {
Expand Down
37 changes: 34 additions & 3 deletions controllers/ratio_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/appuio/appuio-cloud-agent/ratio"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/inf.v0"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -45,6 +46,28 @@ func TestRatioReconciler_Warn(t *testing.T) {
require.Len(t, recorder.Events, 2)
}

func TestRatioReconciler_Threshold_Ok(t *testing.T) {
recorder := record.NewFakeRecorder(4)
_, err := prepareRatioTest(t, testRatioCfg{
limit: resource.MustParse("4G"),
fetchMemory: resource.MustParse("4G"),
fetchCPU: resource.MustParse("1100m"),
recorder: recorder,
threshold: inf.NewDec(90, 2),
obj: []client.Object{
testNs,
testPod,
},
}).Reconcile(context.TODO(), ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: testNs.Name,
Name: testPod.Name,
},
})
assert.NoError(t, err)
requireNEvents(t, recorder, 0)
}

func TestRatioReconciler_Ok(t *testing.T) {
recorder := record.NewFakeRecorder(4)
_, err := prepareRatioTest(t, testRatioCfg{
Expand Down Expand Up @@ -129,11 +152,17 @@ func TestRatioReconciler_RecordFailed(t *testing.T) {
},
})
assert.NoError(t, err)
if !assert.Len(t, recorder.Events, 0) {
requireNEvents(t, recorder, 0)
}

func requireNEvents(t *testing.T, recorder *record.FakeRecorder, n int) {
t.Helper()

if !assert.Len(t, recorder.Events, n) {
for i := 0; i < len(recorder.Events); i++ {
e := <-recorder.Events
t.Log(e)
t.Log("Recorded event: ", <-recorder.Events)
}
t.FailNow()
}
}

Expand All @@ -142,6 +171,7 @@ type testRatioCfg struct {
fetchErr error
fetchCPU resource.Quantity
fetchMemory resource.Quantity
threshold *inf.Dec
obj []client.Object
recorder record.EventRecorder
}
Expand Down Expand Up @@ -175,6 +205,7 @@ func prepareRatioTest(t *testing.T, cfg testRatioCfg) *RatioReconciler {
Limit: &cfg.limit,
},
},
RatioWarnThreshold: cfg.threshold,
}
}

Expand Down
2 changes: 2 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ func registerRatioController(mgr ctrl.Manager, conf Config, orgLabel string) {
Ratio: &ratio.Fetcher{
Client: mgr.GetClient(),
},
RatioWarnThreshold: conf.MemoryPerCoreWarnThreshold,
},
})

Expand All @@ -335,6 +336,7 @@ func registerRatioController(mgr ctrl.Manager, conf Config, orgLabel string) {
Client: mgr.GetClient(),
OrganizationLabel: orgLabel,
},
RatioWarnThreshold: conf.MemoryPerCoreWarnThreshold,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "ratio")
os.Exit(1)
Expand Down
11 changes: 9 additions & 2 deletions ratio/ratio.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,15 @@ func (r Ratio) Ratio() *resource.Quantity {

// Below returns if the memory to CPU ratio of the recorded objects is below the given limit.
// Always returns false if no CPU is requested.
func (r Ratio) Below(limit resource.Quantity) bool {
return r.Ratio() != nil && r.Ratio().Cmp(limit) < 0
// The limit is multiplied by the optional threshold before comparison.
// If threshold is nil, it defaults to 1.
// A threshold of 0.95 would mean that the function returns true if the ratio is below 95% of the limit.
func (r Ratio) Below(limit resource.Quantity, threshold *inf.Dec) bool {
if threshold == nil {
threshold = inf.NewDec(1, 0)
}

return r.Ratio() != nil && r.Ratio().AsDec().Cmp(inf.NewDec(0, 0).Mul(limit.AsDec(), threshold)) < 0
}

// String implements Stringer to print ratio
Expand Down
18 changes: 15 additions & 3 deletions ratio/ratio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"gopkg.in/inf.v0"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -211,6 +212,8 @@ func TestRatio_ratio(t *testing.T) {
memory string
ratio string

threshold *inf.Dec

smallerThan string
largerOrEqualTo string
}{
Expand Down Expand Up @@ -252,6 +255,15 @@ func TestRatio_ratio(t *testing.T) {
memory: "801Mi",
ratio: "401Mi",
},
{
cpu: "1",
memory: "4Gi",
ratio: "4Gi",

threshold: inf.NewDec(95, 2),
largerOrEqualTo: "3.9Gi",
smallerThan: "4.3Gi",
},
}
for _, tc := range tcs {
t.Run(fmt.Sprintf("[%s/%s=%s]", tc.memory, tc.cpu, tc.ratio), func(t *testing.T) {
Expand All @@ -272,10 +284,10 @@ func TestRatio_ratio(t *testing.T) {
assert.Equalf(t, tc.ratio, r.String(), "should pretty print")
}
if tc.smallerThan != "" {
assert.Truef(t, r.Below(resource.MustParse(tc.smallerThan)), "should be smaller than %s", tc.smallerThan)
assert.Truef(t, r.Below(resource.MustParse(tc.smallerThan), tc.threshold), "should be smaller than %s", tc.smallerThan)
}
if tc.largerOrEqualTo != "" {
assert.Falsef(t, r.Below(resource.MustParse(tc.largerOrEqualTo)), "should not be smaller than %s", tc.largerOrEqualTo)
assert.Falsef(t, r.Below(resource.MustParse(tc.largerOrEqualTo), tc.threshold), "should not be smaller than %s", tc.largerOrEqualTo)
}
})
}
Expand Down Expand Up @@ -310,7 +322,7 @@ func FuzzRatio(f *testing.F) {
out := r.Warn(&lim, "")
assert.NotEmpty(t, out)

r.Below(lim)
r.Below(lim, nil)
})
})
}
8 changes: 5 additions & 3 deletions webhooks/ratio_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"
"strings"

"gopkg.in/inf.v0"
admissionv1 "k8s.io/api/admission/v1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand All @@ -30,8 +31,9 @@ type RatioValidator struct {
Decoder admission.Decoder
Client client.Client

Ratio ratioFetcher
RatioLimits limits.Limits
Ratio ratioFetcher
RatioLimits limits.Limits
RatioWarnThreshold *inf.Dec

// DefaultNodeSelector is the default node selector to apply to pods
DefaultNodeSelector map[string]string
Expand Down Expand Up @@ -117,7 +119,7 @@ func (v *RatioValidator) Handle(ctx context.Context, req admission.Request) admi
continue
}

if r.Below(*limit) {
if r.Below(*limit, v.RatioWarnThreshold) {
l.Info("ratio too low", "node_selector", nodeSel, "ratio", r)
warnings = append(warnings, r.Warn(limit, nodeSel))
}
Expand Down
21 changes: 21 additions & 0 deletions webhooks/ratio_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http"
"strings"

"gopkg.in/inf.v0"
admissionv1 "k8s.io/api/admission/v1"
appsv1 "k8s.io/api/apps/v1"
authenticationv1 "k8s.io/api/authentication/v1"
Expand Down Expand Up @@ -41,6 +42,7 @@ func TestRatioValidator_Handle(t *testing.T) {
mangleObject bool
create bool
limits limits.Limits
threshold *inf.Dec
warn bool
fail bool
statusCode int32
Expand Down Expand Up @@ -303,6 +305,24 @@ func TestRatioValidator_Handle(t *testing.T) {
fail: true,
statusCode: http.StatusBadRequest,
},
"Allow_UnfairNamespace_WithThreshold": {
user: "appuio#foo",
namespace: "bar",
resources: []client.Object{
podFromResources("pod1", "foo", podResource{
{cpu: "1", memory: "4Gi"},
}),
podFromResources("pod2", "foo", podResource{
{cpu: "1", memory: "4Gi"},
}),
podFromResources("slightlyunfair", "bar", podResource{
{cpu: "1050m", memory: "4Gi"},
}),
},
limits: limits.Limits{{Limit: requireParseQuantity(t, "4Gi")}},
warn: false,
threshold: inf.NewDec(95, 2),
},
}

for name, tc := range tests {
Expand All @@ -312,6 +332,7 @@ func TestRatioValidator_Handle(t *testing.T) {

v := prepareTest(t, tc.resources...)
v.RatioLimits = tc.limits
v.RatioWarnThreshold = tc.threshold

ar := admission.Request{
AdmissionRequest: admissionv1.AdmissionRequest{
Expand Down

0 comments on commit f953439

Please sign in to comment.