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

Expand CCS tests coverage #84481

Closed
5 tasks done
javanna opened this issue Mar 1, 2022 · 7 comments
Closed
5 tasks done

Expand CCS tests coverage #84481

javanna opened this issue Mar 1, 2022 · 7 comments
Assignees
@cbuescher
Labels
Meta :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team >tech debt >test Issues or PRs that are addressing/adding tests

Comments

@javanna
Copy link
Member

javanna commented Mar 1, 2022 •
edited by cbuescher
Loading

We currently have integration tests that cover cross-cluster search. What they don't cover is cross-cluster communication across multiple versions. We should make sure that these tests run against the previous minor version and double check their coverage (e.g. do they use the fields API? do they also cover field_caps? Which features of search and field_caps they cover?).

As a natural follow-up, it might make sense to consider reusing the existing field_caps and search tests yaml tests and run them in a CCS scenario, so that we don't need to maintain parallel CCS tests manually, as they tend to be forgotten over time.

Perhaps related to #78472 as it involves more CCS testing

Currently the steps we identified to reach better coverage are
@javanna javanna added >test Issues or PRs that are addressing/adding tests :Search/Search Search-related issues that do not fall into other categories labels Mar 1, 2022
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Mar 1, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@javanna javanna mentioned this issue Mar 1, 2022
Closed
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Mar 23, 2022
Expand coverage of :qa:multi-cluster-search
e68b48a 
Currently this qa module runs integration tests that cover cross-cluster search
with both the local and the remote cluster on the same version. In order to also
cover cross-cluster communication across multiple versions, this change adds
additional test tasks that also start a remote cluster in the precious minor
version and run the same tests against this configuration.

Relates elastic#84481
@cbuescher cbuescher mentioned this issue Mar 23, 2022
Merged
@cbuescher
Copy link
Member

As a first step I opened a PR that runs the existing qa:multi-cluster-search tests also against the previous minor version (#85281)
This most likely will require a follow up that extends our ability to skip yaml tests based on cluster versions. Currently this only takes the local cluster version into account, but I think future PRs similar to e.g. #78042 will require also to be able to skip based on the remote version.

@javanna javanna mentioned this issue Mar 30, 2022
Open
@cbuescher cbuescher added the Meta label Mar 30, 2022
cbuescher pushed a commit that referenced this issue Mar 31, 2022
Expand coverage of :qa:multi-cluster-search (#85281)
fc38909 
Currently this qa module runs integration tests that cover cross-cluster search
with both the local and the remote cluster on the same version. In order to also
cover cross-cluster communication across multiple versions, this change adds
additional test tasks that also start a remote cluster in all wire-compatible 
previous minor version and run the same tests against this configuration.

Relates #84481
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Mar 31, 2022
[Tests] Enhance multi cluster search test skipping
c25751a 
Currently the "skip" section in out rest yaml tests take into account the lowest
minor version of nodes in the connected local cluster. For some multi cluster
CCS test we will also need the ability to skip certain tests based on the
connected remote cluster version, e.g. if in bwc tests some functionality isn't
available yet on some bwc versions we test against. This change adds that
ability to yaml rest test in the :qa:multi-cluster-search module.

Relates to elastic#84481
@cbuescher cbuescher mentioned this issue Mar 31, 2022
Merged
cbuescher pushed a commit that referenced this issue Apr 20, 2022
[Tests] Enhance multi cluster search test skipping (#85566)
feb0462 
Currently the "skip" section in out rest yaml tests take into account the lowest
minor version of nodes in the connected local cluster. For some multi cluster
CCS test we will also need the ability to skip certain tests based on the
connected remote cluster version, e.g. if in bwc tests some functionality isn't
available yet on some bwc versions we test against. This change adds that
ability to yaml rest test in the :qa:multi-cluster-search module.

Relates to #84481
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Apr 21, 2022
Expand coverage of :qa:multi-cluster-search (elastic#85281)
c6d309b 
Currently this qa module runs integration tests that cover cross-cluster search
with both the local and the remote cluster on the same version. In order to also
cover cross-cluster communication across multiple versions, this change adds
additional test tasks that also start a remote cluster in all wire-compatible 
previous minor version and run the same tests against this configuration.

Relates elastic#84481
cbuescher pushed a commit that referenced this issue Apr 21, 2022
Expand coverage of :qa:multi-cluster-search (#85281) (#86062)
7c0a165 
Currently this qa module runs integration tests that cover cross-cluster search
with both the local and the remote cluster on the same version. In order to also
cover cross-cluster communication across multiple versions, this change adds
additional test tasks that also start a remote cluster in all wire-compatible 
previous minor version and run the same tests against this configuration.

Relates #84481
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Apr 21, 2022
[Tests] Enhance multi cluster search test skipping (elastic#85566)
770951c 
Currently the "skip" section in out rest yaml tests take into account the lowest
minor version of nodes in the connected local cluster. For some multi cluster
CCS test we will also need the ability to skip certain tests based on the
connected remote cluster version, e.g. if in bwc tests some functionality isn't
available yet on some bwc versions we test against. This change adds that
ability to yaml rest test in the :qa:multi-cluster-search module.

Relates to elastic#84481
cbuescher pushed a commit that referenced this issue Apr 21, 2022
[Tests] Enhance multi cluster search test skipping (#85566) (#86065)
44b24d3 
Currently the "skip" section in out rest yaml tests take into account the lowest
minor version of nodes in the connected local cluster. For some multi cluster
CCS test we will also need the ability to skip certain tests based on the
connected remote cluster version, e.g. if in bwc tests some functionality isn't
available yet on some bwc versions we test against. This change adds that
ability to yaml rest test in the :qa:multi-cluster-search module.

Relates to #84481
@cbuescher cbuescher mentioned this issue May 6, 2022
Merged
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue May 11, 2022
Run more search rest tests in a CCS setup
e7e1b77 
Currently we only test a small subset of cross cluster search in rest tests in
the 'multi-cluster-search' qa module. In order to increase test coverage for
basic CCS setups, this change adds a new qa modula that uses a subset of
existing search rest tests tests and runs them in a CCS scenario.

Relates to elastic#84481
@cbuescher
Copy link
Member

investigate if the proxy mode in CCS is covered well in tests or if it also needs more coverage

Regarding the last bullet point on the list I just did a quick sweep through the tests that somehow use the "proxy" mode setting instead of the default "sniff" mode. I found the following references

test/framework/src/main/java/org/elasticsearch/test/AbstractMultiClustersTestCase.java

  • configureRemoteCluster() seems to use default “mode” setting (sniff)
  • only CrossClusterSearchLeakIT seems to overwrite this method (see below)

qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/20_info.yml

  • sets “proxy” mode once and checks that remote_cluster/_info is returning it

qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/15_connection_mode_configuration.yml

  • The following tests deal with prox mode:
    • “Add persistent remote cluster in proxy mode with invalid sniff settings”
    • “Add persistent remote cluster using proxy connection mode using valid settings”
      • puts valid settings, does one search
    • “Switch connection mode for configured cluster”

server/src/internalClusterTest/java/org/elasticsearch/search/ccs/CrossClusterSearchLeakIT.java

  • frequently uses “mode: proxy” in the connection

qa/ccs-rolling-upgrade-remote-cluster/src/test/java/org/elasticsearch/upgrades/SearchStatesIT.java

  • uses both “sniff” and “proxy” mode randomly (equal probability)

x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/multi_cluster/70_connection_mode_configuration.yml

  • looks similar to “qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/15_connection_mode_configuration.yml”, just with security

qa/remote-clusters/src/test/java/org/elasticsearch/cluster/remote/test/RemoteClustersIT.java

  • extends AbstractMultiClusterRemoteTestCase for cluster setup
    • testProxyModeConnectionWorks
    • testHAProxyModeConnectionWorks
    • testHAProxyModeConnectionWithSNIToCluster1Works
    • testHAProxyModeConnectionWithSNIToCluster2Works

From what I can see so far it seems to me we do have some good coverage of the basic scenarios here. We might consider moving randomization of the connection to AbstractMultiClustersTestCase (similar to the setup in SearchStatesIT) and see if that breaks any existing tests. Also we can probably randomize the connection mode for the CCS test we currently add in #86521

@cbuescher cbuescher mentioned this issue May 17, 2022
Merged
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue May 17, 2022
Randomize connection mode for AbstractMultiClustersTestCase
9872871 
Currently tests extending AbstractMultiClustersTestCase, i.e. mostly the CCS
tests, use the default connection mode configuration of the abstract base tests,
which is the default "sniff" mode. In order to increase coverage for "proxy"
mode connections this change starts to randomize the connection type in the
setup. Subclasses of this test are free to use or overwrite this behaviour.

Relates to elastic#84481
@javanna
Copy link
Member Author

javanna commented May 23, 2022

From what I can see so far it seems to me we do have some good coverage of the basic scenarios here. We might consider moving randomization of the connection to AbstractMultiClustersTestCase (similar to the setup in SearchStatesIT) and see if that breaks any existing tests. Also we can probably randomize the connection mode for the CCS test we currently add in #86521

Agreed @cbuescher thanks for digging.

@cbuescher
Copy link
Member

I just saw that randoming the connection mode already seems to be a thing in "qa/multi-cluster-search-security", maybe in the end we do want to add something similar to #86521?

cbuescher pushed a commit that referenced this issue May 24, 2022
Randomize connection mode for AbstractMultiClustersTestCase (#86865)
9048c2d 
Currently tests extending AbstractMultiClustersTestCase, i.e. mostly the CCS
tests, use the default connection mode configuration of the abstract base tests,
which is the default "sniff" mode. In order to increase coverage for "proxy"
mode connections this change starts to randomize the connection type in the
setup. Subclasses of this test are free to use or overwrite this behaviour.

Relates to #84481
cbuescher pushed a commit that referenced this issue May 24, 2022
Run yaml rest tests using CCS in multi-cluster setup (#86521)
410d381 
Currently we only test a small subset of cross cluster search functionality in
rest tests living in the 'multi-cluster-search' qa module. In order to increase
test coverage for basic CCS functionality , this change adds a new qa modula
that re-uses a subset of existing yaml rest tests and runs them in a slightly
modified fashion in a CCS scenario.
Document data and other write operations are executed agains a "remote"
cluster, while all calls to the search API aand other APIs that support CCS are
performed on a local cluster connected to the remote with all the data via CCS.

Relates to #84481
@javanna
Copy link
Member Author

javanna commented May 30, 2022

@cbuescher sounds like this can be closed? :)

@cbuescher
Copy link
Member

Yes, we can close this, we covered all major points here.

@javanna javanna mentioned this issue Mar 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cbuescher cbuescher

Labels
Meta :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team >tech debt >test Issues or PRs that are addressing/adding tests
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@javanna @cbuescher @elasticmachine