Add the feature of showing useful error message during CI #1083

RyanL1997 · 2022-08-24T16:45:19Z

Signed-off-by: Ryan Liang [email protected]

Description

Create a separate workflow and implemented environment variable of version to check for the existence of the OpenSearch Security Plugin artifact

Issues Resolved

Resolved [Feature] Provide useful errors and next steps when this plugins dependencies are out of alignment #1073

Testing

~~CI test case#1 with "403 Forbidden" response~~
~~CI test case#2 with "404 Not Found" response~~
Created a new workflow for "Verify there is matching build of security plugin" by running prerequisites-test

Check List

~~New functionality includes testing~~
~~New functionality has been documented~~
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov-commenter · 2022-08-24T17:24:29Z

Codecov Report

Merging #1083 (cac0c5e) into main (7299822) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1083   +/-   ##
=======================================
  Coverage   72.27%   72.27%           
=======================================
  Files          87       87           
  Lines        1915     1915           
  Branches      244      244           
=======================================
  Hits         1384     1384           
  Misses        478      478           
  Partials       53       53

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

cwperks · 2022-08-24T18:07:58Z

.github/workflows/integration-test.yml

@@ -16,6 +16,9 @@ jobs:
          wget https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.3.0/latest/linux/x64/tar/builds/opensearch/dist/opensearch-min-2.3.0-linux-x64.tar.gz
          tar -xzf opensearch-*.tar.gz
          rm -f opensearch-*.tar.gz
+
+      - name: Check the validation of OpenSearch Security Plugin
+        run: wget -S --spider https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.3.0/latest/linux/x64/tar/builds/opensearch/plugins/opensearch-security-2.3.0.0.zip || (echo "Please make sure security plugin has been bumped to the same version and added to manifest." && exit 1)


Is it possible to create variables in the Github actions files? We use the version in a number of places and it would be nice to define it once.

Hi @cwperks. Thanks for the reply. I think we can only set up the environment variables in the Github actions files. I can definitely try to set it up in the environment variable to see if it works.

peternied · 2022-08-24T18:09:45Z

.github/workflows/integration-test.yml

@@ -16,6 +16,9 @@ jobs:
          wget https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.3.0/latest/linux/x64/tar/builds/opensearch/dist/opensearch-min-2.3.0-linux-x64.tar.gz
          tar -xzf opensearch-*.tar.gz
          rm -f opensearch-*.tar.gz
+
+      - name: Check the validation of OpenSearch Security Plugin


This would fail the integration-test workflow, what would you think about creating a separate workflow called something like prerequisites that could have a task named "Verify there is matching build of security plugin"

Hi @peternied. Thanks for the reply! The reason I set to check it up here because if this one fails, the next test of Download OpenSearch Security Plugin will also be failed and break the integration-test. I was planning to use this test only as the prerequisite for checking the test of Download OpenSearch Security Plugin. Sorry for the confusions anyways. Please correct me if I miss some of the steps.

Here is the UI that our users would see on failures with the current design, might be useful to have this spelled out more plainly (Less clicks is better!)

I think I get it now! Ya I will create a new work flow with a better name for running this separately! Thanks @peternied!

RyanL1997 · 2022-08-24T20:06:56Z

He guys, I have made the update of this PR:

Instead of directly to implement the test in integration-test, I created a separate workflow called prerequisite-test.
Implemented the environment variable of version numbers in prerequisite-test
Seems like it is also possible to setup the environment variable of version in the Integration-test. However, we may need another PR for doing that if it is necessary.

.github/workflows/integration-test.yml

cliu123 · 2022-08-24T20:16:50Z

@RyanL1997 Would you please add the new behavior in the PR description?

.github/workflows/integration-test.yml

cwperks · 2022-08-24T20:35:34Z

.github/workflows/prerequisite-test.yml

+    name: Run prerequisite tests
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check the validation of OpenSearch Security Plugin


The name of this should reflect what the check is performing. How does "Check for the existence of the OpenSearch Security Plugin artifact" sound?

Sure, I have updated the commit and also changed the PR description with that. Thx for the review @cwperks!

cwperks · 2022-08-24T20:48:16Z

.github/workflows/prerequisite-test.yml

@@ -0,0 +1,14 @@
+name: Verify there is matching build of security plugin


This name appears in the checks like this:

Shorten this to "Prerequisite Checks"?

cwperks · 2022-08-24T20:48:54Z

.github/workflows/prerequisite-test.yml

+
+jobs:
+  tests:
+    name: Run prerequisite tests


Similar to above: maybe "Run prerequisite checks"?

Thanks! I have changed the name for both test's name and the sub tests' name. Please review it.

…sion variable Signed-off-by: Ryan Liang <[email protected]>

