Terraform inconsistency with Redpanda vs confluent

[mattgodbolt]

Version & Environment

Redpanda version: (use rpk version): 22.2.8
OS: Ubuntu 22.04 Linux
Docker: n/a
Kubernetes: n/a
terraform: 0.13.6
mongey-kafka: 0.5.1

What went wrong?

When creating topics with mongey-kafka in terraform, there are certain required parameters that get "echoed back" as config key-value pairs, which makes them appear to need to be re-terraformed every time.

What should have happened instead?

Ideally the confluent-compatible expression should apply cleanly and remain so. Or else some kind of useful error/response. I appreciate this is a third-party tool but I think it's a result of RP "echoing" back parameters (or not) that it accepted during configuration.

How to reproduce the issue?

terraform apply with a configlike:

terraform {
  required_version = ">= 0.13"

  required_providers {
    kafka = {
      source  = "mongey/kafka"
      version = "~> 0.5.1"
    }
  }
}

provider "kafka" {
  alias = "redpanda"
  bootstrap_servers = [
    "yourserversehere"
  ]
}

resource "kafka_topic" "test" {
  provider           = kafka.redpanda
  name               = "godbolt_test"
  replication_factor = 3
  partitions         = 1
}

Note that it creates ok:

kafka_topic.test: Creating...
kafka_topic.test: Creation complete after 1s [id=godbolt_test]

Then try to terraform apply a second time (which should be a no-op):

  # kafka_topic.test will be updated in-place
  ~ resource "kafka_topic" "test" {
      ~ config             = {
          - "cleanup.policy"     = "delete" -> null
          - "partition_count"    = "1" -> null
          - "replication_factor" = "3" -> null
        }
        id                 = "godbolt_test"
        name               = "godbolt_test"
        partitions         = 1
        replication_factor = 3
    }

Note how the config block is echoing the default cleanup policy, and also replicating the partition counts and replication factors, as strings. Our workaround is to duplicate the required configuration of partisions and replications, and explicitly add the default cleanup policy:

resource "kafka_topic" "test" {
  provider           = kafka.redpanda
  name               = "godbolt_test"
  replication_factor = 3
  partitions         = 1
  config = {
    "cleanup.policy"     = "delete" # only for RP
    "partition_count"    = "1" # only for RP
    "replication_factor" = "3" # only for RP
  }
}

This workaround is not necessary when terraforming a confluent kafka broker setup.

[jcsp]

cleanup.policy is a known divergence between Redpanda and Kafka: Redpanda always makes this property "sticky" on new topics, such that if you create a compacted topic while the default is compacted, it won't suddenly stop being compacted if someone later changes the default to delete. There was some discussion about this recently here #7276

The partition count and replication factor part needs investigating. Redpanda's API behavior aside, it is kind of peculiar that this terraform code is reporting something like "partition_count" = "1" -> null -- presumably this is meant to mean "reset partition count to default", but would you really want it to behave that way? Like, if you create a topic with 10 partitions, the default is 1, and then you change the definition to remove the topics partition count, one surely wouldn't want 9 partitions to get deleted.

[emaxerrno]

partition_count and replication_count... I could have sworn that @mmaslankaprv had removed them.

good point of cleanup.policy from john

[graphcareful]

Yup partition_count and replication_factor were removed by @mmaslankaprv pretty recently here #6259. This change was also backported to v22.1 and v22.2.

Edit: Backport to v22.2.x PR here: #7488 , link to commit which made the cut for the 22.2.8 tag: https://github.com/redpanda-data/redpanda/blob/279d483252f0ba959ded2b52d6d010a8a7580cbf/src/v/kafka/server/handlers/describe_configs.cc

[mattgodbolt]

Thanks all for the colour on this, and for the replies. Appreciated.

[emaxerrno]

@mattgodbolt let me know if we should move this to a discussion or keep open as an issue.

[mattgodbolt]

I think this is probably cool as a discussion, I mentioned the whole issue to @patrickangeles in a call and he said to file an issue to start things moving along :) It's not hurting me (other than the partition_count stuff, which sounds like it'll be coming down in a newer RP version soon). The rest is kinda just "be aware" so maybe a "RedPanda and terraform best practices/considerations" doc is in order?

Thanks!

[emaxerrno]

got ya, moving to discussion then. yeah will be on the next release

[mattgodbolt]

Looks like some of these things are fixed! Thanks! However, looking at the remote replication/tiered storage and we hit similar problems:

resource "kafka_topic" "tradewinds_eval_rtsdk_raw" {
  provider           = kafka.redpanda_eval_prod
  name               = "tradewinds_eval_rtsdk_raw"
  replication_factor = 3
  partitions         = 1
  config             = {
    "retention.bytes"       = -1
    "cleanup.policy"        = "delete"  # we don't want anything deleted but if we don't specify, complains
#    "redpanda.remote.write" = true, # maybe not needed as it's a cluster default? terraform complains
    "redpanda.remote.read"  = false,
  }
}

Two issues in this example:

• if we don't specify the cleanup policy, terraform keeps wanting to reset it from "delete" back to "unset"
• if I try and specify "redpanda.remote.write" to be true (which is the cluster default but I want to specify it here). then it keeps thinking the setting needs to be changed:

  # kafka_topic.tradewinds_eval_rtsdk_raw will be updated in-place
  ~ resource "kafka_topic" "tradewinds_eval_rtsdk_raw" {
      ~ config             = {
            "cleanup.policy"        = "delete"
            "redpanda.remote.read"  = "false"
          + "redpanda.remote.write" = "true"
            "retention.bytes"       = "-1"
        }
        id                 = "tradewinds_eval_rtsdk_raw"
        name               = "tradewinds_eval_rtsdk_raw"
        partitions         = 1
        replication_factor = 3
    }

...and if I let it try:

kafka_topic.tradewinds_eval_rtsdk_raw: Modifying... [id=tradewinds_eval_rtsdk_raw]
kafka_topic.tradewinds_eval_rtsdk_raw: Still modifying... [id=tradewinds_eval_rtsdk_raw, 10s elapsed]
kafka_topic.tradewinds_eval_rtsdk_raw: Still modifying... [id=tradewinds_eval_rtsdk_raw, 20s elapsed]

Error: Error waiting for topic (tradewinds_eval_rtsdk_raw) to become ready: couldn't find resource (21 retries)

  on kafka.tf line 57, in resource "kafka_topic" "tradewinds_eval_rtsdk_raw":
  57: resource "kafka_topic" "tradewinds_eval_rtsdk_raw" {

Separately, if I try and configure a readreplica on a different cluster, it "works" the first time but then every subsequent terraform it complains as the "redpanda.remote.readreplica" = "aq-redpanda-ts" equivalent line it wants to reapply (and fails to do so).

config:

resource "kafka_topic" "tradewinds_eval_rtsdk_raw_replica" {
  provider           = kafka.redpanda_eval_replica
  name               = kafka_topic.tradewinds_eval_rtsdk_raw.name
  replication_factor = 3
  partitions         = 1
  config             = {
    "retention.bytes"             = -1
    "cleanup.policy"              = "delete"  # we don't want anything deleted but if we don't specify, complains
    "redpanda.remote.readreplica" = "aq-redpanda-ts"  # this works the first time but terraform wants to reapply every time
  }
  depends_on = [kafka_topic.tradewinds_eval_rtsdk_raw]
}

On this last point even rpk topic describe doesn't show any indication the readreplica is set - though empirical observations shows the replication appears to be working.

[mattgodbolt]

in fact I just tried reapplying any change to the tradewinds_eval_rtsdk_raw and it's hanging with Error waiting for topic (tradewinds_eval_rtsdk_raw) to become ready: couldn't find resource (21 retries) now (I'm not sure what's going on behind the scenes).

this is with (old) terraform v0.13.6 but latest provider registry.terraform.io/mongey/kafka v0.5.2

[emaxerrno]

cc: @piyushredpanda @mattschumpert

[mattgodbolt]

Thanks @emaxerrno :)

[emaxerrno]

the team is back next week, so we'll chase.

[piyushredpanda]

Thanks for letting us know; I'm away from my laptop but know there were some Tiered Storage default config handling in #7602 (by @jcsp). But will circle back more completely next week.