Remove Theia related files #4048
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4048 +/- ##
==========================================
+ Coverage 61.53% 67.21% +5.68%
==========================================
Files 293 297 +4
Lines 43686 45200 +1514
==========================================
+ Hits 26882 30381 +3499
+ Misses 14567 12457 -2110
- Partials 2237 2362 +125
*This pull request uses carry forward flags. Click here to find out more.
|
This PR removes Theia related files, including Grafana Flow Collector, ClickHouse monitor, manifests, CI jobs and scripts. Signed-off-by: Yongming Ding <dyongming@vmware.com>
dreamtalen
requested review from
yanjunz97,
heanlan,
antoninbas and
salv-orlando
build/yamls/flow-visibility-e2e.yml
Outdated
| @@ -0,0 +1,158 @@ | |||
| apiVersion: v1 | |||
| data: | |||
| create_table.sh: "#!/usr/bin/env bash\n\n# Copyright 2022 Antrea Authors.\n#\n# | |||
There was a problem hiding this comment.
hopefully this does not change too often as it's a bit messy to update. BTW, is there any way to use a "raw" string here, so that the data is easier to read / troubleshoot / update. Something like this:
---
apiVersion: v1
data:
create_table.sh: |
# line 1
# line 2
echo "hello"
end: trueThe alternative is to auto-generate this file, which may be the case (maybe there is a script in Theia?), in which case ignore this comment.
There was a problem hiding this comment.
Thanks Antonin, this file doesn't need to be modified unless we change the fields of flow records exported by Flow Aggregator.
I also thought about auto-generate this file in Theia and downloading it during the e2e test. But I could imagine that when we want to change fields of flow records in the future, we need to change flow-visibility-e2e.yml on the Theia side first to let our tests on the Antrea side pass. Not sure whether this is a good approach, may I ask your suggestion pls?
There was a problem hiding this comment.
I assume that you have a way to auto-generate it manually with a script though, right? based on some files in the theia repo. If the answer is yes, then it's good enough for me and there is no need to change anything.
There was a problem hiding this comment.
Auto-generate this file from Theia repo sounds good to me. Just to add, create_table.sh content can be formatted well when we using Helm to generate this file as in Theia repo.
There was a problem hiding this comment.
Yes, we have a script in Theia to generate this manifest.
Although I made some changes to delete the clickhouse monitor in the generated manifest for now, we will have an update on the script in Theia to generate one exactly same manifest.
There was a problem hiding this comment.
Hi @antoninbas, we have updated this manifest with a better format and comments about how to generate it, please take a look again, thanks!
|
/test-all |
build/yamls/flow-visibility-e2e.yml
Outdated
| @@ -0,0 +1,335 @@ | |||
| # This manifest is used in the E2E test to check Flow Aggregator exporting flow records | |||
| # to ClickHouse database. It is autogenerated by scripts in the Theia, please refer to | |||
There was a problem hiding this comment.
| # to ClickHouse database. It is autogenerated by scripts in the Theia, please refer to | |
| # to ClickHouse database. It is autogenerated by scripts in Theia, please refer to |
There was a problem hiding this comment.
Thanks, addressed.
Signed-off-by: Yongming Ding <dyongming@vmware.com>
This PR removes Theia related files, including Grafana Flow Collector,
ClickHouse monitor, manifests, CI jobs and scripts.
Retain a small manifest
flow-visibility-e2e.ymlfor the Kind e2e test of Flow Aggregator.Signed-off-by: Yongming Ding dyongming@vmware.com