Adding Backward Compatibility Tests for k-NN (opensearch-project#226)

* Adding Backward Compatibility Tests for k-NN Signed-off-by: Naveen Tatikonda <navtat@amazon.com> * Delete unnecessary Binary files Signed-off-by: Naveen Tatikonda <navtat@amazon.com> * Updated Developer Guide and refactored code Signed-off-by: Naveen Tatikonda <navtat@amazon.com> Signed-off-by: Martin Gaievski <gaievski@amazon.com>
martin-gaievski · Mar 7, 2022 · cfeac95 · cfeac95
1 parent 2e1e19f
commit cfeac95
Show file tree

Hide file tree

Showing 9 changed files with 410 additions and 2 deletions.
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -35,6 +35,11 @@ jobs:
         run: |
           ./gradlew build -Dopensearch.version=1.2.0-SNAPSHOT
 
+      - name: Run k-NN Backwards Compatibility Tests
+        run: |
+          echo "Running backwards compatibility tests ..."
+          ./gradlew bwcTestSuite -Dtests.security.manager=false
+
       - name: Upload Coverage Report
         uses: codecov/codecov-action@v1
         with:

diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md
@@ -12,6 +12,8 @@
     - [Run Single-node Cluster Locally](#run-single-node-cluster-locally)
     - [Run Multi-node Cluster Locally](#run-multi-node-cluster-locally)
   - [Debugging](#debugging)
+  - [Backwards Compatibility Testing](#backwards-compatibility-testing)
+    - [Adding new tests](#adding-new-tests)
   - [Submitting Changes](#submitting-changes)
 
 # Developer Guide
@@ -205,6 +207,27 @@ Additionally, it is possible to attach one debugger to the cluster JVM and anoth
 ./gradlew :integTest -Dtest.debug=1 -Dcluster.debug=1
 ```
 
+## Backwards Compatibility Testing
+
+The purpose of Backwards Compatibility Testing and different types of BWC tests are explained [here](https://github.com/opensearch-project/opensearch-plugins/blob/main/TESTING.md#backwards-compatibility-testing)
+
+Use these commands to run BWC tests for k-NN:
+
+1. Mixed cluster test: `./gradlew knnBwcCluster#mixedClusterTask -Dtests.security.manager=false`
+2. Rolling upgrade tests: `./gradlew knnBwcCluster#rollingUpgradeClusterTask -Dtests.security.manager=false`
+3. Full restart upgrade tests: `./gradlew knnBwcCluster#fullRestartClusterTask -Dtests.security.manager=false`
+4. `./gradlew bwcTestSuite -Dtests.security.manager=false` is used to run all the above bwc tests together.
+
+Use this command to run BWC tests for a given Backwards Compatibility Version:
+```
+./gradlew bwcTestSuite -Dbwc.version=1.0.0.0-SNAPSHOT
+```
+Here, we are testing BWC Tests with BWC version of plugin as 1.0.0.0. Make sure to add the binary file of that version in the bwc directory in resources.
+
+### Adding new tests
+
+Before adding any new tests to Backward Compatibility Tests, we should be aware that the tests in BWC are not independent. While creating an index, a test cannot use the same index name if it is already used in other tests. Also, adding extra operations to the existing test may impact other existing tests like graphCount. 
+
 ## Submitting Changes
 
-See [CONTRIBUTING](CONTRIBUTING.md).
+See [CONTRIBUTING](CONTRIBUTING.md).
diff --git a/build.gradle b/build.gradle
@@ -3,9 +3,14 @@
  *  SPDX-License-Identifier: Apache-2.0
  */
 
+import java.util.concurrent.Callable
+import org.opensearch.gradle.testclusters.StandaloneRestIntegTestTask
+
 buildscript {
     ext {
         opensearch_version = System.getProperty("opensearch.version", "1.2.0-SNAPSHOT")
+        knn_bwc_version = System.getProperty("bwc.version", "1.1.0.0-SNAPSHOT")
+        opensearch_bwc_version = "${knn_bwc_version}" - ".0-SNAPSHOT"
         opensearch_group = "org.opensearch"
     }
 
@@ -170,6 +175,12 @@ integTest {
     systemProperty "user", System.getProperty("user")
     systemProperty "password", System.getProperty("password")
 
+    if (System.getProperty("tests.rest.bwcsuite_cluster") == null) {
+        filter {
+            excludeTestsMatching "org.opensearch.knn.bwc.*IT"
+        }
+    }
+
     doFirst {
         // Tell the test JVM if the cluster JVM is running under a debugger so that tests can
         // use longer timeouts for requests.
@@ -208,6 +219,160 @@ testClusters.integTest {
     systemProperty("java.library.path", "$rootDir/jni/release")
 }
 
+// bwcFilePath contains the gradlew assemble binary files of k-NN plugins
+String baseName = "knnBwcCluster"
+String bwcFilePath = "src/test/resources/org/opensearch/knn/bwc/"
+
+// Creates two test clusters of previous version and loads k-NN plugin of bwcVersion
+2.times { i ->
+    testClusters {
+        "${baseName}$i" {
+            testDistribution = "ARCHIVE"
+            versions = [opensearch_bwc_version, opensearch_version] //Opensearch Cluster Versions
+            numberOfNodes = 3
+            plugin(provider(new Callable<RegularFile>() {
+                @Override
+                RegularFile call() throws Exception {
+                    return new RegularFile() {
+                        @Override
+                        File getAsFile() {
+                            return fileTree(bwcFilePath + knn_bwc_version).getSingleFile()
+                        }
+                    }
+                }
+            }))
+            setting 'path.repo', "${buildDir}/cluster/shared/repo/${baseName}"
+            setting 'http.content_type.required', 'true'
+            systemProperty "java.library.path", "$rootDir/src/test/resources/org/opensearch/knn/bwc/lib:$rootDir/jni/release"
+        }
+    }
+}
+
+// upgradeNodeAndPluginToNextVersion(plugins) upgrades plugin on the upgraded node with project.version binary file in bwcFilePath
+// upgradeAllNodesAndPluginsToNextVersion(plugins) upgrades plugins on all the 3 nodes after upgrading the nodes
+List<Provider<RegularFile>> plugins = [
+        provider(new Callable<RegularFile>(){
+            @Override
+            RegularFile call() throws Exception {
+                return new RegularFile() {
+                    @Override
+                    File getAsFile() {
+                        return fileTree(bwcFilePath + project.version).getSingleFile()
+                    }
+                }
+            }
+        })
+]
+
+// Creates 2 test clusters with 3 nodes of the old version.
+2.times { i ->
+    task "${baseName}#oldVersionClusterTask$i"(type: StandaloneRestIntegTestTask) {
+        useCluster testClusters."${baseName}$i"
+        filter {
+            includeTestsMatching "org.opensearch.knn.bwc.*IT"
+        }
+        systemProperty 'tests.rest.bwcsuite_cluster', 'old_cluster'
+        systemProperty 'tests.rest.bwcsuite_round', 'old'
+        systemProperty 'tests.plugin_bwc_version', knn_bwc_version
+        nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}$i".allHttpSocketURI.join(",")}")
+        nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}$i".getName()}")
+    }
+}
+
+// Upgrades one node of the old cluster to new OpenSearch version with upgraded plugin version
+// This results in a mixed cluster with 2 nodes on the old version and 1 upgraded node.
+// This is also used as a one third upgraded cluster for a rolling upgrade.
+task "${baseName}#mixedClusterTask"(type: StandaloneRestIntegTestTask) {
+    useCluster testClusters."${baseName}0"
+    dependsOn "${baseName}#oldVersionClusterTask0"
+    doFirst {
+        testClusters."${baseName}0".upgradeNodeAndPluginToNextVersion(plugins)
+    }
+    filter {
+        includeTestsMatching "org.opensearch.knn.bwc.*IT"
+    }
+    systemProperty 'tests.rest.bwcsuite_cluster', 'mixed_cluster'
+    systemProperty 'tests.rest.bwcsuite_round', 'first'
+    systemProperty 'tests.plugin_bwc_version', knn_bwc_version
+    nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}0".allHttpSocketURI.join(",")}")
+    nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}0".getName()}")
+}
+
+// Upgrades the second node to new OpenSearch version with upgraded plugin version after the first node is upgraded.
+// This results in a mixed cluster with 1 node on the old version and 2 upgraded nodes.
+// This is used for rolling upgrade.
+task "${baseName}#twoThirdsUpgradedClusterTask"(type: StandaloneRestIntegTestTask) {
+    dependsOn "${baseName}#mixedClusterTask"
+    useCluster testClusters."${baseName}0"
+    doFirst {
+        testClusters."${baseName}0".upgradeNodeAndPluginToNextVersion(plugins)
+    }
+    filter {
+        includeTestsMatching "org.opensearch.knn.bwc.*IT"
+    }
+    systemProperty 'tests.rest.bwcsuite_cluster', 'mixed_cluster'
+    systemProperty 'tests.rest.bwcsuite_round', 'second'
+    systemProperty 'tests.plugin_bwc_version', knn_bwc_version
+    nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}0".allHttpSocketURI.join(",")}")
+    nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}0".getName()}")
+}
+
+// Upgrades the third node to new OpenSearch version with upgraded plugin version after the second node is upgraded.
+// This results in a fully upgraded cluster.
+// This is used for rolling upgrade.
+task "${baseName}#rollingUpgradeClusterTask"(type: StandaloneRestIntegTestTask) {
+    dependsOn "${baseName}#twoThirdsUpgradedClusterTask"
+    useCluster testClusters."${baseName}0"
+    doFirst {
+        testClusters."${baseName}0".upgradeNodeAndPluginToNextVersion(plugins)
+    }
+    filter {
+        includeTestsMatching "org.opensearch.knn.bwc.*IT"
+    }
+    mustRunAfter "${baseName}#mixedClusterTask"
+    systemProperty 'tests.rest.bwcsuite_cluster', 'mixed_cluster'
+    systemProperty 'tests.rest.bwcsuite_round', 'third'
+    systemProperty 'tests.plugin_bwc_version', knn_bwc_version
+    nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}0".allHttpSocketURI.join(",")}")
+    nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}0".getName()}")
+}
+
+// Upgrades all the nodes of the old cluster to new OpenSearch version with upgraded plugin version
+// at the same time resulting in a fully upgraded cluster.
+task "${baseName}#fullRestartClusterTask"(type: StandaloneRestIntegTestTask) {
+    dependsOn "${baseName}#oldVersionClusterTask1"
+    useCluster testClusters."${baseName}1"
+    doFirst {
+        testClusters."${baseName}1".upgradeAllNodesAndPluginsToNextVersion(plugins)
+    }
+    filter {
+        includeTestsMatching "org.opensearch.knn.bwc.*IT"
+    }
+    systemProperty 'tests.rest.bwcsuite_cluster', 'upgraded_cluster'
+    systemProperty 'tests.rest.bwcsuite_round', 'fullrestart'
+    systemProperty 'tests.plugin_bwc_version', knn_bwc_version
+    nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}1".allHttpSocketURI.join(",")}")
+    nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}1".getName()}")
+}
+
+// A bwc test suite which runs all the bwc tasks combined.
+task bwcTestSuite(type: StandaloneRestIntegTestTask) {
+    dependsOn "copyLatestSnapshot"
+    exclude '**/*Test*'
+    exclude '**/*IT*'
+    dependsOn tasks.named("${baseName}#mixedClusterTask")
+    dependsOn tasks.named("${baseName}#rollingUpgradeClusterTask")
+    dependsOn tasks.named("${baseName}#fullRestartClusterTask")
+}
+
+task copyLatestSnapshot(type: Copy){
+    dependsOn 'assemble'
+    mkdir "$bwcFilePath$project.version"
+    from "$buildDir/distributions"
+    include "*.zip"
+    into "$bwcFilePath$project.version"
+}
+
 run {
     useCluster project.testClusters.integTest
     dependsOn buildJniLib

diff --git a/src/test/java/org/opensearch/knn/ODFERestTestCase.java b/src/test/java/org/opensearch/knn/ODFERestTestCase.java
@@ -35,6 +35,9 @@
 import org.opensearch.test.rest.OpenSearchRestTestCase;
 import org.junit.After;
 
+import static org.opensearch.knn.TestUtils.KNN_BWC_PREFIX;
+import static org.opensearch.knn.TestUtils.OPENDISTRO_SECURITY;
+
 /**
  * ODFE integration test base class to support both security disabled and enabled ODFE cluster.
  */
@@ -138,11 +141,19 @@ protected void wipeAllODFEIndices() throws IOException {
 
             for (Map<String, Object> index : parserList) {
                 String indexName = (String) index.get("index");
-                if (indexName != null && !".opendistro_security".equals(indexName)) {
+                if (!skipDeleteIndex(indexName)) {
                     adminClient().performRequest(new Request("DELETE", "/" + indexName));
                 }
             }
         }
     }
+
+    private boolean skipDeleteIndex(String indexName){
+        if (indexName != null && !OPENDISTRO_SECURITY.equals(indexName) && !indexName.matches(KNN_BWC_PREFIX+"(.*)")){
+            return false;
+        }
+
+        return true;
+    }
 }
 
diff --git a/src/test/java/org/opensearch/knn/TestUtils.java b/src/test/java/org/opensearch/knn/TestUtils.java
@@ -25,6 +25,13 @@
 import java.util.Map;
 
 public class TestUtils {
+    public static final String KNN_BWC_PREFIX = "knn-bwc-";
+    public static final String OS_KNN = "opensearch-knn";
+    public static final String OPENDISTRO_SECURITY = ".opendistro_security";
+    public static final String BWCSUITE_CLUSTER = "tests.rest.bwcsuite_cluster";
+    public static final String BWCSUITE_ROUND = "tests.rest.bwcsuite_round";
+    public static final String TEST_CLUSTER_NAME = "tests.clustername";
+
     /**
      * Class to read in some test data from text files
      */