ROX-21124: Move non-e2e testing OSCI jobs to GitHub Actions #1347

BradLugo · 2023-12-11T22:40:29Z

Description

These changes move the OpenShift CI jobs into GitHub Actions and change the updater to use an API token for grabbing NVD data.

Some interesting side-effects from this work:

We're able to download artifacts such as the genesis-dump directly from CI (i.e., without needing to pull the image)
We can run scanner code related CI (excluding e2e tests) without needing to wait for the data processing

openshift-ci · 2023-12-11T22:40:34Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

scripts/ci/lib.sh

.github/workflows/scheduled-build.yaml

ghost · 2023-12-19T19:51:04Z

Images are ready for the commit at 1c61044.

To use the images, use the tag 2.31.x-92-g1c61044e44.

RTann

THANK YOU for doing this. This is greatly appreciated

pkg/vulnloader/nvdloader/loader.go

RTann · 2023-12-20T01:48:09Z

scripts/ci/postgres.sh

+            adduser pg -u 1001 -g 1001 -d /var/lib/postgresql -s /bin/sh
+
+            # The PATH is not completely preserved, so set the PATH here to ensure postgres-related commands can be found.
+            runuser -l pg -c "PATH=$PATH $SCRIPTS_ROOT/scripts/ci/postgres.sh _start_postgres" # TODO(DO NOT MERGE): this is a mess


still a mess?

I don't love calling _start_postgres like this (I don't really even like the name tbh), but I don't fully understand idiomatic bash so 🤷. I also see some scripts use su <user> ... so I wasn't sure which should be used here. I'm fine to keep this as-is if y'all think it's fine

RTann · 2023-12-20T01:56:34Z

.github/workflows/build.yaml

+        uses: ./.github/actions/cache-go-dependencies
+
+      - name: Build Scanner
+        run: make scanner-build-nodeps


I don't remember this too well, but I remember this was created because of OpenShift CI oddities (no idea what). Do you think we still need to skip the deps step? I guess we've been skipping it for over a year by this point, so I guess it's ok

Honestly, I think scanner-build-nodeps is a bit of a misnomer. AFAICT, it still grabs the dependencies anyway. That being said, now that I look at the deps target, it might be worth adding that step first (since it has some other dependent targets). The main reason I used scanner-build-nodeps (without deps) was because that's how we do it in stackrox/stackrox

let's keep it as-is to stay aligned with stackrox/stackrox

.github/workflows/build.yaml

RTann · 2023-12-20T02:04:15Z

.github/workflows/build.yaml

+      - name: diff-dumps
+        run: ./scripts/ci/jobs/diff-dumps.sh
+
+  # TODO(DO NOT MERGE): These store functions don't seem quite right


what's wrong with them?

Basically what Gavin brought up here: https://github.com/stackrox/scanner/pull/1347/files#r1433028812
I'm not sure if these store functions are actually needed in GHA

They should be removed. The artifacts are stored earlier (and available through the GH UI) via upload-artifact.

scripts/ci/jobs/diff-dumps.sh

scripts/ci/lib.sh

RTann · 2023-12-20T02:09:18Z

Build / generate-db-dump (pull_request) Successful in 44m

I missed this

gavin-stackrox

I have some questions about testing which I'll bring up elsewhere.

gavin-stackrox · 2023-12-20T17:50:27Z

.github/workflows/build.yaml

+    env:
+      ARTIFACT_DIR: /artifacts


Can you make this a workflow env. i.e. only declare once: https://docs.github.com/en/actions/learn-github-actions/variables#defining-environment-variables-for-a-single-workflow

Can you make it /tmp/artifacts. In other environments we run with a RO root and limited user and using /tmp is a good practice.

gavin-stackrox · 2023-12-20T18:05:10Z

.github/workflows/build.yaml

+      - name: Create artifacts dir
+        run: mkdir -p "$ARTIFACT_DIR"


I took a deeper look at this ARTIFACT_DIR. I think you can remove it from this PR. It is only used in store_test_results and that returns for non openshift environments without doing anything.

The only reason to save those Junit test results would be to display them with something like https://github.com/test-summary/action which could be done in a follow on PR if warranted (I don't think there is enough ongoing dev in stackrox/scanner to warrant it IMHO).

I changed the implementation of store_test_results in this branch, but yeah, that's actually why I had comment at https://github.com/stackrox/scanner/pull/1347/files#diff-d0777657fa3fd81d23aaf7273e58aee453b04e67882517900c56daeef9b3e4c1R370. Would it make sense to remove all the usage of ARTIFACT_DIR and all the jobs that only call store_test_results?

Ah I see. I missed that. I'd say remove store_test_results() from any test/job that is moved to GHA. The jobs that remain in OSCI will still benefit from it.

gavin-stackrox · 2023-12-21T17:42:57Z

.github/workflows/scheduled-build.yaml

Can you remove this empty file, it is triggering an action failure.

BradLugo · 2023-12-21T19:28:17Z

fysa, upload-db-dump, upload-dumps-for-downstream, and upload-dumps-for-embedding are untested. Do y'all have any suggestions on how to test them?

.github/workflows/CI.yaml

scripts/ci/lib.sh

BradLugo · 2024-01-09T04:27:42Z

.github/workflows/ci.yaml

+          github.event_name != 'pull_request' ||
+          contains(github.event.pull_request.labels.*.name, 'generate-dumps-on-pr')


Could also be needs.generate-genesis-dump.result == 'success' maybe? Probably doesn't matter that much

BradLugo · 2024-01-09T04:58:03Z

Needs to be rebased once #1364 is merged.

RTann · 2024-01-10T19:34:13Z

@BradLugo is the genesis dump accessible to a user who would want to download it? When cutting a new release, we will need access to the latest one generated from the master branch. Just remembered this

can be done in a followup PR, if not already done here

BradLugo · 2024-01-10T20:56:12Z

@BradLugo is the genesis dump accessible to a user who would want to download it? When cutting a new release, we will need access to the latest one generated from the master branch. Just remembered this

@RTann Would this suffice? https://github.com/stackrox/scanner/actions/runs/7292017884

RTann · 2024-01-10T22:25:38Z

@BradLugo is the genesis dump accessible to a user who would want to download it? When cutting a new release, we will need access to the latest one generated from the master branch. Just remembered this

@RTann Would this suffice? https://github.com/stackrox/scanner/actions/runs/7292017884

yep!

BradLugo · 2024-01-11T18:13:21Z

/retest

BradLugo · 2024-01-11T18:16:49Z

/override ci/prow/images ci/prow/style-checks ci/prow/unit-tests

openshift-ci · 2024-01-11T18:16:57Z

@BradLugo: Overrode contexts on behalf of BradLugo: ci/prow/images, ci/prow/style-checks, ci/prow/unit-tests

In response to this:

/override ci/prow/images ci/prow/style-checks ci/prow/unit-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

BradLugo · 2024-01-11T18:18:06Z

All stackrox/stackrox tests passed: stackrox/stackrox#9146

This reverts commit b9dfecf.

BradLugo · 2024-01-11T19:58:36Z

/retest

openshift-ci · 2024-01-11T20:28:53Z

@BradLugo: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-tests	`2dbcd09`	link	false	`/test e2e-tests`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

BradLugo · 2024-01-11T21:12:00Z

e2e tests are fixed here: #1368

openshift-ci bot added the do-not-merge/work-in-progress label Dec 11, 2023

BradLugo force-pushed the blugo/ROX-21124-initial branch 4 times, most recently from 874f959 to 112788f Compare December 15, 2023 20:08

BradLugo changed the title ~~WIP~~ ROX-21124: Move non-e2e testing OSCI jobs to GitHub Actions Dec 15, 2023

BradLugo force-pushed the blugo/ROX-21124-initial branch from 14d3b5c to 7aca1c8 Compare December 15, 2023 23:03

BradLugo commented Dec 19, 2023

View reviewed changes

scripts/ci/lib.sh Show resolved Hide resolved

BradLugo force-pushed the blugo/ROX-21124-initial branch from 02ddd0c to 7e5f666 Compare December 19, 2023 19:34

BradLugo commented Dec 19, 2023

View reviewed changes

.github/workflows/scheduled-build.yaml Outdated Show resolved Hide resolved

BradLugo requested review from RTann and gavin-stackrox December 19, 2023 19:36

BradLugo marked this pull request as ready for review December 19, 2023 19:36

openshift-ci bot removed the do-not-merge/work-in-progress label Dec 19, 2023

RTann reviewed Dec 20, 2023

View reviewed changes

gavin-stackrox reviewed Dec 20, 2023

View reviewed changes

BradLugo self-assigned this Dec 20, 2023

This was referenced Dec 21, 2023

ROX-21124: Remove OSCI jobs that were moved to GHA openshift/release#47012

Merged

ROX-21124: Test scanner v2 images stackrox/stackrox#9146

Closed

gavin-stackrox reviewed Dec 21, 2023

View reviewed changes

.github/workflows/scheduled-build.yaml Outdated

Copy link

Contributor

gavin-stackrox Dec 21, 2023

Choose a reason for hiding this comment

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