…sion variable (#1083) Signed-off-by: Ryan Liang <[email protected]> (cherry picked from commit 964e804)

…sion variable (#1083) (#1085) Signed-off-by: Ryan Liang <[email protected]> (cherry picked from commit 964e804) Co-authored-by: Ryan Liang <[email protected]>

RyanL1997 force-pushed the Add-error-msg-to-CI branch 3 times, most recently from 045ea0b to 6d689bc Compare August 24, 2022 17:22

RyanL1997 marked this pull request as ready for review August 24, 2022 17:56

RyanL1997 requested a review from a team August 24, 2022 17:56

cliu123 previously approved these changes Aug 24, 2022

View reviewed changes

cwperks reviewed Aug 24, 2022

View reviewed changes

peternied reviewed Aug 24, 2022

View reviewed changes

RyanL1997 dismissed cliu123’s stale review via e4e5ec9 August 24, 2022 19:04

RyanL1997 force-pushed the Add-error-msg-to-CI branch from 5a346cf to cf9334e Compare August 24, 2022 19:18

RyanL1997 marked this pull request as draft August 24, 2022 19:22

RyanL1997 force-pushed the Add-error-msg-to-CI branch 3 times, most recently from 63d3b75 to 7a7531e Compare August 24, 2022 19:49

RyanL1997 marked this pull request as ready for review August 24, 2022 19:58

cliu123 reviewed Aug 24, 2022

View reviewed changes

.github/workflows/integration-test.yml Outdated Show resolved Hide resolved

RyanL1997 force-pushed the Add-error-msg-to-CI branch from 7a7531e to ace1a7b Compare August 24, 2022 20:22

cliu123 reviewed Aug 24, 2022

View reviewed changes

.github/workflows/integration-test.yml Outdated Show resolved Hide resolved

RyanL1997 force-pushed the Add-error-msg-to-CI branch from ace1a7b to 98cfda1 Compare August 24, 2022 20:29

cwperks reviewed Aug 24, 2022

View reviewed changes

RyanL1997 force-pushed the Add-error-msg-to-CI branch from 98cfda1 to 3c01353 Compare August 24, 2022 20:41

cwperks reviewed Aug 24, 2022

View reviewed changes

RyanL1997 force-pushed the Add-error-msg-to-CI branch from 3c01353 to cac0c5e Compare August 24, 2022 20:58

cliu123 previously approved these changes Aug 24, 2022

View reviewed changes

Add the feature of showing useful error message during tests with ver…

b62d5cd

…sion variable Signed-off-by: Ryan Liang <[email protected]>

RyanL1997 dismissed cliu123’s stale review via b62d5cd August 24, 2022 21:30

RyanL1997 force-pushed the Add-error-msg-to-CI branch from cac0c5e to b62d5cd Compare August 24, 2022 21:30

cwperks approved these changes Aug 24, 2022

View reviewed changes

cliu123 approved these changes Aug 24, 2022

View reviewed changes

cliu123 merged commit 964e804 into opensearch-project:main Aug 24, 2022

cliu123 added the backport 2.x backport to 2.x branch label Aug 24, 2022

opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 24, 2022

Add the feature of showing useful error message during tests with ver…

34f2967

…sion variable (#1083) Signed-off-by: Ryan Liang <[email protected]> (cherry picked from commit 964e804)

opensearch-trigger-bot bot mentioned this pull request Aug 24, 2022

[Backport 2.x] Add the feature of showing useful error message during CI #1085

Merged

RyanL1997 deleted the Add-error-msg-to-CI branch December 6, 2022 19:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the feature of showing useful error message during CI #1083

Add the feature of showing useful error message during CI #1083

RyanL1997 commented Aug 24, 2022 •

edited

Loading

codecov-commenter commented Aug 24, 2022 •

edited

Loading

cwperks Aug 24, 2022

RyanL1997 Aug 24, 2022 •

edited

Loading

peternied Aug 24, 2022

RyanL1997 Aug 24, 2022 •

edited

Loading

peternied Aug 24, 2022 •

edited

Loading

RyanL1997 Aug 24, 2022

RyanL1997 commented Aug 24, 2022

cliu123 commented Aug 24, 2022

cwperks Aug 24, 2022

RyanL1997 Aug 24, 2022

cwperks Aug 24, 2022

cwperks Aug 24, 2022

RyanL1997 Aug 24, 2022

		@@ -0,0 +1,14 @@
		name: Verify there is matching build of security plugin

Add the feature of showing useful error message during CI #1083

Add the feature of showing useful error message during CI #1083

Conversation

RyanL1997 commented Aug 24, 2022 • edited Loading

Description

Issues Resolved

Testing

Check List

codecov-commenter commented Aug 24, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

RyanL1997 Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyanL1997 Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

peternied Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyanL1997 commented Aug 24, 2022

cliu123 commented Aug 24, 2022

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

RyanL1997 commented Aug 24, 2022 •

edited

Loading

codecov-commenter commented Aug 24, 2022 •

edited

Loading

RyanL1997 Aug 24, 2022 •

edited

Loading

RyanL1997 Aug 24, 2022 •

edited

Loading

peternied Aug 24, 2022 •

edited

Loading