-
Notifications
You must be signed in to change notification settings - Fork 40
Add Basic Jenkins Integ Test for Demo Setup #1099
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
Conversation
Signed-off-by: Tanner Lewis <[email protected]>
Signed-off-by: Tanner Lewis <[email protected]>
Signed-off-by: Tanner Lewis <[email protected]>
Signed-off-by: Tanner Lewis <[email protected]>
Signed-off-by: Tanner Lewis <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1099 +/- ##
============================================
- Coverage 80.66% 80.55% -0.11%
- Complexity 2893 2930 +37
============================================
Files 383 390 +7
Lines 14361 14462 +101
Branches 989 998 +9
============================================
+ Hits 11584 11650 +66
- Misses 2184 2209 +25
- Partials 593 603 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Tanner Lewis <[email protected]>
@@ -0,0 +1,9 @@ | |||
def gitBranch = params.GIT_BRANCH ?: 'main' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why make a new Pipeline for this instead of use our existing ES 6.8 -> OS 2.X Pipeline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is testing a combination of RFS + C&R, having this separation I think will be useful in the future to do more targeted testing with say a pre-made snapshot in the RFS specific pipeline. It might also be nice to see if both pipelines or a single pipeline is failing when a bug is introduced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few thoughts:
- Why wouldn't we want to use a pre-made snapshot for all of our integ tests?
- Why don't we want to test both RFS and C&R for each of our integ tests?
see if both pipelines or a single pipeline is failing
-> Can you elaborate a bit more on this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its helpful for testing our workflow where the user performs the snapshot creation themselves from the migration console
In a similar vein, I like testing RFS and C&R together and separately as it affirms that these are separate independent pieces that can operate on their own, and that we haven't introduced some logic that say requires a snapshot to exist when we are only doing a C&R scenario
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per discussion, my takeaways:
- Combine w/ the existing ES 6.8 test
- Update GitHub actions to use this instead of the existing 7.10 RFS only test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have updated this PR to include both these items. See recent Jenkins build here: https://migrations.ci.opensearch.org/job/full-es68source-e2e-test/4/
...ture/dockerSolution/src/main/docker/migrationConsole/lib/integ_test/integ_test/full_tests.py
Show resolved
Hide resolved
source_cluster.endpoint = f"https://alb.migration.{stage}.local:9201" | ||
target_cluster.endpoint = f"https://alb.migration.{stage}.local:9202" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hardcoding is pretty fragile; is there a way we could pass these domains into the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed and passing in when running test command
Signed-off-by: Tanner Lewis <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just pushed an update and responded to comments. Updated Jenkins run: https://migrations.ci.opensearch.org/job/full-default-e2e-test/20/
...ture/dockerSolution/src/main/docker/migrationConsole/lib/integ_test/integ_test/full_tests.py
Show resolved
Hide resolved
@@ -0,0 +1,9 @@ | |||
def gitBranch = params.GIT_BRANCH ?: 'main' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is testing a combination of RFS + C&R, having this separation I think will be useful in the future to do more targeted testing with say a pre-made snapshot in the RFS specific pipeline. It might also be nice to see if both pipelines or a single pipeline is failing when a bug is introduced.
Signed-off-by: Tanner Lewis <[email protected]>
Signed-off-by: Tanner Lewis <[email protected]>
Signed-off-by: Tanner Lewis <[email protected]>
Signed-off-by: Tanner Lewis <[email protected]>
Signed-off-by: Tanner Lewis <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Tanner!
Description
This change introduces a new Jenkins workflow for a full Migration workflow (Metadata, Backfill, C&R) using the Capture Proxy and Target Proxy ECS services and ALB as opposed to the previous workflow of having the capture proxy installed directly on the source nodes.
This is a basic integration test in the sense that it is only moving a few key documents that get added in different parts of the workflow and is verifying source and target connections within the ALB and outside of it. In its current state it should add a lot of value for checking this setup, but within the Epic of the Jira Issue linked, there are other tasks for adding more interesting data and checking an ALB cutover from source to target that would be great additions
Also updates were added after discussion to combine this with the previous ES 6.8 RFS only test and make this the default PR test that is executed
Issues Resolved
https://opensearch.atlassian.net/browse/MIGRATIONS-2149
Testing
Jenkins testing here: https://migrations.ci.opensearch.org/job/full-default-e2e-test/19/consoleUpdate test here: https://migrations.ci.opensearch.org/job/full-es68source-e2e-test/4/
Check List
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.