civo / terraform-provider-civo

Terraform Civo provider
https://www.civo.com
Mozilla Public License 2.0
71 stars 56 forks source link

[BUG] Changing `kubernetes_version` after creation for civo_kubernetes_cluster does nothing #261

Closed fernando-villalba closed 2 months ago

fernando-villalba commented 2 months ago

Description

Issue

Changing kubernetes_version after creation for civo_kubernetes_cluster does nothing


provider "civo" {
  region = "LON1"
}

data "civo_size" "xsmall" {
    filter {
        key = "type"
        values = ["kubernetes"]
    }

    sort {
        key = "ram"
        direction = "asc"
    }
}

resource "civo_kubernetes_cluster" "my-cluster" {
    name = "my-cluster"
  #  kubernetes_version = "1.29.2+k3s1" # uncomment after creation

    firewall_id = civo_firewall.my-firewall.id
    pools {
        label = "front-end" // Optional
        size = element(data.civo_size.xsmall.sizes, 0).name
        node_count = 2
    }
}

Acceptance criteria

Screenshots

No response

Additional information

No response

Praveen005 commented 2 months ago

Hi @uzaxirr,

I tried solving this issue. If you allow, I would like to share my approach.

uzaxirr commented 2 months ago

@Praveen005 sure go ahead and raise a PR

Praveen005 commented 2 months ago

For this,

I commented out, the following:

https://github.com/civo/terraform-provider-civo/blob/8eeb57061a2f4ba234f6d74aaa4e4c732a3f2cb6/civo/kubernetes/resource_kubernetes_cluster.go#L373-L375

Reason:

If I keep this part as is, update won't go through, unless you have made changes to the pools attribute. which is not the case here.

Also, we will update the config and remove the return statement. https://github.com/civo/terraform-provider-civo/blob/8eeb57061a2f4ba234f6d74aaa4e4c732a3f2cb6/civo/kubernetes/resource_kubernetes_cluster.go#L406-L410

I tested it, and it does upgrade the version.

One caveat: Downgrading of version doesn't go through, and needs to handled on the API side.

uzaxirr commented 2 months ago

@Praveen005 looks good can you raise a PR for this make sure to attach relevant screenshots

Praveen005 commented 2 months ago

Sure. Doing right away.