Change server/tags from List to Set

[mikegager]

I was encountering an annoyance where some of my server/tags would show changed with each plan and be removed and readded with each apply. It would complete the job successfully, just that it should not have been reporting changes being required each run.

I found that server/tags is defined as a TypeList, where it seemed it should be a TypeSet, which appears to be what the deprecated tags resource used.

This patch changes server/tags to be defined as a TypeSet, and updates the ExpandStrings utility method to work with the Set. This change resolved my issue and I did not notice any side-effects in my testing (server create, add tag, remove tag, server delete).

The change seems pretty trivial to me, but this is my first time editing Go, so I can only say that it appears to work, not that it is correct.

Thank you.

[kaminek]

Hi @mikegager Thanks for the effort, but I cannot reproduce the behavior you described above (even in the tests), can you please share your config?

[mikegager]

Thank you for having a look at this, and I'm sorry for the delay getting back to you (I've been swamped lately). I was afraid that this might be something relating to something specific to my account or its history (I only recently started here, trying to move into terraform where previously it had generally been managed through the WebUI). The odd part being that it appears to only happen with certain tags, though I haven't been able to find a meaningful pattern to them (and at one point tried removing one of them from everything, seeing that it didn't try to re-tag anything, then re-added it).

I am able to reproduce it on my setup with the attached minimal terraform. main.txt Note that in this example, it appears to only happen for the "stg" tag, and if I change the "env" variable to some random string instead, it appears to work as expected.

I'll also mention, as it is likely somewhat related, my normal setup is now a bit more elaborate using some modules to organize things. I will generally get some TAG_NOT_FOUND error messages for some of the tags, but AFAICT they don't cause any trouble finishing out the run, and don't obviously align with the ones that it wants to recreate on the following run.

Being that most of the changes look like the one pasted below, my first thought was that it had something to do with ordering, which is what got me thinking of making it a set rather than a list. Adding a toset() to the concat() in the terraform file was insufficient, so went looking a bit deeper for where I could try forcing it to be a set. I did some basic testing and it appeared to work, though I still haven't spent enough time with the "fixed" version to be super-confident in it.

  ~ tags     = [
      - "stg",
        "neverused11",
        "neverused12",
      + "stg",
    ]

Given the circumstances, I certainly will not be offended if you would like to reject this as unreproducible, especially as it seems to just be me, and I haven't been able to pin down the patterns involved yet. Thanks again.

[kaminek]

Hi @mikegager

Thanks for sharing your setup but still I cannot reproduce even with stg tag, here is my proof

❯ terraform show
# upcloud_server.server[0]:
resource "upcloud_server" "server" {
    cpu      = 1
    firewall = false
    hostname = "us-chi1-stg-mgagertest-1"
    id       = "009f97f3-c708-4282-9d24-70e5f70c5f23"
    mem      = 1024
    metadata = false
    plan     = "1xCPU-1GB"
    tags     = [
        "neverused11",
        "neverused12",
        "stg",
    ]
    title    = "us-chi1-stg-mgagertest-1 (managed by terraform)"
    zone     = "us-chi1"

    login {
        create_password   = false
        keys              = [
            "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIGejo0BNgvt/p7nq0tZRHooMMKT/zihO9c3huyrEkMnJ",
        ]
        password_delivery = "none"
        user              = "root"
    }

    network_interface {
        bootable            = false
        ip_address          = "209.50.53.242"
        ip_address_family   = "IPv4"
        ip_address_floating = false
        mac_address         = "f6:76:ab:f1:2c:ae"
        network             = "03000000-0000-4000-8049-000000000000"
        source_ip_filtering = true
        type                = "public"
    }

    template {
        address = "virtio"
        id      = "01b7c19b-601f-4132-a8ab-9568c52c9d4d"
        size    = 10
        storage = "Ubuntu Server 20.04 LTS (Focal Fossa)"
        tier    = "maxiops"
        title   = "terraform-us-chi1-stg-mgagertest-1-disk"
    }
}
misc/terraform/upcloud-provider-173 via 💠 default
❯ terraform plan
upcloud_server.server[0]: Refreshing state... [id=009f97f3-c708-4282-9d24-70e5f70c5f23]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

I don't really understand why it is randomly failing for you, do you have these tags managed somewhere else? the current API forces the uniqueness of tags for a given user and not resource. This tag update behaviors may be due to some delete/create of the given tag somewhere else in you configs (maybe using the deprecated tag resource) Let me know if you find out from where it comes from.

[kangasta]

Here is a simple config that I was able to reproduce this with:

terraform {
  required_providers {
    upcloud = {
      source  = "registry.upcloud.com/upcloud/upcloud"
      version = "~> 2.4"
    }
  }
}

resource "upcloud_server" "tag-test-34" {
  zone     = "pl-waw1"
  hostname = "tag-test-34"
  tags = [
    "tag-3",
    "tag-4"
  ]

  template {
    storage = "01000000-0000-4000-8000-000030200200"
    size    = 10
  }

  network_interface {
    type = "public"
  }
}

resource "upcloud_server" "tag-test-1234" {
  zone     = "pl-waw1"
  hostname = "tag-test-1234"
  tags = [
    # Run terraform apply once, uncomment these after that, and run terraform apply twice again
    # "tag-1",
    # "tag-2",
    "tag-3",
    "tag-4"
  ]

  template {
    storage = "01000000-0000-4000-8000-000030200200"
    size    = 10
  }

  network_interface {
    type = "public"
  }
}

[kangasta]

Thanks again for reporting this and proposing an solution. This might be a breaking change, so we will probably have to wait until next major version before merging this in. Before that, changes from #257 will supress diff if only the order of the tags changes.

[kangasta]

I'll close this as completed: the proposed changes were cherry-picked into #414 and have been since released