Can you remove this empty file, it is triggering an action failure.

BradLugo requested review from RTann and gavin-stackrox December 21, 2023 18:43

BradLugo force-pushed the blugo/ROX-21124-initial branch from 41231dc to c0eecec Compare December 21, 2023 18:43

RTann reviewed Dec 21, 2023

View reviewed changes

.github/workflows/CI.yaml Outdated Show resolved Hide resolved

scripts/ci/lib.sh Show resolved Hide resolved

BradLugo commented Jan 9, 2024

View reviewed changes

BradLugo added 16 commits January 11, 2024 11:34

ROX-21124: Move non-e2e testing OSCI jobs to GitHub Actions

f3d5222

PR feedback

f9255f0

Remove use of ARTIFACT_DIR from GHA

c89ba34

Add remaining OSCI jobs

290ca33

Remove GHA unused workflow

c1de7fc

Add cron schedule for jobs

8c20baa

Move sanity-check-vuln-updates job

ee7cfcd

PR feedback

45de09d

Restrict some jobs and steps during PRs without proper label

e774283

Because GHA knows about the ancestory of the DAG for some reason

23f5a37

What happens without always()

c56cd8c

Revert "What happens without always()"

eef108d

This reverts commit b9dfecf.

fix some spacing

bc5e69a

PR feedback

94b58ca

fix some spacing

738e82a

proc image build

2dbcd09

BradLugo force-pushed the blugo/ROX-21124-initial branch from 1ec96a6 to 2dbcd09 Compare January 11, 2024 19:35

Remove DO NOT MERGE comment

290d0d3

BradLugo merged commit bda5a7e into master Jan 11, 2024
15 of 17 checks passed

BradLugo deleted the blugo/ROX-21124-initial branch January 11, 2024 21:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ROX-21124: Move non-e2e testing OSCI jobs to GitHub Actions #1347

ROX-21124: Move non-e2e testing OSCI jobs to GitHub Actions #1347

BradLugo commented Dec 11, 2023 •

edited

Loading

openshift-ci bot commented Dec 11, 2023

ghost commented Dec 19, 2023 •

edited by ghost

Loading

RTann left a comment

RTann Dec 20, 2023

BradLugo Dec 20, 2023

RTann Dec 20, 2023

BradLugo Dec 20, 2023

RTann Dec 21, 2023

RTann Dec 20, 2023

BradLugo Dec 20, 2023

gavin-stackrox Dec 20, 2023

RTann commented Dec 20, 2023

gavin-stackrox left a comment

gavin-stackrox Dec 20, 2023

gavin-stackrox Dec 20, 2023

BradLugo Dec 20, 2023

gavin-stackrox Dec 20, 2023

gavin-stackrox Dec 21, 2023

BradLugo commented Dec 21, 2023

BradLugo Jan 9, 2024

BradLugo commented Jan 9, 2024

RTann commented Jan 10, 2024

BradLugo commented Jan 10, 2024

RTann commented Jan 10, 2024

BradLugo commented Jan 11, 2024

BradLugo commented Jan 11, 2024

openshift-ci bot commented Jan 11, 2024

BradLugo commented Jan 11, 2024 •

edited

Loading

BradLugo commented Jan 11, 2024

openshift-ci bot commented Jan 11, 2024

BradLugo commented Jan 11, 2024

		github.event_name != 'pull_request' \|\|
		contains(github.event.pull_request.labels.*.name, 'generate-dumps-on-pr')

ROX-21124: Move non-e2e testing OSCI jobs to GitHub Actions #1347

ROX-21124: Move non-e2e testing OSCI jobs to GitHub Actions #1347

Conversation

BradLugo commented Dec 11, 2023 • edited Loading

Description

openshift-ci bot commented Dec 11, 2023

ghost commented Dec 19, 2023 • edited by ghost Loading

RTann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RTann commented Dec 20, 2023

gavin-stackrox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BradLugo commented Dec 21, 2023

Choose a reason for hiding this comment

BradLugo commented Jan 9, 2024

RTann commented Jan 10, 2024

BradLugo commented Jan 10, 2024

RTann commented Jan 10, 2024

BradLugo commented Jan 11, 2024

BradLugo commented Jan 11, 2024

openshift-ci bot commented Jan 11, 2024

BradLugo commented Jan 11, 2024 • edited Loading

BradLugo commented Jan 11, 2024

openshift-ci bot commented Jan 11, 2024

BradLugo commented Jan 11, 2024

BradLugo commented Dec 11, 2023 •

edited

Loading

ghost commented Dec 19, 2023 •

edited by ghost

Loading

BradLugo commented Jan 11, 2024 •

edited

Loading