diff --git a/pkg/fpgacontroller/patcher/patcher.go b/pkg/fpgacontroller/patcher/patcher.go index ba1d0f51..bbee9254 100644 --- a/pkg/fpgacontroller/patcher/patcher.go +++ b/pkg/fpgacontroller/patcher/patcher.go @@ -133,14 +133,17 @@ func (p *patcher) RemoveRegion(name string) { delete(p.resourceModeMap, namespace+"/"+name) } -// getRequestedResources validates the container's requirements first, then returns them as a map. -func getRequestedResources(container corev1.Container) (map[string]int64, error) { +func validateContainer(container corev1.Container) error { for _, v := range container.Env { if strings.HasPrefix(v.Name, "FPGA_REGION") || strings.HasPrefix(v.Name, "FPGA_AFU") { - return nil, errors.Errorf("environment variable '%s' is not allowed", v.Name) + return errors.Errorf("environment variable '%s' is not allowed", v.Name) } } + return nil +} +// getRequestedResources validates the container's requirements first, then returns them as a map. +func getRequestedResources(container corev1.Container) (map[string]int64, error) { // Container may happen to have Requests, but not Limits. Check Requests first, // then in the next loop iterate over Limits. for resourceName, resourceQuantity := range container.Resources.Requests { @@ -183,6 +186,10 @@ func getRequestedResources(container corev1.Container) (map[string]int64, error) } func (p *patcher) getPatchOps(containerIdx int, container corev1.Container) ([]string, error) { + if err := validateContainer(container); err != nil { + return nil, err + } + requestedResources, err := getRequestedResources(container) if err != nil { return nil, err @@ -203,14 +210,11 @@ func (p *patcher) getPatchOps(containerIdx int, container corev1.Container) ([]s } switch mode { - case regiondevel: + case regiondevel, af: // Do nothing. - // The requested resources are exposed by FPGA plugins working in "regiondevel" mode. - // In this mode the workload is supposed to program FPGA regions. + // The requested resources are exposed by FPGA plugins working in "regiondevel/af" mode. + // In "regiondevel" mode the workload is supposed to program FPGA regions. // A cluster admin has to add FpgaRegion CRDs to allow this. - case af: - // Do nothing. - // The requested resources are exposed by FPGA plugins working in "af" mode. case region: // Let fpga_crihook know how to program the regions by setting ENV variables. // The requested resources are exposed by FPGA plugins working in "region" mode. diff --git a/pkg/fpgacontroller/patcher/patcher_test.go b/pkg/fpgacontroller/patcher/patcher_test.go index 41b96c5a..bbc1c725 100644 --- a/pkg/fpgacontroller/patcher/patcher_test.go +++ b/pkg/fpgacontroller/patcher/patcher_test.go @@ -86,6 +86,71 @@ func TestPatcherStorageFunctions(t *testing.T) { } } +func TestValidateContainerEnv(t *testing.T) { + tcases := []struct { + name string + container corev1.Container + expectedErr bool + }{ + { + name: "Container OK", + container: corev1.Container{ + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "cpu": resource.MustParse("1"), + }, + }, + }, + expectedErr: false, + }, + { + name: "Wrong ENV FPGA_AFU", + container: corev1.Container{ + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "cpu": resource.MustParse("1"), + }, + }, + Env: []corev1.EnvVar{ + { + Name: "FPGA_AFU", + Value: "fake value", + }, + }, + }, + expectedErr: true, + }, + { + name: "Wrong ENV FPGA_REGION", + container: corev1.Container{ + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "cpu": resource.MustParse("1"), + }, + }, + Env: []corev1.EnvVar{ + { + Name: "FPGA_REGION", + Value: "fake value", + }, + }, + }, + expectedErr: true, + }, + } + for _, tt := range tcases { + t.Run(tt.name, func(t *testing.T) { + err := validateContainer(tt.container) + if tt.expectedErr && err == nil { + t.Errorf("Test case '%s': no error returned", tt.name) + } + if !tt.expectedErr && err != nil { + t.Errorf("Test case '%s': unexpected error: %+v", tt.name, err) + } + }) + } +} + func TestGetPatchOps(t *testing.T) { tcases := []struct { name string @@ -229,23 +294,6 @@ func TestGetPatchOps(t *testing.T) { }, expectedErr: true, }, - { - name: "Wrong ENV", - container: corev1.Container{ - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "cpu": resource.MustParse("1"), - }, - }, - Env: []corev1.EnvVar{ - { - Name: "FPGA_REGION", - Value: "fake value", - }, - }, - }, - expectedErr: true, - }, { name: "Wrong type of quantity", container: corev1.Container{