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.

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

[systemtest][upgrade] Attempt to deflake the upgrade tests and fix sending/receiving messages during upgrade #3546

Merged
merged 2 commits into from Aug 27, 2020

Conversation

im-konge
Copy link
Member

@im-konge im-konge commented Aug 25, 2020

Signed-off-by: Lukas Kral lukywill16@gmail.com

Type of change

  • Bugfix

Description

In #3200 I totally forgot that the change of --broker-list to --boostrap-server can affect older versions of Kafka (during the upgrade) -> I tested it only with Kafka 2.5.0 and 2.6.0, problem is that --boostrap-server option was added to 2.5.0. So I added ternary operator to determine which option use based on Kafka version.

Also this is an attempt to deflake the upgrade tests. I assume that main problem of the flakyness was this block of code:

if (KafkaTopicResource.kafkaTopicClient().inNamespace(NAMESPACE).withName(continuousTopicName).get() == null) {
                KafkaTopicResource.topic(CLUSTER_NAME, continuousTopicName, 3, 3, 2).done();
                // Add continuous topic to expectedTopicCount which will be check after upgrade procedures
                expectedTopicCount += 1;
            }

The problem is that if happen that topic is deleted and recreated, the expectedTopicCount will be increased and next check will not pass. This assumption is based on given result from PR run:

[WARNING] Flakes: 
[WARNING] io.strimzi.systemtest.upgrade.StrimziUpgradeST.testUpgradeStrimziVersion(JsonObject)[9]
[ERROR]   Run 1: StrimziUpgradeST.testUpgradeStrimziVersion:97->performUpgrade:351 KafkaTopic list doesn't have expected size
Expected: is <44>
     but: was <43>
[INFO]   Run 2: PASS

Checklist

  • Make sure all tests pass

@im-konge im-konge self-assigned this Aug 25, 2020
@im-konge im-konge added this to the 0.20.0 milestone Aug 25, 2020
@im-konge im-konge marked this pull request as draft August 25, 2020 11:10
@im-konge
Copy link
Member Author

@strimzi-ci run tests test_only profile=upgrade

@strimzi-ci
Copy link

Systemtests Failed (no tests results are present)

@im-konge im-konge marked this pull request as ready for review August 25, 2020 11:38
@im-konge
Copy link
Member Author

@strimzi-ci run tests test_only profile=upgrade

@im-konge im-konge requested review from Frawless and a team August 25, 2020 11:42
@Frawless
Copy link
Member

/azp run upgrade

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strimzi-ci
Copy link

✔️ Test Summary ✔️

TEST_PROFILE: upgrade
EXCLUDED_GROUPS: networkpolicies,flaky
TEST_CASE: *ST
TOTAL: 13
PASS: 12
FAIL: 0
SKIP: 1
BUILD_NUMBER: 1452
BUILD_ENV: oc cluster up

@Frawless
Copy link
Member

/azp run upgrade

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Lukas Kral <lukywill16@gmail.com>
Signed-off-by: Lukas Kral <lukywill16@gmail.com>
@Frawless
Copy link
Member

/azp run upgrade

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@im-konge
Copy link
Member Author

After discussion with @Frawless I'm gonna fix the azure pipelines for upgrade in next PR -> so this can be merge and all issues fixed.

@im-konge im-konge added ready for merge Label for PRs which are ready for merge and removed needs review labels Aug 27, 2020
@im-konge im-konge requested a review from Frawless August 27, 2020 11:21
@Frawless Frawless merged commit 16c9018 into strimzi:master Aug 27, 2020
@im-konge im-konge deleted the fix-upgrade branch August 27, 2020 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ready for merge Label for PRs which are ready for merge System tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants