[PH] Create workflow to test backward compatibility of performance harness #1356

oschwaldp-oci · 2023-06-30T14:49:12Z

Create workflow to test backward compatibility of performance harness with prior leap releases.

Introduces a reusable workflow build-base for the steps common to these workflows.

Resolves: #1156

… with prior leap releases.

This reverts commit d42f0e7.

spoonincode

I know this is still in draft but I have some maintenance concerns with where it's going.

spoonincode · 2023-07-05T21:34:59Z

.github/workflows/ph_backward_compatibility.yaml

+          uses: AntelopeIO/upload-artifact-large-chunks-action@v1
+          with:
+            name: ${{matrix.platform}}-build
+            path: build.tar.zst


We shouldn't be copy pasting the 3 jobs above. If we need a leap build to be performed as part of this workflow we should refactor the existing workflows so that building leap is a reusable workflow, that way it can be used from the per-PR workflow and from here.

Performing a build here may not strictly be required since you can download artifacts from other workflow runs, but that requires the user to manually perform a build with the other workflow (and waiting for it to complete) before calling this workflow.

@spoonincode - Starting to address this initially with a reusable workflow build-base. Would like to further refine to allow it to either do the build from scratch or, if the build exists, reuse the artifacts. Still strategizing how to make this work.

spoonincode · 2023-07-05T21:37:03Z

.github/workflows/ph_backward_compatibility.yaml

+        with:
+          owner: AntelopeIO
+          repo: leap
+          file: 'leap-.*-${{matrix.platform}}.04-x86_64.deb' # Ex. leap-3.1.4-ubuntu20.04-x86_64.deb


You might be able to combine these two if blocks via something like file: '(leap).*${{matrix.platform}}.04.*(x86_64|amd64).deb'

Thanks, good call. Addressed: c63042b

spoonincode · 2023-07-05T21:40:52Z

.github/workflows/ph_backward_compatibility.yaml

+      - name: Extract and Place Rev Leap Version artifacts
+        run: |
+          mkdir tmp
+          dpkg -x leap*.deb tmp


Why extract it instead of installing it? This won't be reliable across versions due to dependency changes. For example, let's say in 5.0 we no longer require gmp: it won't be in the base builder image any longer yet leap 4.0 requires that to be installed to run. Using the leap-4.0.deb resolves these for us.

Good call. Addressed: b7dea9b

spoonincode · 2023-07-05T21:42:56Z

.github/workflows/ph_backward_compatibility.yaml

        run: |
-          echo "Workflow Stub"
+          cd build
+          ctest --output-on-failure -R performance_test_basic_read_only_trxs --timeout 480


Having all of these manually defined like this is at risk to forgetting to add one later. Maybe we should have a new label for performance tests, or maybe just -R performance_test is enough.

Collapsing these back down into a regular expression is the plan. Was splitting them out to get clearer information when a test failed during initial debugging. Although it is going to make skipping tests of newer features more difficult, where older versions of nodeos aren't expected to succeed.

I believe I can do this with a mix of -R and -E depending on nodeos version.

Addressed: 800a37e

cleos time-limit option was added in 3.2 but is not in 3.1 so need to be able to differentiate on minor version number.

… prior releases.

…orted command line options in earlier versions.

…duce future maintenance

… those nodeos and cleos versions.

Make use of reusable workflow in ph_backward_compatibility workflow.

spoonincode · 2023-07-12T20:09:58Z

.github/workflows/build.yaml

-          password: ${{secrets.GITHUB_TOKEN}}
-          package-name: builders
+  build-base:
+    uses: AntelopeIO/leap/.github/workflows/build_base.yaml@GH-1156-ph-cicd-nodeos-versions


I think this should just be

uses: .github/workflows/build_base.yaml

Hardcoding AntelopeIO and/or leap breaks forks/mirrors, and we shouldn't be hardcoding the branch -- it should just use the same ref as the caller

Addressed: 477f62c and 0da3e7b

spoonincode · 2023-07-12T20:14:18Z

.github/workflows/build.yaml

@@ -23,28 +23,17 @@ on:
        type: string

 permissions:
-  packages: read
+  packages: write


I would need to refresh my memory on how permissions & reusable workflows work to know for sure.. but can we leave this to read and only plumb through the write permission on the resusable workflow call? That is, can we do something like

build-base: uses: .github/workflows/build_base.yaml permissions: packages: write contents: read

Good call. Finally got that to work. Had tried a couple other options before and didn't get the right combination it seems. Addressed: 495d6ea and 4e19e36

…build-base.

spoonincode

just one permission nit, otherwise it seems good best I can tell

spoonincode · 2023-07-13T21:03:09Z

.github/workflows/build_base.yaml

+        value: ${{ jobs.d.outputs.p }}
+
+permissions:
+  packages: write


I think this should just be read, since below the build-platforms job has

permissions: packages: write contents: read

So I think as long as build_base.yaml is called with packages: write, which it seems to be:

leap/.github/workflows/build.yaml

Lines 34 to 38 in 4e19e36

build-base:

uses: ./.github/workflows/build_base.yaml

permissions:

packages: write

contents: read

Having packages: read in this location should cause all jobs in this workflow other then build-platform be read, where build-platform is write.

(btw your ph_backward_compatibility.yaml already does this)

leap/.github/workflows/ph_backward_compatibility.yaml

Lines 6 to 8 in 4e19e36

permissions:

packages: read

contents: read

Good catch. Slipped past when I was changing the others. I've corrected that now.

spoonincode · 2023-07-14T02:26:11Z

.github/workflows/build.yaml

-          platform-file: .cicd/platforms.json
-          password: ${{secrets.GITHUB_TOKEN}}
-          package-name: builders
+  build-base:


Since this job (that uses the reusable workflow) doesn't have a name it seems to show up as simply build-base in the GUI in various places. I'm a little surprises it doesn't inherit the callee workflow name. Maybe we could consider giving this a name (even just Build) to show up nicer.

Updated with name Run Build Workflow:

Create workflow to test backward compatibility of performance harness…

fe05332

… with prior leap releases.

oschwaldp-oci added the OCI Work exclusive to OCI team label Jun 30, 2023

oschwaldp-oci self-assigned this Jun 30, 2023

oschwaldp-oci marked this pull request as draft June 30, 2023 14:56

Base automatically changed from GH-1156-initial-ph-cicd-workflow to main June 30, 2023 18:18

oschwaldp-oci added 13 commits June 30, 2023 13:21

Merge branch 'main' into GH-1156-ph-cicd-nodeos-versions

c42f25b

Try to placate different version artifact naming conventions.

f848de5

missed the extract step

3a72042

Do not run tests in parallel.

2e90e39

Use low-tier for longer run time

cd69265

Split up tests into individual runs.

a2aa513

Tests not found - move change dir build into first test.

258fefc

Add regex flag to ctest.

99d2316

Give tests a little longer to complete.

91cf4db

Split test scenarios into separate jobs.

d42f0e7

Revert "Split test scenarios into separate jobs."

58b0f32

This reverts commit d42f0e7.

Always run test scenarios.

5499dd0

make sure to change to the build dir.

ea37b69

spoonincode requested changes Jul 5, 2023

View reviewed changes

oschwaldp-oci added 4 commits July 10, 2023 09:00

Use full nodeosVersion.

3a0960e

cleos time-limit option was added in 3.2 but is not in 3.1 so need to be able to differentiate on minor version number.

Read only transaction feature support was added in 4.0, don't test in…

441f5d3

… prior releases.

Read only transaction feature support was added in 4.0, remove unsupp…

e6ea54b

…orted command line options in earlier versions.

Fix multiple if statements in steps.

8cb771a

spoonincode mentioned this pull request Jul 10, 2023

[PH] Create workflow for manually dispatching performance harness runs #1378

Merged

oschwaldp-oci added 6 commits July 10, 2023 11:33

Shouldn't run read only tests in if < v4.0

ff1a837

Collapse perf tests back into regular expressions to simplifly and re…

800a37e

…duce future maintenance

Try reducing steps using regex for deb name matching across versions.

c63042b

Try removing workaround, as it shouldn't be necessary here.

cb447c5

Install the leap deb package to bring along dependencies and then use…

b7dea9b

… those nodeos and cleos versions.

Working to fix install of leap deb pkg.

bc46133

oschwaldp-oci added 9 commits July 10, 2023 14:15

Sanity check nodeos version.

265ee66

Merge branch 'main' into GH-1156-ph-cicd-nodeos-versions

1a3ef7d

Try breaking out build steps into reusable workflow.

4f158de

Make use of reusable workflow in ph_backward_compatibility workflow.

Needs to use this branch's reusable workflow for now, not main

8044aa1

Inherit secrets.

de40f8a

Provide packages write permission for Build Platforms job.

7df3af7

Give packages write permissions.

cf7ae29

Convert build.yaml to use reusable build-base workflow.

7be77cd

Merge branch 'main' into GH-1156-ph-cicd-nodeos-versions

d46a16a

oschwaldp-oci marked this pull request as ready for review July 11, 2023 21:05

oschwaldp-oci requested review from heifner and spoonincode July 11, 2023 21:05

Merge branch 'main' into GH-1156-ph-cicd-nodeos-versions

8eb51fc

oschwaldp-oci changed the title ~~Create workflow to test backward compatibility of performance harness~~ [PH] Create workflow to test backward compatibility of performance harness Jul 12, 2023

spoonincode reviewed Jul 12, 2023

View reviewed changes

oschwaldp-oci added 6 commits July 12, 2023 15:15

Remove hardcoded org and repo and ref.

477f62c

Merge branch 'main' into GH-1156-ph-cicd-nodeos-versions

e0b3caa

Try to rework permissions to leave top level read, but pass write to …

495d6ea

…build-base.

Fix path to workflow.

0da3e7b

Merge branch 'main' into GH-1156-ph-cicd-nodeos-versions

c3db754

Somehow put the wrong permission here.

4e19e36

spoonincode reviewed Jul 13, 2023

View reviewed changes

oschwaldp-oci added 2 commits July 13, 2023 16:14

fix permission.

56c0027

Merge branch 'main' into GH-1156-ph-cicd-nodeos-versions

b59fede

spoonincode approved these changes Jul 14, 2023

View reviewed changes

Add names to jobs for better UI experience.

bfa1685

heifner approved these changes Jul 14, 2023

View reviewed changes

oschwaldp-oci merged commit d243f85 into main Jul 14, 2023

oschwaldp-oci deleted the GH-1156-ph-cicd-nodeos-versions branch July 14, 2023 19:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PH] Create workflow to test backward compatibility of performance harness #1356

[PH] Create workflow to test backward compatibility of performance harness #1356

oschwaldp-oci commented Jun 30, 2023 •

edited

Loading

spoonincode left a comment

spoonincode Jul 5, 2023

oschwaldp-oci Jul 11, 2023

spoonincode Jul 5, 2023

oschwaldp-oci Jul 10, 2023

spoonincode Jul 5, 2023

oschwaldp-oci Jul 10, 2023

spoonincode Jul 5, 2023

oschwaldp-oci Jul 10, 2023 •

edited

Loading

oschwaldp-oci Jul 10, 2023

oschwaldp-oci Jul 10, 2023

spoonincode Jul 12, 2023

oschwaldp-oci Jul 12, 2023

spoonincode Jul 12, 2023

oschwaldp-oci Jul 12, 2023 •

edited

Loading

spoonincode left a comment

spoonincode Jul 13, 2023

oschwaldp-oci Jul 13, 2023

spoonincode Jul 14, 2023

oschwaldp-oci Jul 14, 2023

	build-base:
	uses: ./.github/workflows/build_base.yaml
	permissions:
	packages: write
	contents: read

[PH] Create workflow to test backward compatibility of performance harness #1356

[PH] Create workflow to test backward compatibility of performance harness #1356

Conversation

oschwaldp-oci commented Jun 30, 2023 • edited Loading

spoonincode 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

oschwaldp-oci Jul 10, 2023 • edited Loading

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

oschwaldp-oci Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

spoonincode 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

oschwaldp-oci commented Jun 30, 2023 •

edited

Loading

oschwaldp-oci Jul 10, 2023 •

edited

Loading

oschwaldp-oci Jul 12, 2023 •

edited

Loading