-
Notifications
You must be signed in to change notification settings - Fork 114
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
Fix sriov functional tests for secure boot env #148
Conversation
for _, itf := range s.Status.Interfaces { | ||
if itf.Driver == "mlx5_core" { | ||
if itf.Vendor == mlxVendorID { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
test/util/cluster/cluster.go
Outdated
} | ||
|
||
var runningPod *corev1.Pod | ||
for retry := 0; retry < 180; retry++ { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use the backoff functions here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep done :)
podDefinition = pod.RedefineAsPrivileged(podDefinition) | ||
|
||
volume := corev1.Volume{Name: "host", VolumeSource: corev1.VolumeSource{HostPath: &corev1.HostPathVolumeSource{Path: "/"}}} | ||
mount := corev1.VolumeMount{Name: "host", MountPath: "/host"} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
This commit fix the functional tests when running on a secure boot environment Signed-off-by: Sebastian Sch <sebassch@gmail.com>
555288a
to
1843e9b
Compare
/lgtm |
@mmirecki please take a look |
This commit fix the functional tests when running on a secure boot environment
Signed-off-by: Sebastian Sch sebassch@gmail.com