Conversation
6d44320 to
e7de579
Compare
39df8a0 to
2540053
Compare
Nexucis
left a comment
There was a problem hiding this comment.
overall, thank you for putting this in place.
I have added few suggestions that will improve the CI/CD and simplify it.
I will probably add more comments later, but this is already a good step
8fb1c53 to
c516e2b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
d2392ae to
55bb62e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
d19cd5a to
b8f5dce
Compare
b8f5dce to
0519fe4
Compare
0519fe4 to
f8c0224
Compare
c43f60d to
a2e6d5e
Compare
0fb6e00 to
2e5b42d
Compare
|
@jgbernalp over to you, just to ensure you are fine with this approach. @shahrokni can you solve the conflicts please ? |
Yes sure, I will rebase main and resolve the conflicts |
ebec558 to
09da6c9
Compare
Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com> Signed-off-by: Mahmoud Shahrokni <seyedmahmoud.shahrokni@amadeus.com>
526da27 to
f38bb3b
Compare
| } | ||
|
|
||
| // Instead of a blind sleep 10, could we do a health logic here? I am not sure! Ask Gab | ||
| // Hey Gab, Plugins start on an arbitrary port, how could we check if they are ruining or not?! |
There was a problem hiding this comment.
We already have similar logic in percli start https://github.com/perses/perses/blob/main/internal/cli/cmd/plugin/start/start.go#L268 maybe we can reuse and also reuse the taskhelper which takes care of process synchronization
There was a problem hiding this comment.
Another way to handle that would be to build the plugin and then injecting the archive in the container perhaps by using the provisioning (which is not implemented).
Perhaps it will simplify the process
There was a problem hiding this comment.
@Nexucis and @jgbernalp
Please correct me if I am wrong,
Reusability issue
I started implementing your suggestion, but it’s turning into a bit of a rabbit hole!
To reuse taskhelper, we need to use waitDevServerTasks which is the output of buildWaitDevServerTasks(pluginInDev)
buildWaitDevServerTasks starts with a lowercase letter. This means it is private to the package where it is defined.
So, I decided to include it in my script. However, I ran into some importing issues.
Importing v1.PluginInDevelopment is dependency issue. Because that struct lives in the core v1 API package, Go forces my project to resolve and track the entire Perses dependency tree.
error while importing github.com/perses/perses/pkg/model/api/v1: missing go.sum entry for module providing package github.com/prometheus/common/config (imported by github.com/perses/perses/pkg/model/api/v1/secret); to add:
go get github.com/perses/perses/pkg/model/api/v1/secret@v0.53.0-rc.0.0.20260113112038-1e65a7475ba9compiler
error while importing github.com/perses/perses/pkg/model/api/v1: missing go.sum entry for module providing package github.com/zitadel/oidc/v3/pkg/oidc (imported by github.com/perses/perses/pkg/model/api); to add:
go get github.com/perses/perses/pkg/model/api@v0.53.0-rc.0.0.20260113112038-1e65a7475ba9compiler
There was a problem hiding this comment.
I suggested to reuse the taskHelper which is part of the common package. "github.com/perses/common/async/taskhelper" we probably cannot reuse the exact logic from waitDevServerTasks as this is specific to the internals of percli. My suggestion is to use the process management tooling we use for this and other projects.
There was a problem hiding this comment.
I used taskHelper as suggested to manage async tasks. OK, that is a good suggestion.
However, I would like to mention that the effort here is to eliminate the time.sleep between
- Running a plugin (on an arbitrary port using percli)
- Running its relevant tests (when they are attached and ready)
Simply put, we should wait for the following results, before we run any e2e tests
So the question is, when those plugins are running on arbitrary ports hidden from our script, how could we assure that the plugin is ready? If the ports were predictable, there could be a health check mechanism.
Using taskHelper alone, although a good suggestion, does not resolve the main issue here.
If there is anything that I am missing, please let me know
There was a problem hiding this comment.
We can track and search for the dev start script output, as the percli does. But that might get things overly complex.
Maybe a simpler alternative would be to use the load from multiple folders. The issue we still need to resolve is that we either need to support multiple versions of the same plugin, the one included in the image and the one being tested, or have an option in Perses to disallow the core plugin download and only populate from the existing configured folders when starting up, WDYT @Nexucis ?
Relates to perses/perses#3799
CI e2e Setup
This change adds Playwright e2e testing setup to the Plugins repo.
It tests all plugins which have a test directory under e2e/tests.
Demonstration of tests results for the table and trace table plugins
The idea
Simply speaking the idea is to use the latest version of Perses and CLI to run tests over individual plugins.
Why should we install Perses CLI binary ❓
Couldn't we use the CLI in the running container? Apparently, not. Perses image is distroless meaning, npm/node are not included. Under the hood Plugin Start command uses node.
Therefore, we should run the binary CLI on the VM and communicate with the running Perses.
Screenshots
Checklist
[<catalog_entry>] <commit message>naming convention using one of thefollowing
catalog_entryvalues:FEATURE,ENHANCEMENT,BUGFIX,BREAKINGCHANGE,DOC,IGNORE.UI Changes