Modify Jenkins Full E2E Integ Test to perform Transformations #1182

lewijacn · 2024-12-06T00:40:42Z

Description

Modifies integ test to add transformation to RFS and Metadata migration to rename an index and modify given RFS documents to go to a new index

After facing a lot of difficulty with crafting a transformation and passing as a command argument, added E2E test for DocumentMigration based off a structure recently introduced by Andre to make top level transformation testing in DocumentMigration much easier to test locally.

Issues Resolved

https://opensearch.atlassian.net/browse/MIGRATIONS-2150

Is this a backport? If so, please add backport PR # and/or commits #

Testing

Jenkins and local testing

Check List

New functionality includes testing
- All tests pass, including unit test, integration test and doctest
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.

Signed-off-by: Tanner Lewis <[email protected]>

...shotMigration/src/test/java/org/opensearch/migrations/bulkload/CustomTransformationTest.java

codecov · 2024-12-06T01:02:57Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.64%. Comparing base (d45f4f3) to head (34c967a).

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #1182   +/-   ##
=========================================
  Coverage     80.64%   80.64%           
  Complexity     3076     3076           
=========================================
  Files           421      421           
  Lines         15610    15610           
  Branches       1053     1053           
=========================================
  Hits          12588    12588           
  Misses         2381     2381           
  Partials        641      641

