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

CI: modify to reusable workflow #482

Closed
wants to merge 42 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
0e9ab68
1 connector
Dominik-K Dec 12, 2023
95bdf3e
CI: define workflow as reusable
Dominik-K Dec 18, 2023
fa82ebe
checkout caller repo
Dominik-K Dec 18, 2023
55c303e
fix usage of `{{ inputs. }}`
Dominik-K Dec 18, 2023
b0ea833
`install` and `everestpy` to integration-tests
Dominik-K Dec 18, 2023
ce0da9e
undo unrelated change
Dominik-K Dec 19, 2023
ef701ab
comment 1st partial checkout of `everest-core`
Dominik-K Dec 19, 2023
b7dcc8b
`run_unit_tests`; output shell cmds, correct & portable shebang
Dominik-K Dec 19, 2023
5371770
correct unit-tests invocation
Dominik-K Dec 19, 2023
df793dd
store cache even on failing build
Dominik-K Dec 19, 2023
b00bb1e
defaults for `inputs` if `on.pull_request`
Dominik-K Dec 19, 2023
3ffae7b
no defaults needed for `actions/checkout`
Dominik-K Dec 19, 2023
93aa63a
Merge branch 'main' into CI/reusable-workflow
Dominik-K Dec 21, 2023
a62c1cc
`.gitignore`: ignore all `/build*/`
Dominik-K Dec 21, 2023
f65b397
`integration-tests-image` relies on `compile-container`
Dominik-K Dec 21, 2023
1b93218
compile: common set of cmake flags
Dominik-K Dec 21, 2023
f6eb6db
shorter API
Dominik-K Dec 21, 2023
2b3bf15
fix `cache/` upload
Dominik-K Dec 21, 2023
8e3cf21
`sudo chown cache/josev/`
Dominik-K Dec 22, 2023
9d0b89d
set `--workdir` in `docker run`
Dominik-K Dec 22, 2023
9a4da90
upload test-results as artifact
Dominik-K Dec 22, 2023
d565e5b
Merge branch 'main' into CI/reusable-workflow
Dominik-K Dec 22, 2023
c8f6547
checkout directly to cwd
Dominik-K Dec 22, 2023
e1c8c55
stop execution in scripts on failure
Dominik-K Dec 22, 2023
483526a
tests.sh: remove obsolete cmds
Dominik-K Dec 22, 2023
653707f
explicitly run scripts in docker containers
Dominik-K Dec 22, 2023
5ddc728
forward `CMAKE_FLAGS_EXTRA` to docker-container
Dominik-K Dec 22, 2023
58f05e1
change volumes & workdirs
Dominik-K Dec 23, 2023
24e20d2
cache-path external to workspace
Dominik-K Dec 23, 2023
a84d5d9
bughunt: upload CMake logs
Dominik-K Dec 23, 2023
29b8d1b
`lint` as extra job
Dominik-K Dec 23, 2023
5928cd8
bughunt: list workspace root
Dominik-K Dec 23, 2023
90f4781
move `.ci` out of `everest-core`
Dominik-K Dec 23, 2023
6d4a2e1
avoid doubled checkout
Dominik-K Dec 23, 2023
2147cb8
checkout `ci-repo/` and `source/`
Dominik-K Dec 23, 2023
8701afa
CACHE_PATH=cache
Dominik-K Dec 23, 2023
0776892
fixes
Dominik-K Dec 23, 2023
e1ef485
fix integration-test run
Dominik-K Dec 23, 2023
6c21b2c
fix unit-test report upload
Dominik-K Dec 23, 2023
e3d60cd
fix `docker-compose`
Dominik-K Dec 23, 2023
604ffcd
Merge branch 'main' into CI/reusable-workflow
Dominik-K Jan 10, 2024
82fd457
Merge branch 'main' into CI/reusable-workflow
Dominik-K Jan 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .ci/build-kit/build_install.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/usr/bin/env sh
set -ex

cmake \
-B build/ \
-G Ninja \
-DCMAKE_INSTALL_PREFIX="build/dist/" \
-DBUILD_TESTING=ON \
"${CMAKE_FLAGS_EXTRA}"

ninja -j$(nproc) -C build install
12 changes: 0 additions & 12 deletions .ci/build-kit/compile.sh

This file was deleted.

11 changes: 0 additions & 11 deletions .ci/build-kit/test_and_install.sh

This file was deleted.

