improve elfmachine webhook

smartxworks · Nov 19, 2024 · 63a7143 · 63a7143
1 parent 18c182e
commit 63a7143
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 4 deletions.
diff --git a/webhooks/elfmachine_webhook_mutation.go b/webhooks/elfmachine_webhook_mutation.go
@@ -29,6 +29,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	infrav1 "github.com/smartxworks/cluster-api-provider-elf/api/v1beta1"
+	annotationsutil "github.com/smartxworks/cluster-api-provider-elf/pkg/util/annotations"
 	"github.com/smartxworks/cluster-api-provider-elf/pkg/version"
 )
 
@@ -64,7 +65,22 @@ func (m *ElfMachineMutation) Handle(ctx goctx.Context, request admission.Request
 	}
 
 	if elfMachine.Spec.NumCoresPerSocket <= 0 {
-		elfMachine.Spec.NumCoresPerSocket = elfMachine.Spec.NumCPUs
+		// Prefer to set the value to be the same as elfMachineTemplate
+		elfMachineTemplateName := annotationsutil.GetTemplateClonedFromName(&elfMachine)
+		if elfMachineTemplateName != "" {
+			var elfMachineTemplate infrav1.ElfMachineTemplate
+			if err := m.Get(ctx, client.ObjectKey{
+				Namespace: elfMachine.Namespace,
+				Name:      annotationsutil.GetTemplateClonedFromName(&elfMachine),
+			}, &elfMachineTemplate); err != nil {
+				return admission.Errored(http.StatusBadRequest, err)
+			}
+			elfMachine.Spec.NumCoresPerSocket = elfMachineTemplate.Spec.Template.Spec.NumCoresPerSocket
+		}
+		// If elfMachineTemplate also has no value, set it to be the same as NumCPUs
+		if elfMachine.Spec.NumCoresPerSocket <= 0 {
+			elfMachine.Spec.NumCoresPerSocket = elfMachine.Spec.NumCPUs
+		}
 	}
 
 	if marshaledElfMachine, err := json.Marshal(elfMachine); err != nil {

diff --git a/webhooks/elfmachine_webhook_mutation_test.go b/webhooks/elfmachine_webhook_mutation_test.go
@@ -26,10 +26,13 @@ import (
 	admissionv1 "k8s.io/api/admission/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	infrav1 "github.com/smartxworks/cluster-api-provider-elf/api/v1beta1"
+	"github.com/smartxworks/cluster-api-provider-elf/pkg/util/annotations"
 	"github.com/smartxworks/cluster-api-provider-elf/pkg/version"
 	"github.com/smartxworks/cluster-api-provider-elf/test/fake"
 )
@@ -46,6 +49,7 @@ var (
 
 func TestElfMachineMutation(t *testing.T) {
 	g := NewWithT(t)
+	scheme := newScheme(g)
 	tests := []testCase{}
 
 	elfMachine := fake.NewElfMachine(nil)
@@ -54,7 +58,7 @@ func TestElfMachineMutation(t *testing.T) {
 	raw, err := marshal(elfMachine)
 	g.Expect(err).NotTo(HaveOccurred())
 	tests = append(tests, testCase{
-		name: "should set CAPE version",
+		name: "should set CAPE version and numCoresPerSocket",
 		admissionRequest: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{
 			Kind:      metav1.GroupVersionKind{Group: infrav1.GroupVersion.Group, Version: infrav1.GroupVersion.Version, Kind: "ElfMachine"},
 			Operation: admissionv1.Create,
@@ -67,9 +71,30 @@ func TestElfMachineMutation(t *testing.T) {
 		},
 	})
 
+	elfMachineTemplate := fake.NewElfMachineTemplate()
+	elfMachineTemplate.Spec.Template.Spec.NumCoresPerSocket = 2
+	elfMachine = fake.NewElfMachine(nil)
+	annotations.AddAnnotations(elfMachine, map[string]string{clusterv1.TemplateClonedFromNameAnnotation: elfMachineTemplate.Name})
+	elfMachine.Spec.NumCoresPerSocket = 0
+	raw, err = marshal(elfMachine)
+	g.Expect(err).NotTo(HaveOccurred())
+	tests = append(tests, testCase{
+		name: "should set NumCoresPerSocket from elfMachineTemplate",
+		admissionRequest: admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{
+			Kind:      metav1.GroupVersionKind{Group: infrav1.GroupVersion.Group, Version: infrav1.GroupVersion.Version, Kind: "ElfMachine"},
+			Operation: admissionv1.Update,
+			Object:    runtime.RawExtension{Raw: raw},
+		}},
+		expectRespAllowed: true,
+		expectPatchs: []jsonpatch.Operation{
+			{Operation: "add", Path: "/spec/numCoresPerSocket", Value: float64(elfMachineTemplate.Spec.Template.Spec.NumCoresPerSocket)},
+		},
+		client: fakeclient.NewClientBuilder().WithScheme(scheme).WithObjects(elfMachineTemplate).Build(),
+	})
+
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			mutation := ElfMachineMutation{}
+			mutation := ElfMachineMutation{Client: tc.client}
 			mutation.InjectDecoder(admission.NewDecoder(scheme))
 
 			resp := mutation.Handle(context.Background(), tc.admissionRequest)
@@ -92,4 +117,5 @@ type testCase struct {
 	admissionRequest  admission.Request
 	expectRespAllowed bool
 	expectPatchs      []jsonpatch.Operation
+	client            client.Client
 }
diff --git a/webhooks/elfmachine_webhook_validation.go b/webhooks/elfmachine_webhook_validation.go
@@ -94,7 +94,8 @@ func (v *ElfMachineValidator) ValidateUpdate(ctx goctx.Context, oldObj, newObj r
 		if elfMachine.Spec.NumCPUs != elfMachineTemplate.Spec.Template.Spec.NumCPUs {
 			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "numCPUs"), elfMachine.Spec.NumCPUs, fmt.Sprintf(canOnlyModifiedThroughElfMachineTemplate, elfMachineTemplateName)))
 		}
-		if elfMachine.Spec.NumCoresPerSocket != elfMachineTemplate.Spec.Template.Spec.NumCoresPerSocket {
+		if elfMachine.Spec.NumCoresPerSocket != 0 && elfMachineTemplate.Spec.Template.Spec.NumCoresPerSocket != 0 &&
+			elfMachine.Spec.NumCoresPerSocket != elfMachineTemplate.Spec.Template.Spec.NumCoresPerSocket {
 			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "numCoresPerSocket"), elfMachine.Spec.NumCoresPerSocket, fmt.Sprintf(canOnlyModifiedThroughElfMachineTemplate, elfMachineTemplateName)))
 		}
 		if elfMachine.Spec.MemoryMiB != elfMachineTemplate.Spec.Template.Spec.MemoryMiB {