-
Notifications
You must be signed in to change notification settings - Fork 131
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.
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
Fix upgrade_through_versions_test.py::TestUpgrade* tests #228
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,17 @@ | ||
from distutils.version import LooseVersion | ||
|
||
import logging | ||
import operator | ||
import os | ||
import pprint | ||
import pytest | ||
import psutil | ||
import random | ||
import re | ||
import signal | ||
import subprocess | ||
import time | ||
import uuid | ||
import logging | ||
import pytest | ||
import psutil | ||
|
||
from collections import defaultdict, namedtuple | ||
from multiprocessing import Process, Queue | ||
|
@@ -22,8 +24,8 @@ | |
from tools.misc import generate_ssl_stores, new_node | ||
from .upgrade_manifest import (build_upgrade_pairs, | ||
current_2_2_x, | ||
current_3_0_x, indev_3_11_x, current_3_11_x, | ||
current_4_0_x, indev_4_1_x, current_4_1_x, | ||
current_3_0_x, current_3_11_x, | ||
current_4_0_x, current_4_1_x, | ||
indev_trunk, | ||
CASSANDRA_4_0, CASSANDRA_5_0, | ||
RUN_STATIC_UPGRADE_MATRIX) | ||
|
@@ -297,7 +299,6 @@ def shutdown_gently(): | |
|
||
@pytest.mark.upgrade_test | ||
@pytest.mark.resource_intensive | ||
@pytest.mark.skip("Fake skip so that this isn't run outside of a generated class that removes this annotation") | ||
class TestUpgrade(Tester): | ||
""" | ||
Upgrades a 3-node Murmur3Partitioner cluster through versions specified in test_version_metas. | ||
|
@@ -325,6 +326,8 @@ def fixture_add_additional_log_patterns(self, fixture_dtest_setup): | |
) | ||
|
||
def prepare(self): | ||
if type(self).__name__ == "TestUpgrade": | ||
pytest.skip("Skip base class, only generated classes run the tests") | ||
logger.debug("Upgrade test beginning, setting CASSANDRA_VERSION to {}, and jdk to {}. (Prior values will be restored after test)." | ||
.format(self.test_version_metas[0].version, self.test_version_metas[0].java_version)) | ||
cluster = self.cluster | ||
|
@@ -416,12 +419,6 @@ def upgrade_scenario(self, populate=True, create_schema=True, rolling=False, aft | |
|
||
# upgrade through versions | ||
for version_meta in self.test_version_metas[1:]: | ||
if version_meta.family > '3.11' and internode_ssl: | ||
seeds =[] | ||
for seed in cluster.seeds: | ||
seeds.append(seed.ip_addr + ':7001') | ||
logger.debug("Forcing seeds to 7001 for internode ssl") | ||
cluster.seeds = seeds | ||
|
||
for num, node in enumerate(self.cluster.nodelist()): | ||
# sleep (sigh) because driver needs extra time to keep up with topo and make quorum possible | ||
|
@@ -877,19 +874,11 @@ def create_upgrade_class(clsname, version_metas, protocol_version, | |
print(" using protocol: v{}, and parent classes: {}".format(protocol_version, parent_class_names)) | ||
print(" to run these tests alone, use `nosetests {}.py:{}`".format(__name__, clsname)) | ||
|
||
upgrade_applies_to_env = RUN_STATIC_UPGRADE_MATRIX or version_metas[-1].matches_current_env_version_family | ||
newcls = type( | ||
clsname, | ||
parent_classes, | ||
{'test_version_metas': version_metas, '__test__': True, 'protocol_version': protocol_version, 'extra_config': extra_config} | ||
) | ||
# Remove the skip annotation in the superclass we just derived from, we will add it back if we actually intend | ||
# to skip with a better message | ||
newcls.pytestmark = [mark for mark in newcls.pytestmark if not mark.name == "skip"] | ||
#if not upgrade_applies_to_env: | ||
#print("boo") | ||
#newcls.pytestmark.append(pytest.mark.skip("test not applicable to env")) | ||
print(newcls.pytestmark) | ||
|
||
if clsname in globals(): | ||
raise RuntimeError("Class by name already exists!") | ||
|
@@ -898,6 +887,22 @@ def create_upgrade_class(clsname, version_metas, protocol_version, | |
return newcls | ||
|
||
|
||
def jdk_compatible_steps(version_metas): | ||
metas = [] | ||
java_version = current_env_java_version() | ||
for version_meta in version_metas: | ||
if java_version in version_meta.java_versions: | ||
metas.append(version_meta) | ||
|
||
return metas | ||
|
||
|
||
def current_env_java_version(): | ||
java_command = os.path.join(os.environ['JAVA_HOME'], 'bin', 'java') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please fail somewhere with a clear message when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not needed, see update. |
||
output = subprocess.check_output([java_command, '-version'], stderr=subprocess.STDOUT).decode('utf-8') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is already implemented:
|
||
return int(re.search('version "(?:1\.)?([0-9]*).*"', output).group(1)) | ||
|
||
|
||
MultiUpgrade = namedtuple('MultiUpgrade', ('name', 'version_metas', 'protocol_version', 'extra_config')) | ||
|
||
MULTI_UPGRADES = ( | ||
|
@@ -942,18 +947,20 @@ def create_upgrade_class(clsname, version_metas, protocol_version, | |
for upgrade in MULTI_UPGRADES: | ||
# if any version_metas are None, this means they are versions not to be tested currently | ||
if all(upgrade.version_metas): | ||
metas = upgrade.version_metas | ||
# even for RUN_STATIC_UPGRADE_MATRIX we only test upgrade paths jdk compatible with the end "indev_" version | ||
metas = jdk_compatible_steps(upgrade.version_metas) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you are still calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh, your talking about the outer loop |
||
|
||
if not RUN_STATIC_UPGRADE_MATRIX: | ||
if metas[-1].matches_current_env_version_family: | ||
# looks like this test should actually run in the current env, so let's set the final version to match the env exactly | ||
oldmeta = metas[-1] | ||
newmeta = oldmeta.clone_with_local_env_version() | ||
logger.debug("{} appears applicable to current env. Overriding final test version from {} to {}".format(upgrade.name, oldmeta.version, newmeta.version)) | ||
metas[-1] = newmeta | ||
create_upgrade_class(upgrade.name, [m for m in metas], protocol_version=upgrade.protocol_version, extra_config=upgrade.extra_config) | ||
else: | ||
create_upgrade_class(upgrade.name, [m for m in metas], protocol_version=upgrade.protocol_version, extra_config=upgrade.extra_config) | ||
# replace matching meta with current version | ||
for idx, meta in enumerate(metas): | ||
if meta.matches_current_env_version_family: | ||
java_version = current_env_java_version() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should get that java version once and do not do that in every iteration in a loop There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it should only called once, i was missing a |
||
assert java_version in meta.java_versions, "Incompatible JDK {} for version {}".format(java_version, meta.family) | ||
newmeta = meta.clone_with_local_env_version() | ||
logger.debug("{} appears applicable to current env. Overriding version from {} to {}".format(upgrade.name, meta.version, newmeta.version)) | ||
metas[idx] = newmeta | ||
|
||
create_upgrade_class(upgrade.name, [m for m in metas], protocol_version=upgrade.protocol_version, extra_config=upgrade.extra_config) | ||
|
||
|
||
for pair in build_upgrade_pairs(): | ||
|
@@ -962,4 +969,4 @@ def create_upgrade_class(clsname, version_metas, protocol_version, | |
[pair.starting_meta, pair.upgrade_meta], | ||
protocol_version=pair.starting_meta.max_proto_v, | ||
bootstrap_test=True | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this change