cncf-tags / green-reviews-tooling

Project Repository for the WG Green Reviews which is part of the CNCF TAG Environmental Sustainability
https://github.com/cncf/tag-env-sustainability/tree/main/working-groups/green-reviews
Apache License 2.0
27 stars 14 forks source link

Add benchmark workflow and set `benchmark_path` for Falco #124

Open nikimanoledaki opened 1 month ago

nikimanoledaki commented 1 month ago

What type of PR is this?

kind/feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Addresses part of https://github.com/cncf-tags/green-reviews-tooling/issues/121

nikimanoledaki commented 1 month ago

Issue from running the pipeline manually: https://github.com/cncf-tags/green-reviews-tooling/actions/runs/11462226390/job/31893194549#step:2:26

dipankardas011 commented 1 month ago

https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_iduses

Yes we might need to remove the version thing from here or specify the full path as specified in the option 1 for example octo-org/example-repo/.github/workflows/reusable-workflow.yml@main

also another question are we expecting that all the workflows we are going to specify under uses to have the event of workflow_dispatch? and not workflow_call

dipankardas011 commented 1 month ago

Actually don't have perms so adding the patch file here 😄

commit e29fb40a080acb1a8c511c8cbe419669c6d5ea79
Author: Dipankar Das <65275144+dipankardas011@users.noreply.github.com>
Date:   Wed Oct 23 22:04:06 2024 +0530

    added extra input field for the benchmark pipeline

    Signed-off-by: Dipankar Das <65275144+dipankardas011@users.noreply.github.com>

diff --git a/.github/workflows/benchmark-pipeline.yaml b/.github/workflows/benchmark-pipeline.yaml
index dab9a63..567ab66 100644
--- a/.github/workflows/benchmark-pipeline.yaml
+++ b/.github/workflows/benchmark-pipeline.yaml
@@ -6,6 +6,9 @@ on:
       cncf_project:
         description: Project to be deployed e.g. falco
         required: true
+      full_repo_name:
+        description: Full name of the repository e.g. falcosecurity/falco
+        required: true
       config:
         description: Configuration if project has multiple variants they wish to test e.g. ebpf
         required: false
@@ -69,7 +72,7 @@ jobs:
     steps:
       - uses: jenseng/dynamic-uses@v1
         with:
-          uses: ${{ inputs.benchmark_path }}@${{ github.ref_name }}
+          uses: ${{ inputs.full_repo_name }}/${{ inputs.benchmark_path }}@${{ github.ref_name }}

   delete:
     runs-on: ubuntu-22.04
diff --git a/scripts/project-trigger.sh b/scripts/project-trigger.sh
index 7f74a86..ef3b5a2 100755
--- a/scripts/project-trigger.sh
+++ b/scripts/project-trigger.sh
@@ -50,7 +50,7 @@ jq -c '.projects[]' "$json_file" | while read -r project; do
             -H "Authorization: Bearer $gh_token" \
             -H "X-GitHub-Api-Version: 2022-11-28" \
             "https://api.github.com/repos/$workflow_organization_name/$workflow_project_name/actions/workflows/$workflow_dispatcher_file_name/dispatches" \
-            -d "{\"ref\":\"${git_ref}\",\"inputs\":{\"cncf_project\":\"${proj_name}\",\"config\":\"\",\"version\":\"${latest_proj_version}\"}}")
+            -d "{\"ref\":\"${git_ref}\",\"inputs\":{\"full_repo_name\":\"${proj_organization}/${proj_name}\",\"cncf_project\":\"${proj_name}\",\"config\":\"\",\"version\":\"${latest_proj_version}\"}}")

         status_code=$?
         if [ $status_code -ne 0 ]; then
@@ -68,7 +68,7 @@ jq -c '.projects[]' "$json_file" | while read -r project; do
                 -H "Authorization: Bearer $gh_token" \
                 -H "X-GitHub-Api-Version: 2022-11-28" \
                 "https://api.github.com/repos/$workflow_organization_name/$workflow_project_name/actions/workflows/$workflow_dispatcher_file_name/dispatches" \
