patcher: move ENV validation away from getRequestedResources

This commit adds new function validateContainer() that runs the
same FPGA_* ENV validation checks as before in getRequestedResources().

The restructuring is done in preparations for moving
getRequestedResources() to a separate package.

Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
This commit is contained in:
Mikko Ylinen 2020-09-03 10:55:20 +03:00
parent 67c4ca0c5a
commit f4c33d198e
2 changed files with 78 additions and 26 deletions

View File

@ -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.

View File

@ -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{