4 changes: 4 additions & 0 deletions .ci/build-kit/unit_tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/usr/bin/env sh
set -ex

ninja -j$(nproc) -C build tests/test
3 changes: 2 additions & 1 deletion .ci/e2e/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ services:
driver: none

e2e-test-server:
image: integration-image
image: integration-tests-image
depends_on:
- mqtt-server
environment:
Expand All @@ -20,5 +20,6 @@ services:
- type: bind
source: ./scripts
target: /ext/scripts
working_dir: /ext/
sysctls:
- net.ipv6.conf.all.disable_ipv6=0
8 changes: 6 additions & 2 deletions .ci/e2e/scripts/tests.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
#!/bin/sh
#!/usr/bin/env sh
set -ex

# install everest testing by cmake target to make sure using the version defined in dependencies.yaml
ninja -C build install_everest_testing

cd tests
pytest --everest-prefix ../dist core_tests/*.py framework_tests/*.py
pytest --everest-prefix ../build/dist core_tests/*.py framework_tests/*.py
135 changes: 106 additions & 29 deletions .github/workflows/build_and_test.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,27 @@
name: Build, Lint and Test
on:
pull_request: {}
workflow_call: # defines it as reusable workflow
inputs:
repository:
description: The repo to checkout and run the workflow steps in
type: string
required: true
repository_ref:
description: The ref (branch, tag, commit-hash) of the `inputs.repository`
type: string
required: true
cmake_flags_extra:
description: additional flags to add to the common set of flags (see `.ci/build-kit/compile.sh`)
type: string
unit_tests:
description: Whether to run unit-tests or not
type: boolean
required: true
integration_tests:
description: Whether to run integration-tests or not
type: boolean
required: true
workflow_dispatch:
inputs:
runner:
Expand All @@ -16,54 +37,110 @@ on:

jobs:
build:
name: Build, Lint and Test
name: Build and Test
runs-on: ${{ inputs.runner || 'ubuntu-22.04' }}
env:
CACHE_PATH: cache
CI_REPO: EVerest/everest-core
CI_REPO_DIR: ci-repo
SOURCE_DIR: source
steps:
- name: Format branch name for cache key
run: |
BRANCH_NAME_FOR_CACHE="${GITHUB_REF_NAME//-/_}"
echo "branch_name_for_cache=${BRANCH_NAME_FOR_CACHE}" >> "$GITHUB_ENV"
- name: Setup cache
uses: actions/cache@v3
- name: Restore cache
uses: actions/cache/restore@v3
id: restore-cache
with:
path: cache
path: ${{ env.CACHE_PATH }}
key: compile-${{ env.branch_name_for_cache }}-${{ github.sha }}
restore-keys: |
compile-${{ env.branch_name_for_cache }}-
compile-
- name: Checkout everest-core
uses: actions/checkout@v3
- name: Checkout `${{ env.CI_REPO }}` to `${{ env.CI_REPO_DIR }}/` (for reusable workflow mode, see https://stackoverflow.com/a/74123003/1168315)
uses: actions/checkout@v4
with:
path: source
- name: Run clang-format
uses: everest/everest-ci/github-actions/[email protected]
path: ${{ env.CI_REPO_DIR }}
repository: ${{ env.CI_REPO }}
ref: CI/reusable-workflow # TODO get reference in general, see https://stackoverflow.com/questions/74784735 resp. https://github.com/actions/toolkit/issues/1264
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andistorm Do you know a solution for this? Otherwise, I would just set it to main for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest to not checkout the ci repo in the workflow, but to use custom github actions. There are multiple options for github actions: composite actions that are similiar to reusable workflows, Javascript actions and Docker container actions. The last one would be an option to include additional filesto the action.

I could also imagine a solution with having the bash scripts in each repo with "custom" commands and cmake flags..

- name: Checkout repo ${{ inputs.repository }} to `${{ env.SOURCE_DIR }}/`
uses: actions/checkout@v4
with:
source-dir: source
extensions: hpp,cpp
exclude: cache
- name: Setup run scripts
run: |
mkdir scripts
rsync -a source/.ci/build-kit/ scripts
path: ${{ env.SOURCE_DIR }}
repository: ${{ inputs.repository }}

Choose a reason for hiding this comment

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

This doesn't seem like the right way to set up a reusable workflow. When someone makes a fork, the CI instructions on their forked branches will point to whatever original repository is passed in, for example, EVerest/libocpp. What should happen is that the current branch that initialized the workflow should run, fetching the branch from the forked repository.

In the current state, this fails at the checkout stage because we can't check out the original repository from our fork.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the problem in the way the reusable workflow is implemented, but the way it is called.

In the example of libocpp the parameter repository: EVerest/libocpp is used. You adapted it similiar in your example with repository: EVerest/libfsm

To enable workflows running on forks I suggest the use of the env variables
.

-> In both examples I would replace the line with repository: ${{ github.repository }}.

Choose a reason for hiding this comment

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

Shouldn't the reusable workflow use that by default?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, maybe this parameter is obsolete, not sure about this.. I will spend some time on this in the next weeks to figure those things out

ref: ${{ inputs.repository_ref }}
- name: Pull build-kit image
run: |
docker pull --quiet ghcr.io/everest/build-kit-alpine:latest
docker image tag ghcr.io/everest/build-kit-alpine:latest build-kit
- name: Compile
- name: Build and install
env:
CMAKE_FLAGS_EXTRA: ${{ inputs.cmake_flags_extra || '-DEVC_ENABLE_CCACHE=1 -DISO15118_2_GENERATE_AND_INSTALL_CERTIFICATES=OFF' }}
run: |
ls -hal .
docker run \
--volume "$(pwd):/ext" \
--name compile-container \
build-kit run-script compile
- name: Create integration-image
--volume "${CACHE_PATH:?}:/ext/cache" \
--volume "$(pwd)/${CI_REPO_DIR:?}/.ci:/.ci" \
--volume "$(pwd)/${SOURCE_DIR:?}:/source" \
--workdir "/source" \
--env CMAKE_FLAGS_EXTRA="${CMAKE_FLAGS_EXTRA:?}" \
--name build-container \
build-kit /.ci/build-kit/build_install.sh
- name: upload CMake output & error logs
if: always()
uses: actions/upload-artifact@v3
with:
name: build-install-logs
path: build/CMakeFiles/
- name: Unit tests
if: ${{ inputs.unit_tests || false }}
run: |
docker commit compile-container build-image
docker commit build-container unit-tests-image
docker run \
--volume "$(pwd):/ext" \
--name test-container \
build-image run-script test_and_install
docker commit test-container integration-image
- name: Run integration tests
--volume "$(pwd)/${CI_REPO_DIR:?}/.ci:/.ci" \
--volume "$(pwd)/${SOURCE_DIR:?}:/source" \
--workdir "/source" \
--name test-container \
unit-tests-image /.ci/build-kit/unit_tests.sh
- name: Integration tests
if: ${{ inputs.integration_tests || true }}
run: |
pushd source/.ci/e2e
docker-compose run e2e-test-server run-script tests
docker commit build-container integration-tests-image
docker-compose --file ${CI_REPO_DIR:?}/.ci/e2e/docker-compose.yaml \
run \
--volume "$(pwd)/${CI_REPO_DIR:?}/.ci:/.ci" \
--volume "$(pwd)/${SOURCE_DIR:?}:/source" \
--workdir "/source" \
e2e-test-server /.ci/e2e/scripts/tests.sh
- name: FIXME own all files in cache by current user and group
# FIXME Workaround for non-readable certs in `cache/josev/[ID]/Josev/iso15118/shared/pki/iso15118_2/certs . See https://github.com/EVerest/everest-core/actions/runs/7286554315/job/19855498928#step:13:10
if: always()
run: |
sudo chown --recursive $(id --user --name):$(id --user --name) ${CACHE_PATH}
- name: Store cache
uses: actions/cache/save@v3
if: always()
with:
path: ${{ env.CACHE_PATH }}
key: ${{ steps.restore-cache.outputs.cache-primary-key }}
- name: upload test results
if: always()
uses: actions/upload-artifact@v3
with:
name: ctest-report
path: ${{ env.SOURCE_DIR }}/build/Testing/Temporary/
lint:
name: Lint C++/C files
runs-on: ubuntu-latest
steps:
- name: Checkout repo ${{ inputs.repository || 'EVerest/everest-core' }}
uses: actions/checkout@v4
with:
repository: ${{ inputs.repository }}
ref: ${{ inputs.repository_ref }}
- name: Run clang-format
uses: everest/everest-ci/github-actions/[email protected]
with:
source-dir: . # FIXME Make sure `.` is the default to remove this line.
extensions: hpp,cpp
3 changes: 1 addition & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
*build
*build-cross
/build*/
.cache/
workspace.yaml
.vscode/
Loading