-
Notifications
You must be signed in to change notification settings - Fork 3
traffic-gen: Get pci env var by name #94
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
Conversation
09f43e6
to
bb8230b
Compare
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.
Thank you for the PR @RamLavi.
As we've discussed offline, this implementation will probably not solve BZ 2177668.
The SR-IOV device plugin uses the resourceName when it passes the PCI addresses of the VFs to kubelet as environment variables: https://github.com/k8snetworkplumbingwg/sriov-network-device-plugin#pod-device-information.
We need to use the NetworkAttachmentDefinition's resourceName, instead of its name:
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
annotations:
k8s.v1.cni.cncf.io/resourceName: openshift.io/intel_nics_dpdk
6816925
to
8827f61
Compare
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.
Thank you for the changes @RamLavi.
Could you please update the PR's description with a short explanation of the problem and the suggested solution?
Could you also please add a link to the BZ?
Signed-off-by: Ram Lavi <[email protected]>
Signed-off-by: Ram Lavi <[email protected]>
8827f61
to
aadaa65
Compare
CI still fail (but for another reason).
|
Change: Review fixes |
if checkupConfig.Verbose { | ||
log.Printf("envVars: %v", envVars) |
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.
Could you please split this change to a commit of its own?
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.
DONE
The env var name is extracted from the net-attach-def annotation with given recipe [0]. This env var is exported to the traffic generator pod as an env var. [0] https://github.com/k8snetworkplumbingwg/sriov-network-device-plugin#pod-device-information Signed-off-by: Ram Lavi <[email protected]>
Signed-off-by: Ram Lavi <[email protected]>
Currently the trex config script is trying to find the PCI-address env variable, assuming that there is only one env var that fits the description. Since this assumption is wrong, changing the script to use the exact name of the env variable. Signed-off-by: Ram Lavi <[email protected]>
Signed-off-by: Ram Lavi <[email protected]>
aadaa65
to
da04b23
Compare
passes on CNV4.12 cluster:
|
on CNV4.13 cluster it fails but the PR does fix what it intened to fix:
|
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.
Thank you for the changes @RamLavi.
On sriov-network-device-plugin a new label was added to the container env vars.
This is causing the checkup to fail the traffic generator pod initiation.
The reason is the currently the trex config script is trying to find the PCI-address env
variable, assuming that there is only one env var that fits the description.
Since this assumption is wrong, changing the script to use the exact
name of the env variable.