-                -d "{\"ref\":\"${git_ref}\",\"inputs\":{\"cncf_project\":\"${proj_name}\",\"config\":\"${config}\",\"version\":\"${latest_proj_version}\"}}")
+                -d "{\"ref\":\"${git_ref}\",\"inputs\":{\"full_repo_name\":\"${proj_organization}/${proj_name}\",\"cncf_project\":\"${proj_name}\",\"config\":\"${config}\",\"version\":\"${latest_proj_version}\"}}")

             status_code=$?
             if [ $status_code -ne 0 ]; then

Try this out @nikimanoledaki also @rossf7 added a few extra input params for the benchmark pipeline

rossf7 commented 1 week ago

Hey @nikimanoledaki @locomundo @dipankardas011 @vallss I'm late to the party but I found some time to look at this. I couldn't get it working but I'm a bit clearer on the problems.

For the workflow reference usually ./.github/workflows/benchmark-workflows/falco.yml would work but because we're using the dynamic-uses action we need to use the fully qualified name. Also as @dipankardas011 says we need to provide a ref to a branch or a tag.

So the pipeline input should be cncf-tags/green-reviews-tooling/.github/workflows/benchmark-workflows/falco.yml@nm/add-benchmark-path.

The problem is the run still fails :( and seems we are hitting this https://github.com/jenseng/dynamic-uses/issues/9

Download action repository 'cncf-tags/green-reviews-tooling@rf/test-benchmark-path' (SHA:b8709dd3cb20dc69ecb07058fc8f76c9a0907f87)
Error: Can't find 'action.yml', 'action.yaml' or 'Dockerfile' for action 'cncf-tags/green-reviews-tooling/.github/workflows/benchmark-workflows/falco.yml@rf/test-benchmark-path'.

If we can fix the problem with dynamic-uses I think we should use the fully qualified name in project.json and extend project-trigger.sh to add the ref before triggering the piipeline. WDYT?

dipankardas011 commented 1 week ago

The problem is the run still fails :( and seems we are hitting this https://github.com/jenseng/dynamic-uses/issues/9

Yes the problem is this dynamic one works like what we expect the action/checkout where it is a repo and in that repo it has action.yml and it tries to use it so if we see more closely the source of this dynamic-uses we can see it tries to call another action within it and it fails to find the action.yml as there is nothing AFAIK it seems to have a different objective which it solves

I might be wrong will check again ;)

dipankardas011 commented 1 week ago

I think we should use the fully qualified name in project.json and extend project-trigger.sh to add the ref before triggering the piipeline. WDYT?

Yes, then only we can make it fully resolvable as our design was to make outside repo workflows as well

If the dynamic job dispatch doesn't work we can easily switch basck to jobs.*.if and can provide the with as a static step (Yes as a backup option)

Or Programmatically, can use program of our choice to dispatch the workflows, seems like dagger might be interesting thought as well 😄

@rossf7 WDYT?

rossf7 commented 1 week ago

If the dynamic job dispatch doesn't work we can easily switch back to jobs.*.if and can provide the with as a static step (Yes as a backup option)

@dipankardas011 Good catch that dynamic-uses expects there to be an action.yml in the root. It does look like it has a slightly different use case and I'm not sure trying to adapt it to our use case makes sense.

I think restructuring the workflows so we can use if conditions could work. One solution could be to add a workflow in .github/workflows/benchmark-workflows/benchmark.yml that takes cncf_project as an input. This workflow would then call falco.yml.

Not the most elegant solution but I'd prefer we solve this with GH actions without higher level tooling like Dagger. At least until we've run out of options with actions :)

dipankardas011 commented 1 week ago

One solution could be to add a workflow in .github/workflows/benchmark-workflows/benchmark.yml that takes cncf_project as an input. This workflow would then call falco.yml.

Unable to understand this workaround, sorry for that 😄

rossf7 commented 1 week ago

Unable to understand this workaround, sorry for that

@dipankardas011 Sorry it wasn't clear. It was just an idea to use conditionals instead of dynamic uses but not sure it makes sense! I think it will be easier to discuss in a call and we have the mob pairing session on Wednesday.