Skip to content

KAFKA-18941: Do not test 3.3 in upgrade_tests.py#19162

Merged
cmccabe merged 1 commit intoapache:trunkfrom
josefk31:josefk31/remote-unneeded-tests
Mar 11, 2025
Merged

KAFKA-18941: Do not test 3.3 in upgrade_tests.py#19162
cmccabe merged 1 commit intoapache:trunkfrom
josefk31:josefk31/remote-unneeded-tests

Conversation

@josefk31
Copy link
Copy Markdown
Contributor

@josefk31 josefk31 commented Mar 7, 2025

The upgrade test in question is not supported for AK 3.3.2 due to a known issue. Previous attempt at solving this left the metadata.log.dir empty which leads to the following crash log:

ERROR Exiting Kafka due to fatal exception (kafka.Kafka$)
org.apache.kafka.common.KafkaException: No `meta.properties` found in  (have you run `kafka-storage.sh` to format the directory?)
	at kafka.server.BrokerMetadataCheckpoint$.$anonfun$getBrokerMetadataAndOfflineDirs$2(BrokerMetadataCheckpoint.scala:172)
	at scala.collection.Iterator.foreach(Iterator.scala:943)
	at scala.collection.Iterator.foreach$(Iterator.scala:943)
	at scala.collection.AbstractIterator.foreach(Iterator.scala:1431)
	at scala.collection.IterableLike.foreach(IterableLike.scala:74)
	at scala.collection.IterableLike.foreach$(IterableLike.scala:73)
	at scala.collection.AbstractIterable.foreach(Iterable.scala:56)
	at kafka.server.BrokerMetadataCheckpoint$.getBrokerMetadataAndOfflineDirs(BrokerMetadataCheckpoint.scala:161)
	at kafka.server.KafkaRaftServer$.initializeLogDirs(KafkaRaftServer.scala:184)
	at kafka.server.KafkaRaftServer.<init>(KafkaRaftServer.scala:61)
	at kafka.Kafka$.buildServer(Kafka.scala:79)
	at kafka.Kafka$.main(Kafka.scala:87)
	at kafka.Kafka.main(Kafka.scala)

The upgrade test in question is not supported for AK 3.3.2 due to a known issue.
Previous attempt at solving this left the `metadata.log.dir` empty which leads to the following crash log:

```
ERROR Exiting Kafka due to fatal exception (kafka.Kafka$)
org.apache.kafka.common.KafkaException: No `meta.properties` found in  (have you run `kafka-storage.sh` to format the directory?)
	at kafka.server.BrokerMetadataCheckpoint$.$anonfun$getBrokerMetadataAndOfflineDirs$2(BrokerMetadataCheckpoint.scala:172)
	at scala.collection.Iterator.foreach(Iterator.scala:943)
	at scala.collection.Iterator.foreach$(Iterator.scala:943)
	at scala.collection.AbstractIterator.foreach(Iterator.scala:1431)
	at scala.collection.IterableLike.foreach(IterableLike.scala:74)
	at scala.collection.IterableLike.foreach$(IterableLike.scala:73)
	at scala.collection.AbstractIterable.foreach(Iterable.scala:56)
	at kafka.server.BrokerMetadataCheckpoint$.getBrokerMetadataAndOfflineDirs(BrokerMetadataCheckpoint.scala:161)
	at kafka.server.KafkaRaftServer$.initializeLogDirs(KafkaRaftServer.scala:184)
	at kafka.server.KafkaRaftServer.<init>(KafkaRaftServer.scala:61)
	at kafka.Kafka$.buildServer(Kafka.scala:79)
	at kafka.Kafka$.main(Kafka.scala:87)
	at kafka.Kafka.main(Kafka.scala)
```
@github-actions github-actions Bot added triage PRs from the community tests Test fixes (including flaky tests) small Small PRs labels Mar 7, 2025
@josefk31
Copy link
Copy Markdown
Contributor Author

josefk31 commented Mar 7, 2025

I have tested this locally with the ducker-ak tool. The system tests work.

@chia7712
Copy link
Copy Markdown
Member

chia7712 commented Mar 9, 2025

@josefk31 in #18386 we made the e2e work for the "valid" upgrade path - single folder. Hence, could you please share more details of the description "upgrade test in question is not supported for AK 3.3.2 ", thanks!

@josefk31
Copy link
Copy Markdown
Contributor Author

@chia7712 My apologies, I think I linked to the wrong issue, here is the correct one:
https://issues.apache.org/jira/browse/KAFKA-18442

Issue is as follows:
3.3.2 expects a metadata.properties file on startup. If it cannot find that file it will crash with fatal error. If metadata.properties was created by a newer version of kafka, then there will also be a fatal exception. Hence I'm not sure if we want to support an upgrade and downgrade with this version.

@cmccabe cmccabe changed the title [KAFKA-18941] Removes unneeded tests from upgrade_tests.py [KAFKA-18941] Do not test 3.3 in upgrade_tests.py Mar 10, 2025
@cmccabe cmccabe changed the title [KAFKA-18941] Do not test 3.3 in upgrade_tests.py KAFKA-18941: Do not test 3.3 in upgrade_tests.py Mar 10, 2025
@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Mar 10, 2025

@josefk31 thanks for the explanation. let's disable this for now, given how old 3.3 is. If someone wants to put some time into getting 3.3 working, that is welcome as well... but at least we can fix the brokenness for now.

Copy link
Copy Markdown
Contributor

@cmccabe cmccabe left a comment

Choose a reason for hiding this comment

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

LGTM

@cmccabe cmccabe merged commit b8886b8 into apache:trunk Mar 11, 2025
25 checks passed
@ijuma
Copy link
Copy Markdown
Member

ijuma commented Mar 11, 2025

Hmm, our documentation says that upgrades from 3.3 to 4.0 are supported. So, I'm surprised we removed the test for that version. Also, 3.3.1 was released on October 2022 - that's a bit over 2 years old. Not that old from an upgrade source perspective.

@github-actions github-actions Bot removed the triage PRs from the community label Mar 11, 2025
@TaiJuWu
Copy link
Copy Markdown
Collaborator

TaiJuWu commented Mar 11, 2025

Hi @josefk31 , thanks for this patch, I do two things to check this issue.

I ran following command without this patch, it pass on local and I can't reproduce this issue.
Could you help to check or share your step ?

TC_PATHS="tests/kafkatest/tests/core/upgrade_test.py::TestUpgrade.test_isolated_mode_upgrade_downgrade" _DUCKTAPE_OPTIONS='--parameters '\''{"from_kafka_version": "3.3.2", "metadata_quorum": "ISOLATED_KRAFT"}'\' bash tests/docker/run_tests.sh

image

 
I also checked 3.3.2 branch.
From my understanding, there is no need to set metadata.log.dir if I am wrong please correct me, thanks!

val logDirs = (config.logDirs.toSet + config.metadataLogDir).toSeq
val (rawMetaProperties, offlineDirs) = BrokerMetadataCheckpoint.
getBrokerMetadataAndOfflineDirs(logDirs, ignoreMissing = false)

@josefk31
Copy link
Copy Markdown
Contributor Author

@TaiJuWu That is very odd; I tested this on confluents infrastructure which is where the failure stems from... Since the same bits of 3.3.2 are being tested, I assumed that the failure was in AK. I will do more testing and then revert this PR once I discover more information.

@chia7712
Copy link
Copy Markdown
Member

Previous attempt at solving this left the metadata.log.dir empty which leads to the following crash log:

yes, using "empty string" can cause the error you attached, but in the e2e, the "empty string" will be filtered, so we actually remove the value of metadata.log.dir in the e2e.

filtered_configs = {k: v for k, v in configs.items() if v not in [None, ""]}

That is very odd; I tested this on confluents infrastructure which is where the failure stems from... Since the same bits of 3.3.2 are being tested, I assumed that the failure was in AK. I will do more testing and then revert this PR once I discover more information.

yes, please share more details to us. If we can't run upgrade/downgrade the server with single folder, we have to update our upgrade docs to include the (new) bugs ...

@TaiJuWu
Copy link
Copy Markdown
Collaborator

TaiJuWu commented May 15, 2025

Hi @josefk31 , any update for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

small Small PRs tests Test fixes (including flaky tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants