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

webhook: validate network qos config in webhook #1841

Merged
merged 1 commit into from
Jan 26, 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
125 changes: 124 additions & 1 deletion pkg/webhook/cm/plugins/sloconfig/resource_qos_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,22 @@ package sloconfig
import (
"encoding/json"
"fmt"
"strconv"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/storage"
"k8s.io/klog/v2"

"github.com/koordinator-sh/koordinator/apis/configuration"
"github.com/koordinator-sh/koordinator/apis/slo/v1alpha1"
)

const (
// InvalidPercentageValueMsg is an error message for value must in percentage range.
InvalidPercentageValueMsg string = `must be percentage value just in 0-100'`
)

var _ ConfigChecker = &ResourceQOSChecker{}
Expand All @@ -47,7 +58,119 @@ func NewResourceQOSChecker(oldConfig, newConfig *corev1.ConfigMap, needUnmarshal
}

func (c *ResourceQOSChecker) ConfigParamValid() error {
return c.CheckByValidator(c.cfg)
if err := c.CheckByValidator(c.cfg); err != nil {
return err
}

return CheckNetQosCfg(c.cfg, field.NewPath("ResourceQOSCfg"))
}

func CheckNetQosCfg(cfg *configuration.ResourceQOSCfg, fldPath *field.Path) error {
if cfg == nil {
return nil
}

// check netqos config is valid for cluster strategy
if cfg.ClusterStrategy != nil {
if cfg.ClusterStrategy.BEClass != nil {
if err := CheckSubNetQos(fldPath.Child("ClusterStrategy").Child("BEClass"), cfg.ClusterStrategy.BEClass.NetworkQOS); err != nil {
return err
}
}

if cfg.ClusterStrategy.LSClass != nil {
if err := CheckSubNetQos(fldPath.Child("ClusterStrategy").Child("LSClass"), cfg.ClusterStrategy.LSClass.NetworkQOS); err != nil {
return err
}
}

if cfg.ClusterStrategy.LSRClass != nil {
if err := CheckSubNetQos(fldPath.Child("ClusterStrategy").Child("LSRClass"), cfg.ClusterStrategy.LSRClass.NetworkQOS); err != nil {
return err
}
}
}

// check netqos config is valid for each node strategy
for idx, nodeStrategy := range cfg.NodeStrategies {
if nodeStrategy.BEClass != nil {
if err := CheckSubNetQos(fldPath.Child("nodeStrategy").Child(strconv.Itoa(idx)).Child("BEClass"), nodeStrategy.BEClass.NetworkQOS); err != nil {
return err
}
}
if nodeStrategy.LSClass != nil {
if err := CheckSubNetQos(fldPath.Child("nodeStrategy").Child(strconv.Itoa(idx)).Child("LSClass"), nodeStrategy.LSClass.NetworkQOS); err != nil {
return err
}
}

if nodeStrategy.LSRClass != nil {
if err := CheckSubNetQos(fldPath.Child("nodeStrategy").Child(strconv.Itoa(idx)).Child("LSRClass"), nodeStrategy.LSRClass.NetworkQOS); err != nil {
return err
}
}
}

return nil
}

func CheckSubNetQos(fldPath *field.Path, qos *v1alpha1.NetworkQOSCfg) error {
if qos == nil {
return nil
}

if errs := ValidatePercentageOrQuantity(qos.IngressRequest, fldPath.Child("IngressRequest")); len(errs) > 0 {
return storage.NewInvalidError(errs)
}
if errs := ValidatePercentageOrQuantity(qos.IngressLimit, fldPath.Child("IngressLimit")); len(errs) > 0 {
return storage.NewInvalidError(errs)
}
if errs := ValidatePercentageOrQuantity(qos.EgressRequest, fldPath.Child("EgressRequest")); len(errs) > 0 {
return storage.NewInvalidError(errs)
}
if errs := ValidatePercentageOrQuantity(qos.EgressLimit, fldPath.Child("EgressLimit")); len(errs) > 0 {
return storage.NewInvalidError(errs)
}

return nil
}

// ValidatePercentageOrQuantity tests if a given value is a valid percentage or
// quantity (defines in: https://github.com/kubernetes/apimachinery/blob/master/pkg/api/resource/quantity.go#L100-L111).
func ValidatePercentageOrQuantity(intOrPercent *intstr.IntOrString, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if intOrPercent == nil {
return allErrs
}

switch intOrPercent.Type {
case intstr.String:
allErrs = append(allErrs, ValidateQuantityField(intOrPercent.StrVal, fldPath)...)
case intstr.Int:
allErrs = append(allErrs, ValidatePercentageField(intOrPercent.IntValue(), fldPath)...)
default:
allErrs = append(allErrs, field.Invalid(fldPath, intOrPercent, "must be an integer just in 0-100 or quantity (e.g '50M')"))
}

return allErrs
}

