-
Notifications
You must be signed in to change notification settings - Fork 373
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
Use labels to enhance records filtering in FA e2e tests #5731
Conversation
abc51e5
to
968b45d
Compare
968b45d
to
94ab3f2
Compare
94ab3f2
to
6773619
Compare
9ce0eb8
to
6c8789f
Compare
de429cf
to
70d2b32
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.
I think the PR title & description probably need to be updated?
@@ -1335,10 +1361,10 @@ func checkPodAndNodeDataClickHouse(data *TestData, t *testing.T, record *ClickHo | |||
assert.Equal(record.DestinationPodName, dstPod, "Record with dstIP does not have Pod name: %s", dstPod) | |||
assert.Equal(record.DestinationPodNamespace, data.testNamespace, "Record does not have correct destinationPodNamespace: %s", data.testNamespace) | |||
assert.Equal(record.DestinationNodeName, dstNode, "Record does not have correct destinationNodeName: %s", dstNode) | |||
assert.Equal(record.SourcePodLabels, fmt.Sprintf("{\"antrea-e2e\":\"%s\",\"app\":\"iperf\"}", srcPod), "Record does not have correct label for source Pod") | |||
assert.Equal(record.DestinationPodLabels, fmt.Sprintf("{\"antrea-e2e\":\"%s\",\"app\":\"iperf\"}", dstPod), "Record does not have correct label for destination Pod") | |||
assert.Contains(record.SourcePodLabels, fmt.Sprintf("\"antrea-e2e\":\"%s\",\"app\":\"iperf\"", srcPod), "Record does not have correct label for source Pod") |
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.
that relies on the labels being ordered alphabetically in the record. Is that guaranteed?
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, when we add labels in Flow-Aggregator, we first use json.marshall for the label map, then convert it into a string.
And json.marshall will sort the map key lexicographically.
What I do here is just remove the curly bracket as we have extra label here.
70d2b32
to
6325d6d
Compare
6325d6d
to
707540f
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.
Does this fix #5730, I don't see a reference to the issue anywhere?
Also waiting for @heanlan and @dreamtalen to review
707540f
to
3dea401
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 overall
In this commit, we: 1. Set a label to Perftest Pods before initiating traffic in each subtest to filter records in both the IPFIX collector Pod and the ClickHouse. 2. Remove the --since-time flag used during log retrieval from the IPFIX collector Pod in the e2e test. 3. Cease reliance on timestamps for record filtering due to potential time discrepancies between the testbed and Kubernetes nodes, which might hinder the retrieval of desired logs. Signed-off-by: Yun-Tang Hsu <hsuy@vmware.com>
3dea401
to
8286d42
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
/test-e2e |
In this commit, we: 1. Add a label to Perftest Pods before initiating traffic in each subtest to filter records in both the IPFIX collector Pod and the ClickHouse. 2. Remove the --since-time flag used during log retrieval from the IPFIX collector Pod in the e2e test. 3. Stop relying on timestamps for record filtering due to potential time discrepancies between the testbed and Kubernetes nodes, which might hinder the retrieval of desired records. Signed-off-by: Yun-Tang Hsu <hsuy@vmware.com>
In this commit, we: 1. Add a label to Perftest Pods before initiating traffic in each subtest to filter records in both the IPFIX collector Pod and the ClickHouse. 2. Remove the --since-time flag used during log retrieval from the IPFIX collector Pod in the e2e test. 3. Stop relying on timestamps for record filtering due to potential time discrepancies between the testbed and Kubernetes nodes, which might hinder the retrieval of desired records. Signed-off-by: Yun-Tang Hsu <hsuy@vmware.com>
In this pull request, we:
resolve #5730