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

Fix sriov functional tests for secure boot env #148

Merged
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
97 changes: 89 additions & 8 deletions test/util/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,46 @@ import (
"fmt"
"io"
"strings"
"time"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"

sriovv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
testclient "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/client"
"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/namespaces"
"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/nodes"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/pod"
)

// EnabledNodes provides info on sriov enabled nodes of the cluster.
type EnabledNodes struct {
Nodes []string
States map[string]sriovv1.SriovNetworkNodeState
Nodes []string
States map[string]sriovv1.SriovNetworkNodeState
IsSecureBootEnabled map[string]bool
}

var (
supportedPFDrivers = []string{"mlx5_core", "i40e", "ixgbe", "ice"}
supportedVFDrivers = []string{"iavf", "vfio-pci", "mlx5_core"}
mlxVendorID = "15b3"
intelVendorID = "8086"
)

// DiscoverSriov retrieves Sriov related information of a given cluster.
func DiscoverSriov(clients *testclient.ClientSet, operatorNamespace string) (*EnabledNodes, error) {
nodeStates, err := clients.SriovNetworkNodeStates(operatorNamespace).List(context.Background(), metav1.ListOptions{})
res := &EnabledNodes{}
res.States = make(map[string]sriovv1.SriovNetworkNodeState)
res.Nodes = make([]string, 0)
if err != nil {
return nil, fmt.Errorf("Failed to retrieve note states %v", err)
}

res := &EnabledNodes{}
res.States = make(map[string]sriovv1.SriovNetworkNodeState)
res.Nodes = make([]string, 0)
res.IsSecureBootEnabled = make(map[string]bool)

ss, err := nodes.MatchingOptionalSelectorState(clients, nodeStates.Items)
if err != nil {
return nil, fmt.Errorf("Failed to find matching node states %v", err)
Expand All @@ -59,6 +69,15 @@ func DiscoverSriov(clients *testclient.ClientSet, operatorNamespace string) (*En
}
}

for _, node := range res.Nodes {
isSecureBootEnabled, err := GetNodeSecureBootState(clients, node)
if err != nil {
return nil, err
}

res.IsSecureBootEnabled[node] = isSecureBootEnabled
}

if len(res.Nodes) == 0 {
return nil, fmt.Errorf("No sriov enabled node found")
}
Expand All @@ -73,6 +92,13 @@ func (n *EnabledNodes) FindOneSriovDevice(node string) (*sriovv1.InterfaceExt, e
}
for _, itf := range s.Status.Interfaces {
if IsPFDriverSupported(itf.Driver) && sriovv1.IsSupportedDevice(itf.DeviceID) {

// Skip mlx interfaces if secure boot is enabled
// TODO: remove this when mlx support secure boot/lockdown mode
if itf.Vendor == mlxVendorID && n.IsSecureBootEnabled[node] {
continue
}

return &itf, nil
}
}
Expand All @@ -89,6 +115,12 @@ func (n *EnabledNodes) FindSriovDevices(node string) ([]*sriovv1.InterfaceExt, e

for i, itf := range s.Status.Interfaces {
if IsPFDriverSupported(itf.Driver) && sriovv1.IsSupportedDevice(itf.DeviceID) {
// Skip mlx interfaces if secure boot is enabled
// TODO: remove this when mlx support secure boot/lockdown mode
if itf.Vendor == mlxVendorID && n.IsSecureBootEnabled[node] {
continue
}

devices = append(devices, &s.Status.Interfaces[i])
}
}
Expand All @@ -99,7 +131,7 @@ func (n *EnabledNodes) FindSriovDevices(node string) ([]*sriovv1.InterfaceExt, e
func (n *EnabledNodes) FindOneVfioSriovDevice() (string, sriovv1.InterfaceExt) {
for _, node := range n.Nodes {
for _, nic := range n.States[node].Status.Interfaces {
if nic.Vendor == "8086" {
if nic.Vendor == intelVendorID {
return node, nic
}
}
Expand All @@ -113,8 +145,15 @@ func (n *EnabledNodes) FindOneMellanoxSriovDevice(node string) (*sriovv1.Interfa
if !ok {
return nil, fmt.Errorf("Node %s not found", node)
}

// return error here as mlx interfaces are not supported when secure boot is enabled
// TODO: remove this when mlx support secure boot/lockdown mode
if n.IsSecureBootEnabled[node] {
return nil, fmt.Errorf("Secure boot is enabled on the node mellanox cards are not supported")
}

for _, itf := range s.Status.Interfaces {
if itf.Driver == "mlx5_core" {
if itf.Vendor == mlxVendorID {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To support newer card versions that may use a different driver.

Better to go with the vendor ID

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

return &itf, nil
}
}
Expand Down Expand Up @@ -223,3 +262,45 @@ func SetDisableNodeDrainState(clients *testclient.ClientSet, operatorNamespace s
}
return nil
}

func GetNodeSecureBootState(clients *testclient.ClientSet, nodeName string) (bool, error) {
podDefinition := pod.GetDefinition()
podDefinition = pod.RedefineWithNodeSelector(podDefinition, nodeName)
podDefinition = pod.RedefineAsPrivileged(podDefinition)

volume := corev1.Volume{Name: "host", VolumeSource: corev1.VolumeSource{HostPath: &corev1.HostPathVolumeSource{Path: "/"}}}
mount := corev1.VolumeMount{Name: "host", MountPath: "/host"}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the sr-iov daemons already mount /host, would it be better to leverage those?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that they are doing chroot /host as part of the code so we can have some flakes if when we exec into the pod it was inside a chroot in another process.

I this is just safer and we are already starting pods as part of the tests so it should not be critical

podDefinition = pod.RedefineWithMount(podDefinition, volume, mount)
created, err := clients.Pods(namespaces.Test).Create(context.Background(), podDefinition, metav1.CreateOptions{})
if err != nil {
return false, err
}

var runningPod *corev1.Pod
err = wait.PollImmediate(time.Second, 3*time.Minute, func() (bool, error) {
runningPod, err = clients.Pods(namespaces.Test).Get(context.Background(), created.Name, metav1.GetOptions{})
if err != nil {
return false, err
}

if runningPod.Status.Phase != corev1.PodRunning {
return false, nil
}

return true, nil
})
if err != nil {
return false, err
}

stdout, stderr, err := pod.ExecCommand(clients, runningPod, "cat", "/host/sys/kernel/security/lockdown")
if err != nil {
return false, err
}

if stderr != "" {
return false, fmt.Errorf("command return non 0 code: %s", stderr)
}

return strings.Contains(stdout, "[integrity]") || strings.Contains(stdout, "[confidentiality]"), nil
}
20 changes: 14 additions & 6 deletions test/util/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (

const hostnameLabel = "kubernetes.io/hostname"

func getDefinition() *corev1.Pod {
func GetDefinition() *corev1.Pod {
podObject := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "testpod-",
Expand All @@ -36,14 +36,14 @@ func getDefinition() *corev1.Pod {
}

func DefineWithNetworks(networks []string) *corev1.Pod {
podObject := getDefinition()
podObject := GetDefinition()
podObject.Annotations = map[string]string{"k8s.v1.cni.cncf.io/networks": strings.Join(networks, ",")}

return podObject
}

func DefineWithHostNetwork(nodeName string) *corev1.Pod {
podObject := getDefinition()
podObject := GetDefinition()
podObject.Spec.HostNetwork = true
podObject.Spec.NodeSelector = map[string]string{
"kubernetes.io/hostname": nodeName,
Expand All @@ -52,28 +52,36 @@ func DefineWithHostNetwork(nodeName string) *corev1.Pod {
return podObject
}

// RedefineAsPrivileged uppdates the pod to be privileged
// RedefineAsPrivileged updates the pod to be privileged
func RedefineAsPrivileged(pod *corev1.Pod) *corev1.Pod {
pod.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{}
b := true
pod.Spec.Containers[0].SecurityContext.Privileged = &b
return pod
}

// RedefineWithHostNetwork uppdates the pod definition Spec.HostNetwork to true
// RedefineWithHostNetwork updates the pod definition Spec.HostNetwork to true
func RedefineWithHostNetwork(pod *corev1.Pod) *corev1.Pod {
pod.Spec.HostNetwork = true
return pod
}

// RedefineWithNodeSelector uppdates the pod definition with a node selector
// RedefineWithNodeSelector updates the pod definition with a node selector
func RedefineWithNodeSelector(pod *corev1.Pod, node string) *corev1.Pod {
pod.Spec.NodeSelector = map[string]string{
hostnameLabel: node,
}
return pod
}

// RedefineWithMount updates the pod definition with a volume and volume mount
func RedefineWithMount(pod *corev1.Pod, volume corev1.Volume, mount corev1.VolumeMount) *corev1.Pod {
pod.Spec.Containers[0].VolumeMounts = []corev1.VolumeMount{mount}
pod.Spec.Volumes = []corev1.Volume{volume}

return pod
}

// RedefineWithCommand updates the pod defintion with a different command
func RedefineWithCommand(pod *corev1.Pod, command []string, args []string) *corev1.Pod {
pod.Spec.Containers[0].Command = command
Expand Down