LLT-5878: Xray tests on CI #16
Conversation
tomasz-grz
force-pushed
the
xray_ci
branch
3 times, most recently
from
57c3b60 to
267f8a3
Compare
tomasz-grz
force-pushed
the
xray_ci
branch
10 times, most recently
from
42387d4 to
3132161
Compare
tomasz-grz
force-pushed
the
xray_ci
branch
5 times, most recently
from
d201834 to
3a568fe
Compare
tomasz-grz
force-pushed
the
xray_ci
branch
2 times, most recently
from
4513cd4 to
ca368c8
Compare
| @@ -0,0 +1 @@ | |||
| results/ | |||
There was a problem hiding this comment.
I've been adding other xray-related ignores to the top level .gitignore so far. I like this idea of having them in the xray folder, but it would make it inconsistent. I'll leave it to you to decide if you move this to the top-level or if you move the other xray-related ones here, but I would like to see it fixed
|
|
||
| - **--ascii**: output the analysis chart as text graph. | ||
|
|
||
| - **--disable-drop-privileges**: pass the `disable-drop-privileges` flag to `neptun-cli` and `boringtun-cli`. |
There was a problem hiding this comment.
Is this something that needs to be passed through as a cli arg or can we just pass this unconditionally to neptun/boringtun?
There was a problem hiding this comment.
Good question, I guess it depends if we would ever want to run it without disable-drop-privileges and how it impacts the tests.
| @@ -187,7 +213,7 @@ def packet_funnel(self, ax): | |||
| f"Recv ({recv})", | |||
| ] | |||
| values = [self.count, before_wg, after_wg, recv] | |||
| plt.bar(categories, values, color="blue", width=0.4) | |||
| ax.bar(categories, values, color="blue", width=0.4) | |||
| plt.show() | ||
| if save_output: | ||
| plt.savefig(test_path + ".png") |
There was a problem hiding this comment.
Have you tried running this locally with passing --ascii? I think I read somewhere that savefig needs to be called before show because show will clear the plot, leaving nothing for savefig to save. I saw in the artifacts that it's working though, which is weird. But maybe move the if save_output to be before the show
| return (run.stdout, run.stderr) | ||
|
|
||
|
|
||
| def get_csv_name(wg, test_type, count): | ||
| return f"results/xray_metrics_{wg.lower()}_{test_type}_{count}.csv" | ||
|
|
||
|
|
||
| def get_pcap_name(wg, test_type, count): | ||
| return f"results/{WG_IFC_NAME}_{wg.lower()}_{test_type}_{count}.pcap" | ||
| def get_test_path(wg, test_type, count): |
There was a problem hiding this comment.
I think I would prefer to have a separate function for getting the output files you're creating for now. The naming scheme I started here is not reusable enough (the images you output would be named xraywg1* which is the name of the interface). I think a bit of an overhaul of these path getters would be good and then we could centralize on a single function for this
| @@ -104,6 +110,9 @@ def main(): | |||
| parser.add_argument("--count") | |||
| parser.add_argument("--nobuild-neptun", action="store_true") | |||
| parser.add_argument("--nobuild-xray", action="store_true") | |||
| parser.add_argument("--save-output", action="store_true") | |||
| parser.add_argument("--disable-drop-privileges", action="store_true") | |||
| parser.add_argument("--ascii", action="store_true") | |||
There was a problem hiding this comment.
I think it might be better to have this take a value and then create an enum for it where the options could be Ascii and Graphical (or something like that) for now. I don't see a value in outputting both formats at the same time. It would also make it slightly easier to understand in the analyzer script if it was if output_type == OutputType.Ascii: instead of just if ascii:
| if save_output: | ||
| plt.savefig(test_path + ".png") | ||
|
|
||
| if ascii: |
There was a problem hiding this comment.
Let's make this an if-else so we only show one of the two formats at the same time. And maybe it can be simplified since both branches would have some code in common
| pipenv run python run.py --wg native --ascii --save-output | ||
| pipenv run python run.py --wg neptun --ascii --save-output --disable-drop-privileges |
There was a problem hiding this comment.
We should also run these with --test-type plaintext, and we probably want to have a bigger package count too, maybe 10k
This MR adds a
xray-testsgithub workflow that performs the tests and displays the result in text format, as well as uploading thepnggraphs as artifacts.To achieve this 3 more arguments were added to xray run script:
--save-outputsaves the analysis charts inresultsfolder to be uploaded as job artifacts.--asciioutput the analysis chart as text graph to be displayed in the CI job.--disable-drop-privilegesflag is passed toneptun-cliandboringtun-cli, without it NepTUN is failing to start on the CI runner.