[BUG] machine_labels are 'sticky' to the position of the machine_pool in the list of machine_pools - Githubissues

rancher / terraform-provider-rancher2

Terraform Rancher2 provider

https://www.terraform.io/docs/providers/rancher2/

Mozilla Public License 2.0

260 stars 223 forks source link

[BUG] machine_labels are 'sticky' to the position of the machine_pool in the list of machine_pools #1254

Open hansbogert opened 11 months ago

hansbogert commented 11 months ago

Rancher Server Setup

Rancher version: 2.7.6
Installation option (Docker install/Helm Chart):
- If Helm Chart, Kubernetes Cluster and version (RKE1, RKE2, k3s, EKS, etc): rke2

Information about the Cluster

Kubernetes version: 1.26
Cluster Type (Local/Downstream): local
- If downstream, what type of cluster? (Custom/Imported or specify provider for Hosted/Infrastructure Provider):
  having machine labels in machine_pools causes unnecessary recreation of machine pools during deletion of other machine_pools. Worse though is that these newly recreated machine_pool machines now can have wrong machine_labels on them.
  
  The situation in which this actually occurs is explained below.
  
  To Reproduce
  
  Have a cluster with the following machine_pools (in pseudo-config)
```
machine_pools:
  - pool1
  - pool2
    machine_labels:
      foo: bar
  - pool3
```
  Remove pool1 using Terraform
  
  Actual Result
  
  in pseudo config:
```
machine_pools:
  - pool2
    machine_labels:
      foo: bar
  - pool3
    machine_labels:
      foo: bar
```
  Expected Result
  
  In pseudo config:
```
machine_pools:
  - pool2
    machine_labels:
      foo: bar
  - pool3
```
  Additional context
  
  There are multiple issues at play here:
  - machine_pools is a list. A set or map would've been a better choice.
  - The machine_labels is optional, but computed in the Terraform schema. Why is this computed? This is an edge case in the Terraform SDK, similar to this one
  If the machine_labels does not need to be computed, then this issue is easily solved by removing the computed attribute. I've verified that the behavior is then correct.

hansbogert commented 5 months ago

@a-blender @kkaempf a gentle nudge for this issue; Is everything clear, or is more info or reproduction details needed?

kkaempf commented 5 months ago

Reassigning for better visibility.

hansbogert commented 3 months ago

Is something blocking this? @snasovich @Jono-SUSE-Rancher