-
Notifications
You must be signed in to change notification settings - Fork 113
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
[metrics 3/x] end-to-end tests #702
[metrics 3/x] end-to-end tests #702
Conversation
Thanks for your PR,
To skip the vendors CIs use one of:
|
4ae399a
to
87f725c
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 10453520475Details
💛 - Coveralls |
87f725c
to
2e4cab8
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
2e4cab8
to
f824a33
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
f824a33
to
f11ade3
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
f11ade3
to
20d2790
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
20d2790
to
af25c1b
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
af25c1b
to
9e21801
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
9e21801
to
35b5ded
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
||
metricsExporterPod := metricsExporterPods.Items[0] | ||
|
||
command := []string{"wget", "-qO-", "http://127.0.0.1:9110/metrics"} |
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.
please switch this one to curl as in the ubi images we don't have wget
Trying to pull registry.redhat.io/ubi9/ubi:latest...
Getting image source signatures
Checking if image destination supports signatures
Copying blob bb65efb0bec6 done
Copying config 02eedf8dcc done
Writing manifest to image destination
Storing signatures
[root@d7d4fbbaf778 /]# wget
bash: wget: command not found
[root@d7d4fbbaf778 /]# curl
curl: try 'curl --help' for more information
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.
sure! no problem!
finalRxPackets := getCounterForPod(finalMetrics, pod, "sriov_vf_rx_packets") | ||
|
||
Expect(finalRxBytes).Should(BeNumerically(">", initialRxBytes)) | ||
Expect(finalRxPackets).Should(BeNumerically(">", initialRxPackets+3)) |
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 you add the +3 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.
Because pingPod(...)
function sends 3 ICMP packets
sriov-network-operator/test/conformance/tests/test_sriov_operator.go
Lines 2652 to 2669 in a8476f0
func pingPod(ip string, nodeSelector string, sriovNetworkAttachment string) { | |
ipProtocolVersion := "6" | |
if len(strings.Split(ip, ".")) == 4 { | |
ipProtocolVersion = "4" | |
} | |
podDefinition := pod.RedefineWithNodeSelector( | |
pod.RedefineWithCapabilities( | |
pod.RedefineWithRestartPolicy( | |
pod.RedefineWithCommand( | |
pod.DefineWithNetworks([]string{sriovNetworkAttachment}), | |
[]string{"sh", "-c", fmt.Sprintf("ping -%s -c 3 %s", ipProtocolVersion, ip)}, []string{}, | |
), | |
corev1.RestartPolicyNever, | |
), | |
[]corev1.Capability{"NET_RAW"}, | |
), | |
nodeSelector, | |
) |
I could refactor that function a bit to make this information more explicit
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.
is it not enough to keep the check > initialRxPackets ?
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.
yes, it is. It would also be clearer as the number "3" is hidden in another function.
fixing
35b5ded
to
a8476f0
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
a8476f0
to
ae75722
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
d753fe9
to
bd07223
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.
lgtm
bd07223
to
50e8b8f
Compare
By("Enabling `metricsExporter` feature flag") | ||
setFeatureFlag("metricsExporter", true) | ||
|
||
By("Adding ") |
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.
nit: can you fix this BY
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!
return true | ||
} | ||
|
||
func addLabelToNamespace(ctx context.Context, namespaceName, key, value string) error { |
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.
nit: can you move this to utils.namespace please
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.
sure!
Exposed metrics can be verified by scraping the prometheus endpoint on the `sriov-network-metrics-exporter` pod. Add a test that spawns an SR-IOV consuming pod and verifies its receiving counter increase when the interface is pinged from outside. Signed-off-by: Andrea Panattoni <[email protected]>
50e8b8f
to
5a950b6
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.
LGTM nice work!
Exposed metrics can be verified by scraping the prometheus
endpoint on the
sriov-network-metrics-exporter
pod.Add a test that spawns an SR-IOV consuming pod and verifies
its receiving counter increase when the interface is pinged from
outside.
depends on: