Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Abstract waitForInit to minimize duplication and improve test reliability #1935

Merged
merged 2 commits into from
Jul 26, 2022
Merged

Abstract waitForInit to minimize duplication and improve test reliability #1935

merged 2 commits into from
Jul 26, 2022

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Jul 12, 2022

Description

waitForInit was copied in few different tests classes. This PR abstracts it into SingleClusterTests and changes it from being recursive to iterative with a maxRetries of 3. (i.e. waiting a max of 1.5 seconds for security plugin initialization).

While abstracting the function I noticed that some of the tests its applied to correctly identify that the plugin is initialized, but swallow an error that the user making the health request does not have the correct permissions. This PR keeps the same behavior.

This also adds waitForInit inside of tests in TenantInfoActionTest

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Maintenance, Test Reliability

  • Why these changes are required?

Reduce code duplication and improve reliability

  • What is the old behavior before changes and new behavior after changes?

Issues Resolved

Test reliability is impacting CI workflows #1917

Testing

[Please provide details of testing done: unit testing, integration testing and manual testing]

Check List

  • New functionality includes testing
  • 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.

@cwperks cwperks requested a review from a team July 12, 2022 17:30
@codecov-commenter
Copy link

Codecov Report

Merging #1935 (0b90eb4) into main (be876c0) will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #1935   +/-   ##
=========================================
  Coverage     61.00%   61.01%           
- Complexity     3233     3234    +1     
=========================================
  Files           256      256           
  Lines         18088    18088           
  Branches       3224     3224           
=========================================
+ Hits          11035    11036    +1     
+ Misses         5470     5468    -2     
- Partials       1583     1584    +1
Impacted Files Coverage Δ
...iance/ComplianceIndexingOperationListenerImpl.java 62.31% <0.00%> (+1.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be876c0...0b90eb4. Read the comment docs.

peternied
peternied requested changes Jul 12, 2022
View reviewed changes
Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great step forward, I think by making this fix a little more generic we could see the stability of the entire test suite go up instead of just these couple of tests.

src/test/java/org/opensearch/security/test/SingleClusterTest.java Outdated
//Wait for the security plugin to load roles.
public void waitForInit(Client client) throws Exception {
int maxRetries = 3;
Optional<Exception> retainedException = Optional.empty();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its cleaner without an optional, for your consideration:

    private void waitForHealthyCluster(final Client client) {
        final int maxRetries = 3;
        for (int attempt = 0; true; attempt++) {
            try {
                client.admin().cluster().health(new ClusterHealthRequest()).actionGet();
                return;
            } catch (final Exception ex) {
                if(attempt >= maxRetries) {
                    throw new RuntimeException(ex);
                }
            }
            try {
               Thread.sleep(500);
            } catch (Exception ex) { /* ignored */}
        }
    }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of the RuntimeException here. Will update.

src/test/java/org/opensearch/security/test/SingleClusterTest.java Outdated
@@ -135,6 +138,31 @@ protected void setupGenericNodes(List<Settings> nodeOverride, List<Boolean> sslO
clusterConfiguration);
}

//Wait for the security plugin to load roles.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use javadoc comment style, /** ... */

src/test/java/org/opensearch/security/test/SingleClusterTest.java Outdated
@@ -135,6 +138,31 @@ protected void setupGenericNodes(List<Settings> nodeOverride, List<Boolean> sslO
clusterConfiguration);
}

//Wait for the security plugin to load roles.
public void waitForInit(Client client) throws Exception {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the throws cause, this function is easier to use without it

src/test/java/org/opensearch/security/test/SingleClusterTest.java Outdated
Optional<Exception> retainedException = Optional.empty();
for (int i = 0; i < maxRetries; i++) {
try {
client.admin().cluster().health(new ClusterHealthRequest()).actionGet();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't a value returned that should be checked, could this value be something like not healthy/not up?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted to keep the existing behavior of waitForInit, but I think there's room for improvement. While making updates to the function I was retaining all thrown exceptions and noticed that on failing tests I would receive a no permissions for [cluster:monitor/health].... I believe the implementation did this intentionally because its indicative of security plugin initialization even if it does not explicitly check for status == UP.

How do you think we should handle missing permissions here?

src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java
@@ -67,18 +66,6 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
}
}

//Wait for the security plugin to load roles.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use that new method inside of SingleCluster to after initalized(...) is called? This would effectively make sure all tests start only when the local cluster is fully operational.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In one of the previous iterations I moved this call up to be done directly after initialize for all tests that use SingleClusterTest. It introduced test failures, but that was before I came to realize the permissions issue with the healthcheck call in waitForInit. I'll try moving this up again directly after initialize to be applicable to all tests.

src/test/java/org/opensearch/security/dlic/rest/api/TenantInfoActionTest.java Outdated
@@ -41,6 +42,9 @@ public TenantInfoActionTest(){
public void testTenantInfoAPIAccess() throws Exception {
Settings settings = Settings.builder().put(ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build();
setup(settings);
try (Client tc = getClient()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this should be baked into setup(...) or deeper, if you call setup I would expect a setup up and ready to use server. What do you think?

cliu123
cliu123 previously approved these changes Jul 12, 2022
View reviewed changes
@peternied
Copy link
Member

@cwperks are you still looking to make this change, is there something that is stuck blocking this effort?

Note; the bulk run of integration tests github workflow has been merged if you'd like to use for verification of these changes.

@cwperks cwperks dismissed stale reviews from DarshitChanpura and cliu123 via a110f18 July 19, 2022 14:58
@cwperks cwperks force-pushed the TenantInfoActionTest branch from 0b90eb4 to a110f18 Compare July 19, 2022 14:58
@cwperks
Abstract waitForInit to AbstractSecurityUnitTest and increase configu…
187a2ef 
…ration load timeout to 10s

Signed-off-by: Craig Perkins <cwperx@amazon.com>
@cwperks cwperks force-pushed the TenantInfoActionTest branch from 86af4c4 to 187a2ef Compare July 19, 2022 15:05
DarshitChanpura
DarshitChanpura reviewed Jul 19, 2022
View reviewed changes
src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java
@@ -363,13 +363,13 @@ public Map<CType, SecurityDynamicConfiguration<?>> getConfigurationsFromIndex(Co
} else {
LOGGER.debug("security index exists and was created with ES 7 (new layout)");
}
retVal.putAll(validate(cl.load(configTypes.toArray(new CType[0]), 5, TimeUnit.SECONDS, acceptInvalid), configTypes.size()));
retVal.putAll(validate(cl.load(configTypes.toArray(new CType[0]), 10, TimeUnit.SECONDS, acceptInvalid), configTypes.size()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, why did this value change to 10?

Copy link
Member Author

@cwperks cwperks Jul 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the source of the flakiness in TenantInfoActionTest. We would often receive build output like this:

[2022-07-13T18:01:15,983][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure No shard available for [org.opensearch.action.get.MultiGetShardRequest@143292d5] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,983][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure No shard available for [org.opensearch.action.get.MultiGetShardRequest@143292d5] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,983][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure No shard available for [org.opensearch.action.get.MultiGetShardRequest@143292d5] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,983][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure No shard available for [org.opensearch.action.get.MultiGetShardRequest@143292d5] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,983][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure No shard available for [org.opensearch.action.get.MultiGetShardRequest@143292d5] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,983][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure No shard available for [org.opensearch.action.get.MultiGetShardRequest@143292d5] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,983][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure No shard available for [org.opensearch.action.get.MultiGetShardRequest@143292d5] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,983][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure No shard available for [org.opensearch.action.get.MultiGetShardRequest@143292d5] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,983][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure No shard available for [org.opensearch.action.get.MultiGetShardRequest@143292d5] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,983][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure No shard available for [org.opensearch.action.get.MultiGetShardRequest@143292d5] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,984][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812] Node not connected retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,984][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812] Node not connected retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,984][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812] Node not connected retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,984][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812] Node not connected retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,984][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812] Node not connected retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,984][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812] Node not connected retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,984][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812] Node not connected retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,984][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812] Node not connected retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,984][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812] Node not connected retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:15,984][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812] Node not connected retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:16,343][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812][indices:data/read/mget[shard][s]] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:16,343][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812][indices:data/read/mget[shard][s]] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:16,343][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812][indices:data/read/mget[shard][s]] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:16,343][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812][indices:data/read/mget[shard][s]] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:16,343][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812][indices:data/read/mget[shard][s]] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:16,343][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812][indices:data/read/mget[shard][s]] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:16,343][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812][indices:data/read/mget[shard][s]] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:16,343][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812][indices:data/read/mget[shard][s]] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:16,343][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812][indices:data/read/mget[shard][s]] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
[2022-07-13T18:01:16,343][ERROR][org.opensearch.security.configuration.ConfigurationLoaderSecurity7] Failure [node_utest_n29_fnull_t458578506714_num1][127.0.0.1:6812][indices:data/read/mget[shard][s]] retrieving configuration for [INTERNALUSERS, ACTIONGROUPS, CONFIG, ROLES, ROLESMAPPING, TENANTS, NODESDN, WHITELIST, ALLOWLIST, AUDIT] (index=.opendistro_security)
**[2022-07-13T18:01:20,215][ERROR][org.opensearch.security.rest.TenantInfoAction] OpenSearchException[java.util.concurrent.TimeoutException: Timeout after 5SECONDS while retrieving configuration for [ROLESMAPPING](index=.opendistro_security)]; nested: TimeoutException[Timeout after 5SECONDS while retrieving configuration for [ROLESMAPPING](index=.opendistro_security)];**
>>>> TenantInfoActionTest testTenantInfoAPIUpdate FAILED due to java.lang.AssertionError: expected:<200> but was:<500>

See the second to last line for the timeout. By increasing the timeout gives a bit more of a buffer for the configuration load to complete.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay...got it.

DarshitChanpura
DarshitChanpura reviewed Jul 19, 2022
View reviewed changes
src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java Outdated
@@ -232,7 +216,8 @@ public void testReplication() throws Exception {
}

try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, MockReplicationPlugin.class).start()) {
waitOrThrow(node.client(), "hr-masking");
waitForInit(node.client());
node.client().execute(MockReplicationAction.INSTANCE, new MockReplicationRequest("hr-masking")).actionGet();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These calls can be extracted into in a separate method:

void mockReplicationRequest(Client client, String replicationFor) {
    waitForInit(node.client());
    node.client().execute(MockReplicationAction.INSTANCE, new MockReplicationRequest(replicationFor)).actionGet();
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I brought back waitOrThrow and opted to change the implementation of waitOrThrow to use waitForInit to cut down our duplication.

@cwperks
Use waitForInit inside waitOrThrow for CCReplicationTest
751b20c 
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@peternied peternied merged commit cb24f6e into opensearch-project:main Jul 26, 2022
@cliu123 cliu123 added the backport 2.x backport to 2.x branch label Jul 28, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 28, 2022
@cwperks @github-actions
Tenant info action test (#1935)
76491ec 
* Abstract waitForInit to AbstractSecurityUnitTest and increase configuration load timeout to 10s
* Use waitForInit inside waitOrThrow for CCReplicationTest

Signed-off-by: Craig Perkins <cwperx@amazon.com>
(cherry picked from commit cb24f6e)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Jul 28, 2022
Merged
peternied pushed a commit that referenced this pull request Jul 28, 2022
@opensearch-trigger-bot @cwperks
Tenant info action test (#1935) (#1972)
7dcf342 
* Abstract waitForInit to AbstractSecurityUnitTest and increase configuration load timeout to 10s
* Use waitForInit inside waitOrThrow for CCReplicationTest

Signed-off-by: Craig Perkins <cwperx@amazon.com>
(cherry picked from commit cb24f6e)

Co-authored-by: Craig Perkins <cwperx@amazon.com>
@cwperks cwperks changed the title Tenant info action test Abstract waitForUnit to minimize duplication and improve test reliability Jul 29, 2022
@cwperks cwperks changed the title Abstract waitForUnit to minimize duplication and improve test reliability Abstract waitForInit to minimize duplication and improve test reliability Jul 29, 2022
@DarshitChanpura DarshitChanpura mentioned this pull request Aug 1, 2022
Merged
3 tasks
stephen-crawford pushed a commit to stephen-crawford/security that referenced this pull request Nov 10, 2022
@cwperks @stephen-crawford
Tenant info action test (opensearch-project#1935)
6a9b336 
* Abstract waitForInit to AbstractSecurityUnitTest and increase configuration load timeout to 10s
* Use waitForInit inside waitOrThrow for CCReplicationTest

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
@peternied peternied mentioned this pull request Dec 2, 2022
Merged
3 tasks
peternied added a commit that referenced this pull request Dec 6, 2022
@peternied @stephen-crawford
Windows build and test support for 1.3 (#2291)
8e1adfa 
Windows build and test support for 1.3

- Use most recent format of CI workflows from main
- Backport #2206
- Supply workarounds for JDK8 incompatible APIs for Encoding / Pattern matching (Thanks @cwperks!)
- Backport only freeport logic from #1638
- Backport #1758
- All updates to TestAuditlogImpl.java from main
  - #2180
  - #1935 
  - #1920
  - #1914
  - #1829 
  - And Targeted test updates for ComplianceAuditlogTest and BasicAuditlogTest to keep up with TestAuditlogImpl.java updates

Signed-off-by: Peter Nied <petern@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
Co-authored-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
wuychn pushed a commit to ochprince/security that referenced this pull request Mar 16, 2023
@opensearch-trigger-bot @cwperks
Tenant info action test (opensearch-project#1935) (opensearch-project…
56d9243 
…#1972)

* Abstract waitForInit to AbstractSecurityUnitTest and increase configuration load timeout to 10s
* Use waitForInit inside waitOrThrow for CCReplicationTest

Signed-off-by: Craig Perkins <cwperx@amazon.com>
(cherry picked from commit cb24f6e)

Co-authored-by: Craig Perkins <cwperx@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DarshitChanpura DarshitChanpura DarshitChanpura approved these changes

@peternied peternied peternied approved these changes

@cliu123 cliu123 cliu123 left review comments

Assignees
No one assigned
Labels
backport 2.x backport to 2.x branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cwperks @codecov-commenter @peternied @cliu123 @DarshitChanpura