// ValidateQuantityField validates that given value is quantity format, like 50M.
func ValidateQuantityField(quantityStr string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if _, err := resource.ParseQuantity(quantityStr); err != nil {
eahydra marked this conversation as resolved.
Show resolved Hide resolved
allErrs = append(allErrs, field.Invalid(fldPath, quantityStr, err.Error()))
}
return allErrs
}

// ValidatePercentageField validates that given value is a percentage format just in 0-100.
func ValidatePercentageField(value int, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if value < 0 || value > 100 {
allErrs = append(allErrs, field.Invalid(fldPath, value, InvalidPercentageValueMsg))
}
return allErrs
}

func (c *ResourceQOSChecker) initConfig() error {
Expand Down
113 changes: 113 additions & 0 deletions pkg/webhook/cm/plugins/sloconfig/resource_qos_checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/utils/pointer"

"github.com/koordinator-sh/koordinator/apis/configuration"
Expand Down Expand Up @@ -244,6 +245,16 @@ func Test_ResourceQOS_ConfigContentsValid(t *testing.T) {
cfg configuration.ResourceQOSCfg
}

fromInt := func(in int) *intstr.IntOrString {
res := intstr.FromInt(in)
return &res
}

fromString := func(in string) *intstr.IntOrString {
res := intstr.FromString(in)
return &res
}

tests := []struct {
name string
args args
Expand Down Expand Up @@ -377,6 +388,108 @@ func Test_ResourceQOS_ConfigContentsValid(t *testing.T) {
},
wantErr: false,
},
{
name: "netqos config not valid for percentage less than 0",
args: args{
cfg: configuration.ResourceQOSCfg{
ClusterStrategy: &slov1alpha1.ResourceQOSStrategy{
LSRClass: &slov1alpha1.ResourceQOS{
NetworkQOS: &slov1alpha1.NetworkQOSCfg{
NetworkQOS: slov1alpha1.NetworkQOS{
IngressRequest: fromInt(-1),
},
},
},
},
},
},
wantErr: true,
},
{
name: "netqos config not valid for percentage more than 100",
args: args{
cfg: configuration.ResourceQOSCfg{
ClusterStrategy: &slov1alpha1.ResourceQOSStrategy{
LSRClass: &slov1alpha1.ResourceQOS{
NetworkQOS: &slov1alpha1.NetworkQOSCfg{
NetworkQOS: slov1alpha1.NetworkQOS{
IngressRequest: fromInt(101),
},
},
},
},
},
},
wantErr: true,
},
{
name: "netqos config percentage format is valid",
args: args{
cfg: configuration.ResourceQOSCfg{
ClusterStrategy: &slov1alpha1.ResourceQOSStrategy{
LSRClass: &slov1alpha1.ResourceQOS{
NetworkQOS: &slov1alpha1.NetworkQOSCfg{
NetworkQOS: slov1alpha1.NetworkQOS{
IngressRequest: fromInt(50),
},
},
},
},
},
},
wantErr: false,
},
{
name: "netqos config quantity format invalid",
args: args{
cfg: configuration.ResourceQOSCfg{
ClusterStrategy: &slov1alpha1.ResourceQOSStrategy{
LSRClass: &slov1alpha1.ResourceQOS{
NetworkQOS: &slov1alpha1.NetworkQOSCfg{
NetworkQOS: slov1alpha1.NetworkQOS{
IngressRequest: fromString("50a"),
},
},
},
},
},
},
wantErr: true,
},
{
name: "netqos config quantity format is nil",
args: args{
cfg: configuration.ResourceQOSCfg{
ClusterStrategy: &slov1alpha1.ResourceQOSStrategy{
LSRClass: &slov1alpha1.ResourceQOS{
NetworkQOS: &slov1alpha1.NetworkQOSCfg{
NetworkQOS: slov1alpha1.NetworkQOS{
IngressRequest: fromString(""),
},
},
},
},
},
},
wantErr: true,
},
{
name: "netqos config quantity format valid",
args: args{
cfg: configuration.ResourceQOSCfg{
ClusterStrategy: &slov1alpha1.ResourceQOSStrategy{
LSRClass: &slov1alpha1.ResourceQOS{
NetworkQOS: &slov1alpha1.NetworkQOSCfg{
NetworkQOS: slov1alpha1.NetworkQOS{
IngressRequest: fromString("50m"),
},
},
},
},
},
},
wantErr: false,
},
}

for _, tt := range tests {
Expand Down
Loading