Skip to content
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

workflow: move build operations in a reusable workflow #244

Merged
merged 10 commits into from
Oct 26, 2023

Conversation

chantra
Copy link
Contributor

@chantra chantra commented Oct 24, 2023

This change essentially isolated the old jobs:

  • build
  • test
  • veristat

in re-usable workflows: .github/workflows/kernel-{build,test,veristat}.yml.

There is then another reusable workflow: .github/workflows/kernel-build-test.yml which runs the jobs to build, test, and run veristat for a specific architecture/toolchain.

Finally, .github/workflows/test.yml is still the main workflow, but now run the set-matrix job and initiate a kernel-build-test workflow for each arch/toolchain pairs.
This allow us to essentially start those workflow in parallel, as a build in a specific workflow is finished, it will process with testing directly without waiting for other jobs.

Finally, matrix.py was changed to include the tests to run directly as part of the build-matrix. This allow us to not have to do any filtering as part of the GH actions.
In the future, the test-matrix could be removed. In the future, we may also want to embed whether or not we run veristat at the build matrix level.... which is not a build-matrix anymore, but an actual full end-to-end matrix.

@chantra chantra marked this pull request as ready for review October 24, 2023 21:30
@chantra chantra marked this pull request as draft October 24, 2023 21:31
@chantra chantra force-pushed the reuseable_workflow_tests branch 4 times, most recently from e08d567 to b6206d7 Compare October 24, 2023 21:57
@chantra chantra marked this pull request as ready for review October 25, 2023 00:34
@chantra chantra marked this pull request as draft October 25, 2023 00:35
@chantra chantra force-pushed the reuseable_workflow_tests branch 22 times, most recently from 8ff1d21 to bb192fa Compare October 25, 2023 19:11
This is step 1 of running the kernel build from a re-useable workflow.
This change simply move the "build" job in its own reusable workflow file.

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
Signed-off-by: Manu Bretelle <chantr4@gmail.com>
Signed-off-by: Manu Bretelle <chantr4@gmail.com>
…h/toolchain

matrix.py was modified to embed the tests in the build-matrics, this way
we can trigger all tests within 1 workflow without needing to do some filtering in the GH action side.

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
@chantra chantra force-pushed the reuseable_workflow_tests branch from bb192fa to 1ac463c Compare October 25, 2023 20:20
Signed-off-by: Manu Bretelle <chantr4@gmail.com>
Signed-off-by: Manu Bretelle <chantr4@gmail.com>
@chantra chantra changed the base branch from master to reusable-workflow October 25, 2023 23:02
@chantra chantra marked this pull request as ready for review October 25, 2023 23:02
@chantra
Copy link
Contributor Author

chantra commented Oct 25, 2023

I have ran KPD against this and it seems that it will handle updating KPD just fine.

I am also opening this PR against a different branch than master so we can switch KPD to use the new vmtest code and possibly revert back to main which we know is stable.

This makes it nicer in the UI vs the  auto-generated name... which is super long
because they include the matrix parameters as part of it and it ended looking like:

build-and-test (LATEST, self-hosted, x86_64, x86_64, llvm, 16, llvm-16, test_progs, false, 360, t... / test (test_progs, false, 360) / test_progs on x86_64 with llvm-16

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
At this stage, we are already not using `test-matrix` anymore, so let's remove it
and move the logic of adding the tests within the first loop over the matrix.

For where to run veristat, as eluded to in a comment I am removing, we can control
that from within matrix.py directly.
Finally, we probably don't care on what/where veristat runs on. Let's default
to wherever we are currently running builds and tests.

Unless specified on the matrix, we will not run veristat.

Finally, here is what the output of matrix looks like
```
GITHUB_REPOSITORY_OWNER=kernel-patches GITHUB_REPOSITORY=vmtest GITHUB_OUTPUT=/dev/stdout python3 .github/scripts/matrix.py   | cut -d= -f2- | jq .
{
  "include": [
    {
      "kernel": "LATEST",
      "runs_on": [
        "ubuntu-20.04"
      ],
      "arch": "x86_64",
      "toolchain": "gcc",
      "llvm-version": "16",
      "run_veristat": true,
      "toolchain_full": "gcc",
      "tests": {
        "include": [
          {
            "test": "test_progs",
            "continue_on_error": false,
            "timeout_minutes": 360
          },
          {
            "test": "test_progs_parallel",
            "continue_on_error": true,
            "timeout_minutes": 30
          },
          {
            "test": "test_progs_no_alu32",
            "continue_on_error": false,
            "timeout_minutes": 360
          },
          {
            "test": "test_progs_no_alu32_parallel",
            "continue_on_error": true,
            "timeout_minutes": 30
          },
          {
            "test": "test_maps",
            "continue_on_error": false,
            "timeout_minutes": 360
          },
          {
            "test": "test_verifier",
            "continue_on_error": false,
            "timeout_minutes": 360
          }
        ]
      }
    },
    {
      "kernel": "LATEST",
      "runs_on": [
        "ubuntu-20.04"
      ],
      "arch": "x86_64",
      "toolchain": "llvm",
      "llvm-version": "16",
      "toolchain_full": "llvm-16",
      "run_veristat": false,
      "tests": {
        "include": [
          {
            "test": "test_progs",
            "continue_on_error": false,
            "timeout_minutes": 360
          },
          {
            "test": "test_progs_parallel",
            "continue_on_error": true,
            "timeout_minutes": 30
          },
          {
            "test": "test_progs_no_alu32",
            "continue_on_error": false,
            "timeout_minutes": 360
          },
          {
            "test": "test_progs_no_alu32_parallel",
            "continue_on_error": true,
            "timeout_minutes": 30
          },
          {
            "test": "test_maps",
            "continue_on_error": false,
            "timeout_minutes": 360
          },
          {
            "test": "test_verifier",
            "continue_on_error": false,
            "timeout_minutes": 360
          }
        ]
      }
    }
  ]
}
```

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
Add a new job to run matrix.py with a combination of owner/repo to make sure we
don't raise.

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
Add a step to run black on our python scripts.

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
@chantra chantra force-pushed the reuseable_workflow_tests branch from 2695dda to 6831f53 Compare October 26, 2023 18:23
Copy link
Contributor

@danielocfb danielocfb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good to me, thanks! I didn't look closely at the Python logic changes.

.github/workflows/kernel-test.yml Show resolved Hide resolved
# builds (which may check out some earlier upstream change).
with:
fetch-depth: 50
- if: ${{ github.repository == 'kernel-patches/vmtest' }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather we make this some kind of meaningful input argument than this reliance on global state. Do you think that's possible?

@chantra chantra changed the base branch from reusable-workflow to master October 26, 2023 22:50
Copy link
Contributor Author

@chantra chantra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked offline and agreed to address this separately once the bulk of the change is on master.

.github/workflows/kernel-test.yml Show resolved Hide resolved
@chantra chantra merged commit 1eed97e into kernel-patches:master Oct 26, 2023
chantra added a commit to chantra/kernel-patches-vmtest that referenced this pull request Oct 27, 2023
@danielocfb brought this up in kernel-patches#244.
e.g we should control whether to execute the test job not from the job itself,
but from the caller.

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
chantra added a commit that referenced this pull request Oct 27, 2023
@danielocfb brought this up in #244.
e.g we should control whether to execute the test job not from the job itself,
but from the caller.

Signed-off-by: Manu Bretelle <chantr4@gmail.com>
@chantra chantra deleted the reuseable_workflow_tests branch October 27, 2023 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants