Add JBOD support to KRaft mode

[scholzj]

Type of change

• Enhancement / new feature

Description

This PR adds JBOD support to KRaft-based Apache Kafka cluster. It is implemented based on the Strimzi Proposal SP#67.

Some notable comments about the implementation:

• It does not use an annotation to mark what volume is currently used for the KRaft metadata. It instead detects it from the existing storage configuration stored in an annotation already.
• The check if multiple volumes are marked for KRaft metadata is done in the StorageDiff class. While it does not fit into it based on how it is done, having it in the same class allows us to reject this change while continuing the reconciliation process instead of just throwing an exception.
• KRaft examples are updates as discussed in the proposal
• A check for the Kafka version was not explicitly mentioned in the proposal but is implemented in this PR to prevent users from deploying Kafka 3.6.x clusters with JBOD by mistake (unfortunately, this caused lot of changes as a new field describing the Kafka version had to be passed into the validation, but I believe it is worth it)
• Issue [ST] Adapt JBOD based system tests to KRaft #9938 was opened to track the JBOD-based STs that have not yet been adapted to KRaft

This should resolve #9437.

Checklist

• Write tests
• Make sure all tests pass
• Update documentation
• Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
• Reference relevant issue(s) and close them after merging
• Update CHANGELOG.md

[scholzj]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

/azp run kraft-regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

/azp run kraft-regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

/azp run migration

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

/azp run migration

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[PaulRMellor]

Looks great. I left a few suggestions.

[scholzj]

/azp run migration

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

/azp run kraft-regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ppatierno]

I had a first pass adding some comments. I will have a run before adding more feedback (if any).

[ppatierno]

@scholzj while testing this from a KRaft migration perspective I saw the following issue ...
With the current 0.40.0 (but not supporting more than one JBOD disk in KRaft of course) when you are rolling back, the __cluster_metadata folder is deleted when you back to ZooKeeper. It's happening with the following snipped of code in the kafka_run script when "no KRaft" is detected (see Using KRaft [false] log).

KRAFT_LOG_DIR=$(grep "log\.dirs=" /tmp/strimzi.properties | sed "s/log\.dirs=*//")

  # when in ZooKeeper mode, the __cluster_metadata folder should not exist.
  # if it does, it means a KRaft migration rollback is ongoing and it has to be removed.
  if [ -d "$KRAFT_LOG_DIR/__cluster_metadata-0" ]; then
    echo "Removing __cluster_metadata folder"
    rm -rf "$KRAFT_LOG_DIR/__cluster_metadata-0"
  fi

Of course it can't work when the log.dirs is set with multiple disks, so the check on the existence of a folder fails (because it's not just a single folder but a list).
So I have locally that part changed this way ...

# when in ZooKeeper mode, the __cluster_metadata folder should not exist.
 # if it does, it means a KRaft migration rollback is ongoing and it has to be removed.
 # also checking that metadata state is ZK (0), because if it's MIGRATION (2) it means we are rolling back but not finalized yet and KRaft quorum is still in place.
  CURRENT_KRAFT_METADATA_LOG_DIR=$(ls -d /var/lib/kafka/data-*/kafka-log"$STRIMZI_BROKER_ID"/__cluster_metadata-0 2> /dev/null || true)
  if [[ -d "$CURRENT_KRAFT_METADATA_LOG_DIR" ]] && [ "$STRIMZI_KAFKA_METADATA_CONFIG_STATE" -eq 0 ]; then
    echo "Removing __cluster_metadata folder"
    rm -rf "$CURRENT_KRAFT_METADATA_LOG_DIR"
  fi

Deleting the __cluster_metadata at the end of the rollback also helps us to hit the following KAFKA-16463 which is not fixed yet and needs that metadata folder being deleted before restarting a migration again.

The MigrationST wasn't able to catch this because it doesn't support multiple JBOD disk yet so the check that __cluster_metadata was deleted is hard coded this way.

https://github.com/strimzi/strimzi-kafka-operator/blob/main/systemtest/src/test/java/io/strimzi/systemtest/migration/MigrationST.java#L684

I also think that the check is testing a "wrong" folder, maybe because the tests are not using JBOD at all (even with just one disk) but the persistent storage so the path is just /var/lib/kafka/data/.

@im-konge I think the test should be fixed by using JBOD and even with multiple disks support when this PR is merged.

[scholzj]

@ppatierno Can you maybe comment on some exact parts of the code? Because the comment is quite confusing and it is not clear to me what parts are referring to what code etc.

Also please keep in mind that migration with JBOD has a separate task and this PR does not really intend to enable migration with JBOD in any way (I assume there are some checks etc.).

[ppatierno]

Also please keep in mind that migration with JBOD has a separate task and this PR does not really intend to enable migration with JBOD in any way

I know but this PR is enabling JBOD disks in KRaft and migration rollback is not going to work from this perspective anymore.
I am anyway fine to have the migration (rollback) back to work with a different PR after this was is merged.
Maybe it's more understandable.

(I assume there are some checks etc.).

The check about not allowing migration with multiple JBOD disk in 0.40.0 release relies on the NodePoolUtils.validateNodePools calling validateKRaftJbodStorage that was changed by this PR. The 0.40.0 returns the following error ...

The Kafka cluster my-cluster is invalid: [Using more than one disk in a JBOD storage is currently not supported when the UseKRaft feature gate is enabled (in KafkaNodePool kafka)]                                                       

Now that validation additional check KafkaVersion.compareDottedVersions(versionChange.to().version(), "3.7.0") < 0 "broke" that validation which passes.

So the current PR allows the migration with multiple JBOD disks and it works fine but has a problem on the rollback which I described.
As I said, we can leave this PR as it is and I will fix migration rollback in a different PR.

[ppatierno]

As already written, I can fix the migration rollback with multiple JBOD disks in a different PR so this one LGTM.

[scholzj]

/azp run kraft-regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).