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

DAOS-16262 build: Add weekly bullseye build and test GHA #15287

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

phender
Copy link
Contributor

@phender phender commented Oct 9, 2024

Automate weekly build with Bullseye and VM Functionality test.

Skip-unit-tests: true
Skip-fault-injection-test: true
Skip-test: true

Required-githooks: true

Before requesting gatekeeper:

  • Two review approvals and any prior change requests have been resolved.
  • Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
  • Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
  • Commit messages follows the guidelines outlined here.
  • Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

  • You are the appropriate gatekeeper to be landing the patch.
  • The PR has 2 reviews by people familiar with the code, including appropriate owners.
  • Githooks were used. If not, request that user install them and check copyright dates.
  • Checkpatch issues are resolved. Pay particular attention to ones that will show up on future PRs.
  • All builds have passed. Check non-required builds for any new compiler warnings.
  • Sufficient testing is done. Check feature pragmas and test tags and that tests skipped for the ticket are run and now pass with the changes.
  • If applicable, the PR has addressed any potential version compatibility issues.
  • Check the target branch. If it is master branch, should the PR go to a feature branch? If it is a release branch, does it have merge approval in the JIRA ticket.
  • Extra checks if forced landing is requested
    • Review comments are sufficiently resolved, particularly by prior reviewers that requested changes.
    • No new NLT or valgrind warnings. Check the classic view.
    • Quick-build or Quick-functional is not used.
  • Fix the commit message upon landing. Check the standard here. Edit it to create a single commit. If necessary, ask submitter for a new summary.

Automate weekly build with Bullseye and VM Functionality test.

Skip-unit-tests: true
Skip-fault-injection-test: true
Skip-test: true

Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
@phender phender requested a review from dinghwah October 9, 2024 20:09
Copy link

github-actions bot commented Oct 9, 2024

Ticket title is 'Automate weekly build with Bullseye with simple VM functional test'
Status is 'In Review'
https://daosio.atlassian.net/browse/DAOS-16262

.github/workflows/code-coverage-test.yml Fixed Show fixed Hide fixed
.github/workflows/code-coverage-test.yml Fixed Show fixed Hide fixed
.github/workflows/code-coverage-test.yml Fixed Show fixed Hide fixed
.github/workflows/code-coverage-test.yml Fixed Show fixed Hide fixed
.github/workflows/code-coverage-test.yml Fixed Show fixed Hide fixed
.github/workflows/code-coverage-test.yml Fixed Show fixed Hide fixed
.github/workflows/code-coverage-test.yml Fixed Show fixed Hide fixed
.github/workflows/code-coverage-test.yml Fixed Show fixed Hide fixed
.github/workflows/code-coverage-test.yml Fixed Show fixed Hide fixed
JENKINS_URL: https://build.hpdd.intel.com/

on:
pull_request: # Remove after testing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running with this for now for testing. We don't want to run this on every PR so the plan is to remove this line in the final push.

@brianjmurrell
Copy link
Contributor

Ha ha. I just added a couple of comments to #15206 which seems to be some kind of copy of this one?

Copy link
Contributor

@brianjmurrell brianjmurrell left a comment

Choose a reason for hiding this comment

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

Great to see this being added to GHA instead of Jenkins! Awesome job on jumping in and doing it this way!

- cron: "0 0 * * 6" # Every Saturday

