Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a threshold for the ratio warning to not notify on small violations #109

Merged
merged 1 commit into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading