diff --git a/go.mod b/go.mod index 6c76acf6b1..af4d2e14d9 100644 --- a/go.mod +++ b/go.mod @@ -46,7 +46,7 @@ require ( sigs.k8s.io/controller-runtime v0.13.0 sigs.k8s.io/yaml v1.4.0 stathat.com/c/consistent v1.0.0 - volcano.sh/apis v1.10.0-alpha.0.0.20241016111016-bb93758bd51f + volcano.sh/apis v1.10.0-alpha.0.0.20241218081838-e5d361b6bfbe ) require ( diff --git a/go.sum b/go.sum index b8b18be173..7791857a2e 100644 --- a/go.sum +++ b/go.sum @@ -510,5 +510,5 @@ sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E= sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY= stathat.com/c/consistent v1.0.0 h1:ezyc51EGcRPJUxfHGSgJjWzJdj3NiMU9pNfLNGiXV0c= stathat.com/c/consistent v1.0.0/go.mod h1:QkzMWzcbB+yQBL2AttO6sgsQS/JSTapcDISJalmCDS0= -volcano.sh/apis v1.10.0-alpha.0.0.20241016111016-bb93758bd51f h1:wqvGQgzYCPJSS07xE1LZbJ/Mxb1f/xFWThnII6BzMhg= -volcano.sh/apis v1.10.0-alpha.0.0.20241016111016-bb93758bd51f/go.mod h1:XHIjTlHDMZTLRg2Y2JAkj85iP0iiet2tv+HfPQZrsHs= +volcano.sh/apis v1.10.0-alpha.0.0.20241218081838-e5d361b6bfbe h1:iHd1Xt36a7S47IFksuF0h9W9J4LKzhBEz0C9XbkBvB8= +volcano.sh/apis v1.10.0-alpha.0.0.20241218081838-e5d361b6bfbe/go.mod h1:XHIjTlHDMZTLRg2Y2JAkj85iP0iiet2tv+HfPQZrsHs= diff --git a/pkg/webhooks/admission/hypernodes/validate/admit_hypernode.go b/pkg/webhooks/admission/hypernodes/validate/admit_hypernode.go new file mode 100644 index 0000000000..d320a657c3 --- /dev/null +++ b/pkg/webhooks/admission/hypernodes/validate/admit_hypernode.go @@ -0,0 +1,188 @@ +/* +Copyright 2024 The Volcano Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validate + +import ( + "context" + "fmt" + "strings" + + admissionv1 "k8s.io/api/admission/v1" + whv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2" + + hypernodev1alpha1 "volcano.sh/apis/pkg/apis/topology/v1alpha1" + "volcano.sh/volcano/pkg/webhooks/router" + "volcano.sh/volcano/pkg/webhooks/schema" + "volcano.sh/volcano/pkg/webhooks/util" +) + +const ( + HyperNodeLabel = "volcano.sh/hypernodes" +) + +var config = &router.AdmissionServiceConfig{} + +var service = &router.AdmissionService{ + Path: "/hypernodes/validate", + Func: AdmitHyperNode, + + Config: config, + + ValidatingConfig: &whv1.ValidatingWebhookConfiguration{ + Webhooks: []whv1.ValidatingWebhook{{ + Name: "validatehypernode.volcano.sh", + Rules: []whv1.RuleWithOperations{ + { + Operations: []whv1.OperationType{whv1.Create, whv1.Update}, + Rule: whv1.Rule{ + APIGroups: []string{hypernodev1alpha1.SchemeGroupVersion.Group}, + APIVersions: []string{hypernodev1alpha1.SchemeGroupVersion.Version}, + Resources: []string{"hypernodes"}, + }, + }, + }, + }}, + }, +} + +func init() { + router.RegisterAdmission(service) +} + +// AdmitHyperNode is to admit hypernode and return response. +// Reference: https://github.com/volcano-sh/volcano/issues/3883 +func AdmitHyperNode(ar admissionv1.AdmissionReview) *admissionv1.AdmissionResponse { + klog.V(3).Infof("admitting hypernode -- %s", ar.Request.Operation) + + hypernode, err := schema.DecodeHyperNode(ar.Request.Object, ar.Request.Resource) + if err != nil { + return util.ToAdmissionResponse(err) + } + + switch ar.Request.Operation { + case admissionv1.Create: + err = validateHyperNodeCreate(hypernode) + if err != nil { + return util.ToAdmissionResponse(err) + } + + case admissionv1.Update: + oldHyperNode, err := schema.DecodeHyperNode(ar.Request.OldObject, ar.Request.Resource) + if err != nil { + return util.ToAdmissionResponse(err) + } + err = validateHyperNodeUpdate(oldHyperNode, hypernode) + if err != nil { + return util.ToAdmissionResponse(err) + } + } + + return &admissionv1.AdmissionResponse{ + Allowed: true, + } +} + +// validateHyperNodeCreate is to validate hypernode create +func validateHyperNodeCreate(hypernode *hypernodev1alpha1.HyperNode) error { + if err := validateHyperNodeMembers(hypernode); err != nil { + return err + } + + if hypernode.Labels != nil && hypernode.Labels[HyperNodeLabel] != "" { + hypernodeList := strings.Split(hypernode.Labels[HyperNodeLabel], ",") + + for _, hypernodeName := range hypernodeList { + if !strings.HasPrefix(hypernodeName, "hypernode-") { + return fmt.Errorf("the label %s must be like `hypernode-0,hypernode-1,...,hypernode-n`", HyperNodeLabel) + } + + if _, err := config.VolcanoClient.TopologyV1alpha1().HyperNodes().Get(context.TODO(), hypernodeName, metav1.GetOptions{}); err != nil { + return fmt.Errorf("failed to get hypernode %s: %v", hypernodeName, err) + } + } + } + + return nil +} + +// validateHyperNodeUpdate is to validate hypernode update +func validateHyperNodeUpdate(oldHyperNode, hypernode *hypernodev1alpha1.HyperNode) error { + if err := validateHyperNodeMembers(hypernode); err != nil { + return err + } + + var oldHyperNodeList []string + var newHyperNodeList []string + + if oldHyperNode.Labels != nil && oldHyperNode.Labels[HyperNodeLabel] != "" { + oldHyperNodeList = strings.Split(oldHyperNode.Labels[HyperNodeLabel], ",") + } + + if hypernode.Labels != nil && hypernode.Labels[HyperNodeLabel] != "" { + newHyperNodeList = strings.Split(hypernode.Labels[HyperNodeLabel], ",") + } + + // set hypernode list to empty is ok + if len(newHyperNodeList) == 0 { + return nil + } + + // change hypernode list length is not allowed + if len(newHyperNodeList) != len(oldHyperNodeList) { + return fmt.Errorf("change hypernode list length is not allowed") + } + + // get changed hypernode and validate + for i := range newHyperNodeList { + if newHyperNodeList[i] != oldHyperNodeList[i] { + if !strings.HasPrefix(newHyperNodeList[i], "hypernode-") { + return fmt.Errorf("the label %s must be like `hypernode-0,hypernode-1,...,hypernode-n`", HyperNodeLabel) + } + + hypernode, err := config.VolcanoClient.TopologyV1alpha1().HyperNodes().Get(context.TODO(), newHyperNodeList[i], metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to get hypernode %s: %v", newHyperNodeList[i], err) + } + + if hypernode.Spec.Tier != "1" { + return fmt.Errorf("changed hypernode %s tier is not tier 1 is not allowed", newHyperNodeList[i]) + } + } + } + + return nil +} + +// validateHyperNodeMembers is to validate hypernode members +func validateHyperNodeMembers(hypernode *hypernodev1alpha1.HyperNode) error { + if len(hypernode.Spec.Members) == 0 { + return nil + } + + for _, member := range hypernode.Spec.Members { + if member.Selector == (hypernodev1alpha1.MemberSelector{}) { + continue + } + + if member.Selector.ExactMatch != nil && member.Selector.RegexMatch != nil { + return fmt.Errorf("exactMatch and regexMatch cannot be specified together") + } + } + return nil +} diff --git a/pkg/webhooks/admission/hypernodes/validate/admit_hypernode_test.go b/pkg/webhooks/admission/hypernodes/validate/admit_hypernode_test.go new file mode 100644 index 0000000000..5c50ef20a5 --- /dev/null +++ b/pkg/webhooks/admission/hypernodes/validate/admit_hypernode_test.go @@ -0,0 +1,313 @@ +/* +Copyright 2024 The Volcano Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validate + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + hypernodev1alpha1 "volcano.sh/apis/pkg/apis/topology/v1alpha1" + fakeclient "volcano.sh/apis/pkg/client/clientset/versioned/fake" +) + +func TestValidateHyperNodeMembers(t *testing.T) { + testCases := []struct { + Name string + HyperNode *hypernodev1alpha1.HyperNode + ExpectErr string + }{ + { + Name: "validate valid hypernode", + HyperNode: &hypernodev1alpha1.HyperNode{ + Spec: hypernodev1alpha1.HyperNodeSpec{ + Members: []hypernodev1alpha1.MemberSpec{ + { + Selector: hypernodev1alpha1.MemberSelector{ + ExactMatch: &hypernodev1alpha1.ExactMatch{ + Name: "node-1", + }, + }, + }, + }, + }, + }, + ExpectErr: "", + }, + { + Name: "validate valid hypernode with empty selector", + HyperNode: &hypernodev1alpha1.HyperNode{ + Spec: hypernodev1alpha1.HyperNodeSpec{ + Members: []hypernodev1alpha1.MemberSpec{ + { + Selector: hypernodev1alpha1.MemberSelector{}, + }, + }, + }, + }, + ExpectErr: "", + }, + { + Name: "validate invalid hypernode with both exactMatch and regexMatch", + HyperNode: &hypernodev1alpha1.HyperNode{ + Spec: hypernodev1alpha1.HyperNodeSpec{ + Members: []hypernodev1alpha1.MemberSpec{ + { + Selector: hypernodev1alpha1.MemberSelector{ + ExactMatch: &hypernodev1alpha1.ExactMatch{ + Name: "node-1", + }, + RegexMatch: &hypernodev1alpha1.RegexMatch{ + Pattern: "node-1", + }, + }, + }, + }, + }, + }, + ExpectErr: "exactMatch and regexMatch cannot be specified together", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + err := validateHyperNodeMembers(testCase.HyperNode) + if err != nil && err.Error() != testCase.ExpectErr { + t.Errorf("validateHyperNodeLabels failed: %v", err) + } + }) + } + +} + +// TestValidateHyperNodeLabels tests the validateHyperNodeLabels function +func TestValidateCreatedHyperNodeLabels(t *testing.T) { + testCases := []struct { + Name string + HyperNode *hypernodev1alpha1.HyperNode + ExpectErr string + }{ + { + Name: "validate valid hypernode labels", + HyperNode: &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "volcano.sh/hypernodes": "hypernode-0,hypernode-1", + }, + }, + }, + ExpectErr: "", + }, + { + Name: "validate invalid hypernode labels with name of non-hypernode", + HyperNode: &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "volcano.sh/hypernodes": "non-hypernode-0,non-hypernode-1", + }, + }, + }, + ExpectErr: "the label volcano.sh/hypernodes must be like `hypernode-0,hypernode-1,...,hypernode-n`", + }, + { + Name: "validate invalid hypernode labels with not exist hypernode", + HyperNode: &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "volcano.sh/hypernodes": "hypernode-3,hypernode-4", + }, + }, + }, + ExpectErr: "failed to get hypernode hypernode-3: hypernodes.topology.volcano.sh \"hypernode-3\" not found", + }, + } + + // create hyper node for test + hypernodeList := &hypernodev1alpha1.HyperNodeList{ + Items: []hypernodev1alpha1.HyperNode{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-0", + }, + Spec: hypernodev1alpha1.HyperNodeSpec{ + Tier: "1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-1", + }, + Spec: hypernodev1alpha1.HyperNodeSpec{ + Tier: "2", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-2", + }, + Spec: hypernodev1alpha1.HyperNodeSpec{ + Tier: "3", + }, + }, + }, + } + + client := fakeclient.NewSimpleClientset(hypernodeList) + config.VolcanoClient = client + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + err := validateHyperNodeCreate(testCase.HyperNode) + if err != nil && err.Error() != testCase.ExpectErr { + t.Errorf("validateHyperNodeLabels failed: %v", err) + } + }) + } +} + +func TestValidateUpdateHyperNodeLabels(t *testing.T) { + testCases := []struct { + Name string + HyperNode *hypernodev1alpha1.HyperNode + ExpectErr string + }{ + { + Name: "validate valid hypernode labels", + HyperNode: &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-0", + Labels: map[string]string{ + "volcano.sh/hypernodes": "hypernode-0-new,hypernode-1", + }, + }, + }, + ExpectErr: "", + }, + { + Name: "validate hypernode labels with set to empty", + HyperNode: &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-0", + Labels: map[string]string{ + "volcano.sh/hypernodes": "", + }, + }, + }, + ExpectErr: "", + }, + { + Name: "validate invalid hypernode selector with delete one hypernode", + HyperNode: &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-0", + Labels: map[string]string{ + "volcano.sh/hypernodes": "hypernode-1", + }, + }, + }, + ExpectErr: "change hypernode list length is not allowed", + }, + { + Name: "validate invalid hypernode selector with add one hypernode", + HyperNode: &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-0", + Labels: map[string]string{ + "volcano.sh/hypernodes": "hypernode-0,hypernode-1,hypernode-2", + }, + }, + }, + ExpectErr: "change hypernode list length is not allowed", + }, + { + Name: "validate invalid hypernode selector change hypernode not tier 1", + HyperNode: &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-0", + Labels: map[string]string{ + "volcano.sh/hypernodes": "hypernode-0,hypernode-3", + }, + }, + }, + ExpectErr: "changed hypernode hypernode-3 tier is not tier 1 is not allowed", + }, + { + Name: "validate invalid hypernode selector change hypernode with not exist hypernode", + HyperNode: &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-0", + Labels: map[string]string{ + "volcano.sh/hypernodes": "hypernode-0,hypernode-2", + }, + }, + }, + ExpectErr: "failed to get hypernode hypernode-2: hypernodes.topology.volcano.sh \"hypernode-2\" not found", + }, + } + + // create hyper node for test + oldHyperNode := &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-0", + Labels: map[string]string{ + "volcano.sh/hypernodes": "hypernode-0,hypernode-1", + }, + }, + Spec: hypernodev1alpha1.HyperNodeSpec{ + Tier: "1", + }, + } + + hyperNode1 := &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-1", + }, + Spec: hypernodev1alpha1.HyperNodeSpec{ + Tier: "2", + }, + } + + hyperNodeNew := &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-0-new", + }, + Spec: hypernodev1alpha1.HyperNodeSpec{ + Tier: "1", + }, + } + + hyperNode3 := &hypernodev1alpha1.HyperNode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hypernode-3", + }, + Spec: hypernodev1alpha1.HyperNodeSpec{ + Tier: "3", + }, + } + + client := fakeclient.NewSimpleClientset(oldHyperNode, hyperNode1, hyperNodeNew, hyperNode3) + config.VolcanoClient = client + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + err := validateHyperNodeUpdate(oldHyperNode, testCase.HyperNode) + if err != nil && err.Error() != testCase.ExpectErr { + t.Errorf("validateHyperNodeLabels failed: %v", err) + } + }) + } +} diff --git a/pkg/webhooks/schema/schema.go b/pkg/webhooks/schema/schema.go index 8c1427d74c..47e8d05cdc 100644 --- a/pkg/webhooks/schema/schema.go +++ b/pkg/webhooks/schema/schema.go @@ -30,6 +30,7 @@ import ( batchv1alpha1 "volcano.sh/apis/pkg/apis/batch/v1alpha1" schedulingv1beta1 "volcano.sh/apis/pkg/apis/scheduling/v1beta1" + hypernodev1alpha1 "volcano.sh/apis/pkg/apis/topology/v1alpha1" ) func init() { @@ -127,3 +128,24 @@ func DecodePodGroup(object runtime.RawExtension, resource metav1.GroupVersionRes return &podgroup, nil } + +// DecodeHyperNode decodes the hypernode using deserializer from the raw object. +func DecodeHyperNode(object runtime.RawExtension, resource metav1.GroupVersionResource) (*hypernodev1alpha1.HyperNode, error) { + hypernodeResource := metav1.GroupVersionResource{ + Group: hypernodev1alpha1.SchemeGroupVersion.Group, + Version: hypernodev1alpha1.SchemeGroupVersion.Version, + Resource: "hypernodes", + } + + if resource != hypernodeResource { + klog.Errorf("expect resource to be %s", hypernodeResource) + return nil, fmt.Errorf("expect resource to be %s", hypernodeResource) + } + + hypernode := hypernodev1alpha1.HyperNode{} + if _, _, err := Codecs.UniversalDeserializer().Decode(object.Raw, nil, &hypernode); err != nil { + return nil, err + } + + return &hypernode, nil +}