Skip to content

Conversation

@alpar-t
Copy link
Contributor

@alpar-t alpar-t commented Sep 12, 2019

I ran into trouble while back-porting some of the recent bwc changes, so decided to add this test that would make sure we can start all bwc versions of elasticsearch.
For now it will only run as part of bwcTest so it doesn't add additional time to intake or PRs.

This revealed some changes that were required to be able to have clusters running on older versions. Some of these will not be encountered in master but since they don't add too much complexit, I think it would be better to have them and make back-ports easier.

One added benefit of this test is that we check that testclusters actually started the version and flavor we tell it to.

This PR implements fixes and adds a test to veryfy that testclusters
can start up clusters on old versions.
It also serves as a tool to be able to check changes on master before
backporting to aid in discovering issues earlier.

TODO:
 - check with multi node clusters
 - Add integ test distro
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@rjernst
Copy link
Member

rjernst commented Sep 12, 2019

This seems like overkill to me. I don't think we should be referencing 6.x versions in master; it would be yet another thing that has to be updated on major version bumps. And bwc tests should be run when backporting if the change affects bwc. Perhaps we need a label to trigger full bwc tests like we have for packaging? It also seems like some of these deficiencies here could be caught by better testing within build-tools.

@mark-vieira
Copy link
Contributor

I think I need some more context on this. Perhaps we should sync up and discuss.

@alpar-t
Copy link
Contributor Author

alpar-t commented Sep 13, 2019

I do think we need a full integration test for things like elasticsearch settings and access patterns for distributions, these are hard to mock and the value of the test is reduced because it doesn't verify assumptions we make when implementing.

The 6.x versions don't need to be updated on new releases. I'm also fine with removing that part,. We never call bwcTest in CI any more, we call the version specific v<version>#bwcTest and we do that for index compatible versions only, so the 6.x series will only be tested on 7.x, but having it there allows one to explicitly test from master, so it's more like a tool that was useful for testing that the back-port will work, rather than doing a dance where it doesn't and then master needs an update which can't be tested there. The alternative of doing it only in 7.x puts us on a path that will make it difficult to port changes back.

@alpar-t
Copy link
Contributor Author

alpar-t commented Sep 13, 2019

#45654 is the backport that needs these changes first.

@elasticmachine run elasticsearch-ci/1 adn run elasticsearch-ci/bwc

@mark-vieira
Copy link
Contributor

Perhaps we can split out the smoke test bit and just have the stuff needed to backport the BWC changes to 7.x. We can then separately discuss the test coverage issue w/o holding up those backport changes.

@alpar-t
Copy link
Contributor Author

alpar-t commented Sep 16, 2019

Thanks @mark-vieira that's a great idea, I'll do that

@alpar-t alpar-t closed this Sep 16, 2019
@colings86 colings86 added v7.4.0 and removed v7.4.1 labels Sep 17, 2019
@alpar-t alpar-t deleted the testclusters-extended-bwc branch November 11, 2019 09:41
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team team-discuss v7.4.0 v7.5.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants