Disable fips jobs in PR and release builds, and don't run tests on both branch pushes and PRs #632
Conversation
The pulp org is limited to 20 concurrent jobs in Github Actions and we're starting to see queue times of 15+ minutes. These fips jobs start every time a change is pushed to a PR. I think it might make sense to disable them for PRs and releases and just rely on them running nightly [noissue]
| @@ -63,7 +86,7 @@ jobs: | |||
| if: matrix.test_type == 'release-upgrade' || matrix.test_type == 'source-upgrade' | |||
| run: | | |||
| docker pull quay.io/pulp/pulp-ci-dbuster:3.0.0 | |||
| docker pull quay.io/pulp/pulp_rpm-ci-f31:3.1.0 | |||
There was a problem hiding this comment.
A side effect of having these configs copied in 3 different places - we forget to update things everywhere.
| toxpy: "37" | ||
| ansible: "3.0" | ||
| python: "3.9" | ||
| toxpy: "39" |
There was a problem hiding this comment.
I'm not totally sure if the minor differences in version numbers between these configs is intentional or not, and I didn't cross check it with nightly and release.
There was a problem hiding this comment.
I think, this is to sprinkle some sparse entries into a huge parameter matrix.
However, python and toxpy must match.
|
WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue. |
|
FYI there's a 2nd commit with a description of the other changes that aren't in the main description here |
.github/workflows/ci.yml
Outdated
| name: Pulp PR | ||
| on: pull_request |
There was a problem hiding this comment.
Is this consistent with the plugin-template?
It's not consistent with the filename.
There was a problem hiding this comment.
Maybe I'm out of date but I thought the CI for the installer was totally independent from the plugin-template?
I can change the name to "Pulp CI" if that's what you mean
There was a problem hiding this comment.
I meant having only pull_request in ci.yml.
I see that it is more consistent with the plugin-template.
There was a problem hiding this comment.
There was some discussion about doing this (disabling CI on branch pushes) as well. No quorum on that decision but we can certainly discuss doing that.
| git-check: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v2.3.1 | ||
| - run: | | ||
| git fetch --prune --unshallow | ||
| - name: Set up Python | ||
| uses: actions/setup-python@v2 | ||
| - name: Install requests | ||
| run: pip install requests | ||
| - name: Check commit message | ||
| env: | ||
| GITHUB_CONTEXT: ${{ github.event.pull_request.commits_url }} | ||
| run: | | ||
| for sha in $(curl $GITHUB_CONTEXT | jq '.[].sha' | sed 's/"//g') | ||
| do | ||
| python .github/validate_commit_message.py $sha | ||
| VALUE=$? | ||
| if [ "$VALUE" -gt 0 ]; then | ||
| exit $VALUE | ||
| fi | ||
| done | ||
| shell: bash | ||
| - name: Check submodule status | ||
| run: | | ||
| .ci/submodule_check.sh |
There was a problem hiding this comment.
It's really hard to review this block of code because it is showing up as new code rather than a renamed pull_request.yml.
Suggestions on a different git diff command to run or something?
You might be able to do the delete in 1 commit, and then the rename/changes in the other.
(This assumes that we should be renaming the .yml file where the pull_request is.)
There was a problem hiding this comment.
Here's the sequence of events:
- Deleted the FIPS job from release + PR CI
- Deleted ci.yml
- Renamed pull_request.yml to ci.yml to be more consistent with other repos
I can re-do the PR with more commits if that would help
There was a problem hiding this comment.
I'm indifferent which approach whether we name it pull_request.yml or ci.yml , as long as you come up with some solution for me to review the changes made to pull_request.yml that became ci.yml .
I'm mostly certain, but not 100% certain, that redoing it as multiple commits like that will help.
| toxpy: "37" | ||
| ansible: "3.0" | ||
| python: "3.9" | ||
| toxpy: "39" |
The marginal utility of running tests on branches immediately after merging a PR where the tests had already been run is not zero, but it is very low. Let's reduce our CI consumption by getting rid of the duplication. [noissue]
The pulp org is limited to 20 concurrent jobs in Github Actions and
we're starting to see queue times of 15+ minutes. These fips jobs start
every time a change is pushed to a PR. I think it might make sense to
disable them for PRs and releases and just rely on them running nightly
[noissue]