Flag	Coverage Δ
unittests	`80.64% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Tanner Lewis <[email protected]>

chelma · 2024-12-06T20:18:09Z

...shotMigration/src/test/java/org/opensearch/migrations/bulkload/CustomTransformationTest.java

+
+            var sourceClusterOperations = new ClusterOperations(esSourceContainer.getUrl());
+
+            var shards = 2;


Non-blocking: why define this shard count again? Maybe you can use a class-level member instead of redefining.

This should be removed now, I've done a good amount of refactoring to remove extra logic for this test class that isn't needed

chelma · 2024-12-06T20:22:59Z

...shotMigration/src/test/java/org/opensearch/migrations/bulkload/CustomTransformationTest.java

+        }
+    }
+
+    private static String createIndexNameTransformation(String existingIndexName, String newIndexName) {


Can you please add a comment explaining what this transform is supposed to accomplish? The method name is a good start, but, frankly, Jolt is inscrutable.

chelma · 2024-12-06T20:26:13Z

...shotMigration/src/test/java/org/opensearch/migrations/bulkload/CustomTransformationTest.java

+    }
+
+    @SneakyThrows
+    private static int runProcessAgainstTarget(


It looks like this was basically copied from the ProcessLifecycleTest. Can you please find a way to share/re-use rather than copy?

I've done a good amount of refactoring so this should be different, with some shared code move into parent class

chelma · 2024-12-06T20:26:30Z

...shotMigration/src/test/java/org/opensearch/migrations/bulkload/CustomTransformationTest.java

+
+
+    @NotNull
+    private static ProcessBuilder setupProcess(


It looks like this was basically copied from the ProcessLifecycleTest. Can you please find a way to share/re-use rather than copy?

chelma · 2024-12-06T20:48:28Z

...ture/dockerSolution/src/main/docker/migrationConsole/lib/integ_test/integ_test/full_tests.py

+        expected_target_docs[transformed_index_name] = {"count": 1}
+        check_doc_counts_match(cluster=target_cluster, expected_index_details=expected_target_docs,


I must be missing something here, as I'd expect this check to fail. It appears the index name in the snapshot is the original, and we're passing in a transformation to the backfill process. As a result, I'd expect that the backfill process would create a new index on the target with default settings and the original index name, and put its doc into that.

Can you fill me in on how this check is passing?

The backfill process has a transformation to change the index name to the transformed index name, that is specified in the deployment CDK context

chelma · 2024-12-06T20:54:15Z

...ture/dockerSolution/src/main/docker/migrationConsole/lib/integ_test/integ_test/full_tests.py

-        expected_docs[index_name] = {"count": 3}
-        check_doc_counts_match(cluster=source_cluster, expected_index_details=expected_docs,
+        expected_source_docs[index_name] = {"count": 3}
+        # TODO Replayer transformation needed to only have docs in the transformed index


Can you please make a follow-up Jira for this work and link it here?

Here: https://opensearch.atlassian.net/browse/MIGRATIONS-2277

Signed-off-by: Tanner Lewis <[email protected]>

# Conflicts: # DocumentsFromSnapshotMigration/src/test/java/org/opensearch/migrations/bulkload/LeaseExpirationTest.java # DocumentsFromSnapshotMigration/src/test/java/org/opensearch/migrations/bulkload/SourceTestBase.java

Signed-off-by: Tanner Lewis <[email protected]>

lewijacn added 13 commits December 4, 2024 20:29

Add basic name change transform to verify

569a292

Signed-off-by: Tanner Lewis <[email protected]>

Revert ES5 change and attempt RFS transform

413136f

Signed-off-by: Tanner Lewis <[email protected]>

Update to base64

e171f0f

Signed-off-by: Tanner Lewis <[email protected]>

Update to add comma

90cee67

Signed-off-by: Tanner Lewis <[email protected]>

Update pass unique id and use proper arg

d0d91a3

Signed-off-by: Tanner Lewis <[email protected]>

Modify RFS transformation

79fe4bd

Signed-off-by: Tanner Lewis <[email protected]>

Update transform

79b7af0

Signed-off-by: Tanner Lewis <[email protected]>

Modify transformations for RFS and Replayer

01a7d7c

Signed-off-by: Tanner Lewis <[email protected]>

Remove Replayer transformation for now

b2cc0e1

Signed-off-by: Tanner Lewis <[email protected]>

Try replayer transformation

fae7986

Signed-off-by: Tanner Lewis <[email protected]>

Update checks

a6f51ef

Signed-off-by: Tanner Lewis <[email protected]>

Merge remote-tracking branch 'origin/main' into add-transform-to-integ

77c9577

Signed-off-by: Tanner Lewis <[email protected]>

Handle linting fixes

d3f0d21

Signed-off-by: Tanner Lewis <[email protected]>

lewijacn requested review from AndreKurait, chelma, gregschohn, mikaylathompson, peternied and sumobrian as code owners December 6, 2024 00:40

github-advanced-security bot found potential problems Dec 6, 2024

View reviewed changes

...shotMigration/src/test/java/org/opensearch/migrations/bulkload/CustomTransformationTest.java Fixed Show fixed Hide fixed

lewijacn added 2 commits December 6, 2024 08:48

Fix doc count

552d7f7

Signed-off-by: Tanner Lewis <[email protected]>

Minor naming and cleanup

c5fba6c

Signed-off-by: Tanner Lewis <[email protected]>

chelma reviewed Dec 6, 2024

View reviewed changes

lewijacn added 4 commits December 12, 2024 15:48

Testing refactoring and addressing PR comments

b189282

Signed-off-by: Tanner Lewis <[email protected]>

Handle changes from main and refactor further

0e2f453

Signed-off-by: Tanner Lewis <[email protected]>

Merge remote-tracking branch 'origin/main' into add-transform-to-integ

34c967a

Signed-off-by: Tanner Lewis <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify Jenkins Full E2E Integ Test to perform Transformations #1182

Modify Jenkins Full E2E Integ Test to perform Transformations #1182

lewijacn commented Dec 6, 2024

codecov bot commented Dec 6, 2024 •

edited

Loading

chelma Dec 6, 2024

lewijacn Dec 12, 2024

chelma Dec 6, 2024

lewijacn Dec 12, 2024

chelma Dec 6, 2024

lewijacn Dec 12, 2024

chelma Dec 6, 2024

lewijacn Dec 12, 2024

chelma Dec 6, 2024

lewijacn Dec 11, 2024

chelma Dec 6, 2024

lewijacn Dec 12, 2024


		var sourceClusterOperations = new ClusterOperations(esSourceContainer.getUrl());

		var shards = 2;

		expected_target_docs[transformed_index_name] = {"count": 1}
		check_doc_counts_match(cluster=target_cluster, expected_index_details=expected_target_docs,

Modify Jenkins Full E2E Integ Test to perform Transformations #1182

Are you sure you want to change the base?

Modify Jenkins Full E2E Integ Test to perform Transformations #1182

Conversation

lewijacn commented Dec 6, 2024

Description

Issues Resolved

Testing

Check List

codecov bot commented Dec 6, 2024 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 6, 2024 •

edited

Loading