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
Chore: Enable PR testing in Drone #26189
Conversation
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Are you planning to enable drone as check here so that we can verify it works before merging? Regarding starlark, seems like you have both a starlark file and a drone.yaml file - what's the plan for migrating to jsonnet? |
@marefr the idea is to try to use Drone instead of Circle for PR testing, but making the switch would have to be discussed with leadership first. As a first step we should get builds running on Drone, quite simply. Regarding the .drone.yml, I explain in the PR description that the .drone.yml file has to be auto-generated and committed since our Drone server had to disable the Starlark extension. It hasn't been decided at all if we should use Jsonnet or not for the templating here, but at least while prototyping I way prefer Starlark since it's much easier to write and read. |
Okay. But can't we run them in parallel for a while where CircleCI continues to be required? Less risk of accepting and merging this then. Can you at least link to a drone build using this branch?
Sorry missed that.
Got it. But I got the impression jsonnet have been decided from infra perspective since they disabled starlark and enabled jsonnet. Probably personal taste here, but I find it more disrupting learning a new language and harder to read this. Maybe because I haven't used python? Not important for now, but when company have agreed on starlark or jsonnet we should use that. One thing I'm currently lacking is how you actually generating the drone.yml file. |
@marefr Yes, we can run them in parallel. This PR isn't doing anything to remove Circle. To convert the file: About Jsonnet, from my speaking with Craig; they enabled Jsonnet since there's more need for it, and as a result they had no choice but to disable Starlark. If they're able to technically, they'll re-enable it. There hasn't been any policy choice on which language to use for Grafana's CI, it's a decision that should be made down the line. In the meantime, Starlark is way easier to work with. I've used both Python and Jsonnet, so know their strengths and weaknesses. |
@@ -13,7 +13,7 @@ | |||
"e2e:debug": "./e2e/start-and-run-suite debug", | |||
"e2e:dev": "./e2e/start-and-run-suite dev", | |||
"jest": "jest --notify --watch", | |||
"jest-ci": "mkdir -p reports/junit && export JEST_JUNIT_OUTPUT_DIR=reports/junit && jest --ci --reporters=default --reporters=jest-junit --maxWorkers 2", | |||
"jest-ci": "mkdir -p reports/junit && export JEST_JUNIT_OUTPUT_DIR=reports/junit && jest --ci --reporters=default --reporters=jest-junit -w ${TEST_MAX_WORKERS:-100%}", |
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.
"jest-ci": "mkdir -p reports/junit && export JEST_JUNIT_OUTPUT_DIR=reports/junit && jest --ci --reporters=default --reporters=jest-junit -w ${TEST_MAX_WORKERS:-100%}",
What does the -100% mean?
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.
@dprokop The bash syntax with ${SOMETHING:-<default>}
means to use the default value if the environment variable isn't set. So it defaults to "100%" for max workers. Basically, 100% parallelization works on Drone, but not on Circle (runs out of memory).
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.
kk, gotcha. Thanks!
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
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
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
What this PR does / why we need it:
Enable PR testing in Drone, along with required fixes. Especially e2e and database integration tests needed tweaking to support running under Drone. CircleCI is untouched, so that will work as before.
The Drone configuration is written in .drone.star (Drone has built-in support for the Starlark language). However, since Starlark processing is currently disabled in our Drone server (due to conflicting with the Jsonnet extension), we have to convert to .drone.yml locally. So basically ignore .drone.yml as it's auto-generated.
The Drone configuration consists of two pipelines for testing PRs: OSS and enterprise. Had to break it into two pipelines since Drone will only distribute pipelines across machines, and not steps within them.
The command to convert .drone.star into .drone.yml is
Drone builds of my Grafana fork are here.