concurrency:
group: code-coverage-${{ github.head_ref || github.run_id }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Good, you caught this one then.

@brianjmurrell brianjmurrell self-requested a review October 9, 2024 20:42
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned elsewhere, it's a shame to see this being an almost duplicate of https://github.com/daos-stack/daos/blob/master/.github/workflows/rpm-build-and-test.yml and while I have been working on workflow reusability, I wonder if whatever is different in this copy can be handled in the build and functional test matrices so that all of this can be merged with https://github.com/daos-stack/daos/blob/master/.github/workflows/rpm-build-and-test.yml.

The matrix ability of GHA seems to be quite a bit better than Jenkins.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it would ideal not have to reproduce code. It sounds like composite actions is what we want. Let me try adding some for building RPMs and running Functional (VM) tests.

Copy link
Contributor

@brianjmurrell brianjmurrell Oct 15, 2024

Choose a reason for hiding this comment

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

Yes it would ideal not have to reproduce code. It sounds like composite actions is what we want.

Yes, it is. That is already implemented in #14177 along with a number of other RPMs in other repos.

Since I am in the process of finalizing that PR and it's associates and getting ready to land them, would it be possible for you to rebase this PR on #14177 and it's associated PRs to further validate the concepts that I am trying to employ there? This would be a poster-child for that. Indeed, it might be that some of the work in this PR (I'm thinking splitting out the Functional test jobs for example) needs to be pushed up into #14177 and it's friends and it would be nicer to know that before landing it all.

If you don't have the time, I can fork this PR and do the rebase/validation to see what needs changing in my PRs to support it to further flesh those out.

Copy link
Contributor

@brianjmurrell brianjmurrell Oct 15, 2024

Choose a reason for hiding this comment

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

All of that said, I was actually looking for a solution along the lines of not [re-]creating any more workflows of the same kind of build+test. I was/am hoping we can use the same single workflow for all of the scheduled testing just by permuting within the one single workflow.

This seems useful in that regard.

Although, I was hoping that https://github.com/daos-stack/daos/actions would have greater flexibility than Jenkins does at being able to filter to specific types of runs but it seems it doesn't get more granular than event:schedule.

So that is, we can get a bit better than Jenkins and isolate the landings to a branch (i.e. master from the scheduled runs but we cannot isolate each different schedule and given we might have no choice but to run them on master. 😦

That leaves release/* as being an even bigger problem.

We might have to use reusable workflows that checkout the branch to be tested. 😠

Copy link

github-actions bot commented Oct 9, 2024

Functional on EL 8.8 Test Results (old)

131 tests   127 ✅  1h 36m 44s ⏱️
 41 suites    4 💤
 41 files      0 ❌

Results for commit 0905646.

Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
@daosbuild1
Copy link
Collaborator

Test stage Unit Test on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15287/4/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15287/4/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15287/4/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test with memcheck on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15287/4/display/redirect

Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
on:
pull_request: # Remove after testing
schedule:
- cron: "0 0 * * 6" # Every Saturday
Copy link
Contributor

Choose a reason for hiding this comment

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

If the intention is to run this on a branch other than master like we do with Jenkins, I think this restriction:

  • Scheduled workflows will only run on the default branch.

from https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#schedule might be a problem.

Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
statuses: write
steps:
- name: Checkout code
uses: actions/checkout@v4.1.1

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 1: GitHub-owned GitHubAction not pinned by hash
Click Remediation section below to solve this issue
Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
@daosbuild1
Copy link
Collaborator

Test stage Unit Test with memcheck on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15287/7/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15287/7/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage NLT on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15287/7/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15287/7/display/redirect

Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
needs.build-rpms.result == 'skipped')
steps:
- name: Checkout code
uses: actions/checkout@v4.1.1

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium

score is 1: GitHub-owned GitHubAction not pinned by hash
Click Remediation section below to solve this issue
Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
Copy link

github-actions bot commented Oct 18, 2024

Functional on EL 8.6 Test Results (old)

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit cc7116a.

♻️ This comment has been updated with latest results.

Required-githooks: true

Signed-off-by: Phil Henderson <phillip.henderson@intel.com>
Comment on lines +137 to +142
- name: Publish RPMs
uses: actions/upload-artifact@v4
with:
# yamllint disable-line rule:line-length
name: ${{ steps.build-vars.outputs.distro_name }} ${{ steps.build-vars.outputs.distro_version }} RPM repository
path: ${{ inputs.REPO_PATH}}${{ github.run_number }}/artifact/artifacts/${{ inputs.distro }}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can't do this. Per SDL, we are not allowed to distribute unsigned binaries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the same code as https://github.com/daos-stack/daos/blob/master/.github/workflows/rpm-build-and-test.yml#L301-L305. Doesn't it just add the RPMs to Artifactory so that the functional test stages can install those RPMs?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the same code as master/.github/workflows/rpm-build-and-test.yml#L301-L305.

Which is being replaced by https://github.com/daos-stack/actions-lib/pull/1/files#diff-46a90c32d59a22431144045d82ce732bc5a498ea20229b697bc2fd2154899662R423-R428 and https://github.com/daos-stack/daos/pull/14177/files#diff-681a2eb462568f244d2fd51eff96c3d9945e33dec02ee489c680a742a0bdcef4R79-R85 which you can see comments out that upload.

Doesn't it just add the RPMs to Artifactory so that the functional test stages can install those RPMs?

No. That uploads them to the artifacts in GitHub, where anyone can download them. This is the part that puts us off-side of the SDL.

Moreover, we don't put RPMs from PRs into Artifactory. With Jenkins, we install them from the artifacts of the Jenkins job itself and in GHA they are stored in this location https://github.com/daos-stack/daos/pull/14177/files#diff-681a2eb462568f244d2fd51eff96c3d9945e33dec02ee489c680a742a0bdcef4R79-R85 by ci/rpm/create_repo.sh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants