Skip to content

Conversation

@hendrikmuhs
Copy link

@hendrikmuhs hendrikmuhs commented Oct 25, 2021

add rolling upgrade tests for upgrade endpoint

@hendrikmuhs hendrikmuhs added >test Issues or PRs that are addressing/adding tests v8.0.0 :ml/Transform Transform v7.16.0 labels Oct 25, 2021
@elasticmachine elasticmachine added the Team:ML Meta label for the ML team label Oct 25, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@hendrikmuhs hendrikmuhs marked this pull request as draft October 25, 2021 15:12
@hendrikmuhs
Copy link
Author

The test fails because it upgrades 8.0 to 8.0. This looks like an error in the test setup. I will investigate, however I consider this benign, once 8.0 is branched off, I expect this problem to disappear.

@hendrikmuhs
Copy link
Author

^ @elastic/es-delivery FYI, see my last comment. I am not sure if you want to do something about it or if you suggest to wait for the 8.0 branch.

@droberts195
Copy link

droberts195 commented Oct 27, 2021

It's deliberate that the BWC tests run with the "old" cluster starting on the same version as the "new" cluster - see #39102.

I guess what this shows is that no other functionality that refuses to run in mixed version clusters has this aspect tested by the YAML rolling upgrade tests. For all these other features the refusal to run in mixed version clusters must be tested only by unit tests or by integration tests written in Java that can assert on more complex combinations of things.

@hendrikmuhs hendrikmuhs force-pushed the transform-upgrading-rrtest branch from e3c1f83 to bc57eb4 Compare October 27, 2021 10:10
@hendrikmuhs hendrikmuhs marked this pull request as ready for review October 27, 2021 11:12
client().performRequest(waitForYellow);
verifyContinuousTransformHandlesData(3);
verifyUpgrade();
cleanUpTransforms();
Copy link
Author

Choose a reason for hiding this comment

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

sorry for reformat, verifyUpgradeFailsIfMixedCluster and verifyUpgrade are the 2 methods I added

Response response = client().performRequest(upgradeTransformRequest);
assertEquals(200, response.getStatusLine().getStatusCode());
}

Copy link
Author

Choose a reason for hiding this comment

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

^ above are the 2 new methods

@danhermann danhermann added v8.1.0 and removed v7.16.0 labels Oct 27, 2021
@mark-vieira
Copy link
Contributor

Yep, what @droberts195 said. We "upgrade" from the current version to the current version to effectively simulate a node restart. We should probably make this scenario more explicit though.

@hendrikmuhs
Copy link
Author

@elasticmachine update branch

Copy link

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

@hendrikmuhs hendrikmuhs merged commit 5eae4f0 into elastic:master Oct 28, 2021
@hendrikmuhs hendrikmuhs deleted the transform-upgrading-rrtest branch October 28, 2021 11:51
hendrikmuhs pushed a commit to hendrikmuhs/elasticsearch that referenced this pull request Oct 28, 2021
hendrikmuhs pushed a commit to hendrikmuhs/elasticsearch that referenced this pull request Oct 28, 2021
elasticsearchmachine pushed a commit that referenced this pull request Oct 28, 2021
…80012)

add rolling upgrade tests for upgrade endpoint

Co-authored-by: Elastic Machine <[email protected]>
elasticsearchmachine pushed a commit that referenced this pull request Oct 28, 2021
) (#80013)

* [Transform] add rolling upgrade tests for upgrade endpoint (#79721)

add rolling upgrade tests for upgrade endpoint

* always use the transform endpoint prefix

* remove mixed cluster check, because the endpoint does not exist < 7.16
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Oct 28, 2021
…formance

* upstream/master: (153 commits)
  [ML] update truncation default & adding field output when input is truncated (elastic#79942)
  [ML] stop using isAllowedByLicense for model license checks (elastic#79908)
  [ML] Retain built-in ML roles granting Kibana privileges (elastic#80014)
  [Transform] remove old mixed cluster BWC layers, not required for 8x (elastic#79927)
  Increase test timeout for CoordinatorTests testAllSearchesExecuted
  [Transform] add rolling upgrade tests for upgrade endpoint (elastic#79721)
  [ML] Update trained model docs for truncate parameter for bert tokenization (elastic#79652)
  `CoordinatorTests` sometimes needs three term bumps (elastic#79574)
  [ML] Account for service being triggered twice in tests (elastic#80000)
  SearchContext: remove unused variable (elastic#79917)
  Revert "Deprecate resolution loss on date field (elastic#78921)" (elastic#79914)
  Re-enable GeoIpDownloaderIT#testStartWithNoDatabases() (elastic#79907)
  Fix SnapshotBasedIndexRecoveryIT#testSeqNoBasedRecoveryIsUsedAfterPrimaryFailOver (elastic#79469)
  Fix RecoverySourceHandlerTests (elastic#79546)
  SQL: stabilize SqlSearchPageTimeoutIT (elastic#79928)
  Wait 3 seconds for the server to reload trust (elastic#79778)
  Skip automatically preserved request headers when rewriting (elastic#79973)
  Check whether stdout is a real console (elastic#79882)
  Convert remote license checker to use LicensedFeature (elastic#79876)
  Miscellaneous fixes for LDAP SDK v6 upgrade (elastic#79891)
  ...

# Conflicts:
#	libs/x-content/src/main/java/org/elasticsearch/xcontent/support/filtering/FilterPath.java
#	libs/x-content/src/test/java/org/elasticsearch/xcontent/support/filtering/FilterPathGeneratorFilteringTests.java
#	libs/x-content/src/test/java/org/elasticsearch/xcontent/support/filtering/FilterPathTests.java
@danhermann danhermann added v7.16.0 and removed v7.16.1 labels Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:ml/Transform Transform Team:ML Meta label for the ML team >test Issues or PRs that are addressing/adding tests v7.16.0 v8.0.0-beta1 v8.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants