Repository navigation
Conversation
…ith the delivered sc) don't build/test everything when only the "stream-connectors-ci" workflow is updated (it's only a dispatcher)
…(it was ok before)
635ffea to
73b37f5
Compare
tuntoja
left a comment
There was a problem hiding this comment.
Thanks for the follow-up. The orchestrator split, the single ci-status check and delivering the lib only after the connectors are tested against it all look good to me.
Three points, two with a suggestion you can apply directly:
- The lib is not delivered when only its version changes. After merge, unstable would get connectors requiring lib
>= 3.9.0while the lib there stays 3.8.1. - Dependency fixes of the connectors set in
stream-connectors.ymlare no longer delivered. - The connectors are delivered before the lib they depend on. On a release branch, testing can end up with connectors that cannot be installed.
Note for #409 / #410 (MON-205723), which will be rebased on this: the release tag trigger will move to stream-connectors-ci.yml, and .version.stream-connectors-lib will replace the version literal in point 1.
| lib_code: &lib_code | ||
| - modules/centreon-stream-connectors-lib/** | ||
| - packaging/connectors-lib/** |
There was a problem hiding this comment.
The library is not delivered when only its version changes.
The lib version is set in stream-connectors-lib.yml (version: "3.9.0"), which is only in the ci group. As a result, deliver_lib stays false when the version is bumped.
What happens when this PR is merged on develop:
stream-connectors-lib.ymlchanged, so the lib 3.9.0 is built and tested, but not delivered.packaging/connectors/**changed, so all the connectors are delivered to unstable, and they now requirecentreon-stream-connectors-lib >= 3.9.0.- Unstable ends up with connectors that cannot be installed, because the lib there is still 3.8.1. The run stays green, because the connectors are tested against the lib built in the same run.
Suggested fix: deliver the lib when its workflow changes. A CI-only change then re-delivers the same version to unstable, which is allowed there. #410 will later replace this line with a .version.stream-connectors-lib file.
| lib_code: &lib_code | |
| - modules/centreon-stream-connectors-lib/** | |
| - packaging/connectors-lib/** | |
| lib_code: &lib_code | |
| - modules/centreon-stream-connectors-lib/** | |
| - packaging/connectors-lib/** | |
| # the library version is set in its workflow: a version bump must deliver the library | |
| - .github/workflows/stream-connectors-lib.yml |
| # changes of the CI files are handled by stream-connectors-ci.yml (build_all_connectors input) | ||
| packaging: | ||
| - modified: .github/workflows/stream-connectors.yml | ||
| - added|modified: packaging/connectors/** |
There was a problem hiding this comment.
Dependency fixes of the connectors are no longer delivered.
This workflow is not only CI: it holds packaging data of the connectors. The "Add specific dependencies" step sets the rpm/deb dependencies of each connector (for example the libcurl4t64 fix for trixie), and the "Replace package and connector name variables" step sets the package names.
With this line removed, a fix in those steps is built and tested (through build_all_connectors) but never delivered: users only get it when a connector file also changes.
Suggested fix: keep the workflow in the packaging filter, so that a change here delivers every connector, as before #389. A later improvement would be to move the dependency map into packaging/connectors/, and then drop this line.
| # changes of the CI files are handled by stream-connectors-ci.yml (build_all_connectors input) | |
| packaging: | |
| - modified: .github/workflows/stream-connectors.yml | |
| - added|modified: packaging/connectors/** | |
| # the dependencies of the stream connectors packages are set in this workflow | |
| packaging: | |
| - modified: .github/workflows/stream-connectors.yml | |
| - added|modified: packaging/connectors/** |
| # the library is delivered only once the stream connectors have also been tested against it | ||
| stream-connectors-lib-delivery: | ||
| needs: [detect-changes, stream-connectors-lib, stream-connectors] | ||
| if: ${{ !cancelled() && needs.detect-changes.outputs.deliver_lib == 'true' && needs.stream-connectors-lib.result == 'success' && needs.stream-connectors.result != 'failure' }} |
There was a problem hiding this comment.
The connectors are delivered before the library they depend on.
The connectors are delivered inside the stream-connectors job (its deliver-* jobs), while the lib is delivered afterwards by stream-connectors-lib-delivery. When both change in the same run (as in this PR: lib 3.9.0, connectors requiring >= 3.9.0):
- the repositories hold connectors whose lib isn't published yet, at least for a while;
- if the lib delivery fails, they stay that way: unstable, or testing on a release branch, which QA installs from, contains connectors that cannot be installed.
Suggested fix: deliver the connectors after the lib, the same way this PR already splits the lib delivery.
- In
stream-connectors.yml, movedeliver-rpm,deliver-debanddeliver-pulpto a newstream-connectors-delivery.yml, and expose the list of connectors to deliver:
on:
workflow_call:
inputs:
# ... existing inputs
outputs:
delivered_connectors:
description: "JSON list of the stream connectors built in this run that must be delivered"
value: ${{ jobs.detect-changes.outputs.delivered_connectors }}stream-connectors-delivery.ymlholds the three jobs unchanged, except that the matrix reads the input. The cache keys built by thepackagejob still match within the run, as for the lib:
name: stream-connectors-delivery
# triggered by stream-connectors-ci.yml, once the library (if any) has been delivered
on:
workflow_call:
inputs:
connectors:
description: "JSON list of the stream connectors to deliver"
type: string
required: true
jobs:
get-environment:
uses: ./.github/workflows/get-environment.yml
deliver-rpm:
needs: [get-environment]
# ...
strategy:
matrix:
distrib: [el8, el9, el10]
connector_path: ${{ fromJson(inputs.connectors) }}
# deliver-deb / deliver-pulp: same change- Call it in this workflow, after the lib delivery, and only if the lib delivery didn't fail:
# the stream connectors are delivered after the library they depend on
stream-connectors-delivery:
needs: [stream-connectors, stream-connectors-lib-delivery]
if: ${{ !cancelled() && needs.stream-connectors.result == 'success' && needs.stream-connectors.outputs.delivered_connectors != '[]' && needs.stream-connectors-lib-delivery.result != 'failure' }}
uses: ./.github/workflows/stream-connectors-delivery.yml
with:
connectors: ${{ needs.stream-connectors.outputs.delivered_connectors }}
secrets: inherit
permissions:
contents: read
id-token: write
pull-requests: readAlso add stream-connectors-delivery to the ci filter and to the needs of ci-status.
Description
Fixes for CI after #389 review.
Type of change
Target serie
How this pull request can be tested ?
Please describe the procedure to verify that the goal of the PR is matched. Provide clear instructions so that it can be correctly tested.
Any relevant details of the configuration to perform the test should be added.
Checklist
Community contributors & Centreon team