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: Fix apt-get command in bump.yml, restore workflow summaries #101

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Dec 2, 2024

The bump.yml workflow, running on a schedule, has always failed to complete so far. This is because an "apt-get" command is missing the "install" subcommand, resulting in apt trying to interpret the name of the package we want to install as a subcommand, making the workflow fail.

Fix the command, to hopefully make the workflow succeed.

The bump.yml workflow, running on a schedule, has always failed to
complete so far [0]. This is because an "apt-get" command is missing the
"install" subcommand, resulting in apt trying to interpret the name of
the package we want to install as a subcommand, making the workflow
fail.

Fix the command, to hopefully make the workflow succeed.

[0] https://github.com/githedgehog/dataplane/actions/workflows/bump.yml

Fixes: 08ac167 ("build: Rework many parts of the build framework")
Signed-off-by: Quentin Monnet <[email protected]>
@qmonnet qmonnet added the ci Continuous Integration label Dec 2, 2024
@qmonnet qmonnet requested a review from a team as a code owner December 2, 2024 14:39
@qmonnet qmonnet requested review from daniel-noland and removed request for a team December 2, 2024 14:39
@qmonnet
Copy link
Member Author

qmonnet commented Dec 2, 2024

Oh no, it looks like skipping jobs entirely prevents GitHub from creating the various status check entries with the Rust toolchain versions (stable etc.), keeping instead a generic ${{ matrix.rust }} in the job name, and preventing the requirement on the checks to be satisfied 🤦

@qmonnet
Copy link
Member Author

qmonnet commented Dec 2, 2024

We can bypass the rule here, but I'll need to find a way to address this. Not sure what's the cleanest solution 😕

This reverts commit 6d69a4e.

I thought we could have the individual jobs marked as required. But we
hit the following issue:

Individual jobs (build, test, push in dev.yml and sterile.yml) use a
matrix to run with multiple Rust toolchains (stable/nightly). The Rust
toolchain appears in the job name, whether we specify it or not. Then
when trying to fill the list of required checks in GitHub's interface,
we've got to use these names and as far as I can tell this must be an
exact match, so we need to specify the toolchain. So far this looks good
because ideally we'd like to have the checks using the stable toolchain,
not nightly, as required. However, when the job is skipped, because no
relevant files are touched in the Pull Request, the matrix is not
generated, and the toolchain name is not appended to the job name as one
might expect. In such a case, it's no longer possible to match the
required checks with the titles of the status checks for the Pull
Request, and the Pull Request is blocked. Let's re-add the summaries so
we can mark them as required.

Signed-off-by: Quentin Monnet <[email protected]>
GitHub adds the values from the matrix to the job titles by default if
they're similar between concurrent jobs, there is no need to specify
them manually.

Signed-off-by: Quentin Monnet <[email protected]>
@qmonnet
Copy link
Member Author

qmonnet commented Dec 3, 2024

I couldn't find a simple way to mark the jobs with the stable toolchain as required. I couldn't find an easy way to mark the individual jobs as required, either: as their names contain the toolchain name obtained from the matrix, I don't think we have a solution to mark them as required and have the requirement satisfied when the jobs are skipped (and the matrix not generated).

So instead, I re-added the job summaries that we had before, so we can mark these as required. We don't get the distinction between the toolchains, but we could expand the summary to retrieve some output specific to each job if we really want to. That doesn't look super clean, but should work.

@qmonnet qmonnet changed the title ci: Fix apt-get command in bump.yml ci: Fix apt-get command in bump.yml, restore workflow summaries Dec 3, 2024
Copy link
Collaborator

@daniel-noland daniel-noland left a comment

Choose a reason for hiding this comment

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

LGTM

@qmonnet qmonnet added this pull request to the merge queue Dec 3, 2024
Merged via the queue into main with commit c6f2312 Dec 3, 2024
15 checks passed
@qmonnet qmonnet deleted the pr/qmonnet/bump-fix branch December 3, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants