aiven / terraform-provider-aiven

Aiven Terraform Provider
https://registry.terraform.io/providers/aiven/aiven/latest/docs
MIT License
129 stars 70 forks source link

upgrading to v3.13.0 or higher fails when having aiven_kafka_topic resources with read timeout #1072

Closed wanisfahmyDE closed 1 year ago

wanisfahmyDE commented 1 year ago

Hello,

After upgrading to release v3.13.0 or v4.0.0, we get the below error when running terraform plan. It seems the read property in the timeout block is not accepted anymore.

Is this a bug in the provider or are we doing something wrong?

This issue doesn't occur in v3.12.1

Error: Unsupported argument 

on topics.tf line 26, in resource "aiven_kafka_topic" "kafka_topic": 
26: read = try(each.value.timeouts.read, "5m") 

An argument named "read" is not expected here.
resource "aiven_kafka_topic" "kafka_topic" {
  for_each = local.topics

  project                = data.aiven_project.aiven_project.project
  service_name           = aiven_kafka.aiven_kafka.service_name
  topic_name             = try(each.value.topic_name, each.key)
  partitions             = try(each.value.partitions, 5)
  replication            = try(each.value.replication, 3)
  termination_protection = try(each.value.termination_protection, false)

  config {
    retention_ms              = try(each.value.config.retention_ms, 21600000)
    cleanup_policy            = try(each.value.config.cleanup_policy, "delete")
    min_insync_replicas       = try(each.value.config.min_insync_replicas, try(each.value.replication, 3) - 1)
    segment_ms                = try(each.value.config.segment_ms, 604800000)
    segment_bytes             = try(each.value.config.segment_bytes, 209715200)
    delete_retention_ms       = try(each.value.config.delete_retention_ms, 86400000)
    min_cleanable_dirty_ratio = try(each.value.config.min_cleanable_dirty_ratio, 0.5)
    max_message_bytes         = try(each.value.config.max_message_bytes, 1048588)
    message_timestamp_type    = try(each.value.config.message_timestamp_type, "CreateTime")
    retention_bytes           = try(each.value.config.retention_bytes, -1)
  }

  timeouts {
    create = try(each.value.timeouts.create, "1m")
    read   = try(each.value.timeouts.read, "5m")
  }
}

Thank you for the support

wanisfahmyDE commented 1 year ago

it seems that the result from this PR is causing the issue: https://github.com/aiven/terraform-provider-aiven/pull/1063 but it seems there is no replacement for "read" ?

wanisfahmyDE commented 1 year ago

@Serpentiel @ivan-savciuc any thoughts on that?

Serpentiel commented 1 year ago

hey, @wanisfahmyDE! πŸ‘‹

thanks for reporting this! I just got back from a vacation and will take a look now 🌴

Serpentiel commented 1 year ago

looks like https://github.com/aiven/terraform-provider-aiven/pull/1080 is supposed to fix this issue

we'll roll out a minor release soon so you'll have a chance to check whether it works or not :)

please let us know at the end πŸ™

wanisfahmyDE commented 1 year ago

sure thing, will try it out once released and give feedback.

Serpentiel commented 1 year ago

could you please see if v4.1.0 resolves your issue?

wanisfahmyDE commented 1 year ago

@Serpentiel works, thank you! it would be really nice if breaking changes are following semantic versioning along with adding the commit references to the release notes πŸ™

Serpentiel commented 1 year ago

sorry, it was not supposed to be a breaking change πŸ˜„

also, we'll consider switching to something like Conventional Changelog or Keep a Changelog

wanisfahmyDE commented 1 year ago

@Serpentiel just sharing what we use in our org, maybe it helps.

we're using release-drafter to craft the releases with notes and works well with semantic versioning and conventional commits.

here is the workflow and release config:

name: Release
on:
  push:
    tags:
      - v*.*.*

jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v3
        with:
          fetch-depth: 0

      - name: version
        id: version
        run: |
          tag=${GITHUB_REF/refs\/tags\//}
          version=${tag#v}
          major=${version%%.*}
          echo "tag=${tag}" >> $GITHUB_OUTPUT
          echo "version=${version}" >> $GITHUB_OUTPUT
          echo "major=${major}" >> $GITHUB_OUTPUT

      - uses: release-drafter/release-drafter@v5
        with:
          version: ${{ steps.version.outputs.version }}
          publish: true
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

      - name: force update major tag
        run: |
          git tag v${{ steps.version.outputs.major }} ${{ steps.version.outputs.tag }} -f
          git push origin refs/tags/v${{ steps.version.outputs.major }} -f
name-template: 'v$RESOLVED_VERSION'
tag-template: 'v$RESOLVED_VERSION'
categories:
  - title: 'πŸš€ Features'
    labels:
      - 'feature'
      - 'enhancement'
  - title: 'πŸ› Bug Fixes'
    labels:
      - 'fix'
      - 'bugfix'
      - 'bug'
  - title: '🧰 Maintenance'
    label: 'chore'
change-template: '- $TITLE @$AUTHOR (#$NUMBER)'
change-title-escapes: '\<*_&' # You can add # and @ to disable mentions, and add ` to disable code blocks.
autolabeler:
  - label: 'chore'
    files:
      - '*.md'
    branch:
      - '/docs{0,1}\/.+/'
  - label: 'bug'
    branch:
      - '/fix\/.+/'
    title:
      - '/fix/i'
  - label: 'enhancement'
    branch:
      - '/feature\/.+/'
      - '/feat\/.+/'
version-resolver:
  major:
    labels:
      - 'major'
  minor:
    labels:
      - 'enhancement'
  patch:
    labels:
      - 'patch'
      - 'bug'
  default: patch
template: |
  ## Changes

  $CHANGES
Serpentiel commented 1 year ago

hey, @wanisfahmyDE! thanks a lot!

ebachle commented 1 year ago

Hey @Serpentiel is there any chance this could get backported into the 3.x line? The issue got introduced in 3.x but was only fixed in 4.x, so if you run 3.13.0, 3.13.1, 3.13,2, or 3.13.3, you're still going to experience this. We're currently pinned to ~> 3.12.0 to avoid this.

Serpentiel commented 1 year ago

hey, @ebachle!

of course! as I can see, my colleague has already prepared a PR which adds this

ebachle commented 1 year ago

hey, @ebachle!

of course! as I can see, my colleague has already prepared a PR which adds this

Awesome, thanks so much!

byashimov commented 1 year ago

Hey @ebachle. Released as v3.13.4