Testing 5.x

[12345ieee]

As promised in #709 (comment) to @hyder (also paging @devoncrouse ) I've started testing 5.x against one of my configurations.

My aim for now is to reach a clean (or equivalent) plan to what I have deployed right now and then go from there.
The text below is a bit of a consciousness stream, I appreciate a lot the work you guys are doing here, no offense intended.

My starting configuration is below. What I've noticed, in no particular order:

1. You need to merge 12345ieee/terraform-oci-oke@d8428be for the usual provider reasons, or I (and anyone else) can't even init terraform.

2. A lot of the module variable are severely underspecified, variable "worker_pools" { type = any } is by far the worst offender.
   Terraform 1.3 (which I suggest you adopt, up from 1.2) brings optional map parameters, I've started fully typing the map variables in my custom modules and I'm a big fan of it, but I understand my annoyance could also be mitigated by precise documentation.

3. Nearly every resource name changed, a ton of moved blocks should be added (Terraform version is already > 1.1 so you have them) and kept till, say, 5.1 to let people transition cleanly.
   I list them below from the PoV of the root module, but they should go in the appropriate (sub)module(s) of the oke module.
   Of course my set is non-exhaustive, but it makes my configuration plan with the destruction of seclists/nsg/nulls only, no real cloud recreation.

4. Nested tag maps, I get the same issue of Explicitely type tag map variables in oke submodule #709 and cluster tag doesn't allow more than 8 freeform tags. #698 , but there are also mistakes in parameter passing, which went uncaught due to the lack of explicit typing as discussed in 2.
   I fix them all in 12345ieee/terraform-oci-oke@224b121 , you need to merge it or nonempty tags literally break, but once again the maps should be fully typed.

5. It's not really possible to write moved blocks for seclists and nsgs, as the whole structure changed, but it's not really a problem, as they don't represent real hardware, I cannot be sure my configurations match, this is a case of testing and tweaking the parameters.

6. I'm missing a couple features, I list the associated variables:

# 4. Bastion
# bastion_state = "STOPPED" # TODO missing in 5.x
bastion_availability_domain = "sQTi:EU-FRANKFURT-1-AD-1" # TODO let me specify an ad number like in every other part of this module

# 6. Cluster
# dashboard_enabled       = true # TODO missing in 5.x, will you guys use the new addon thing?

I leave my new config at the bottom, this mostly plans cleanly now, but if I were to do this for real it'd need to do this on a test cluster, check the routing works, etc...
I have not reviewed the code itself except from the variables interface and what I needed to reverse engineer functionality, I can check parts if needed.

Nice work guys, looking forward to trying some new features, you can reply to this discussion if you'd like me to review or try some more stuff.

---

Old config:

module "oke" {
  source  = "oracle-terraform-modules/oke/oci"
  version = "4.5.9"

  # 1. OCI Provider parameters Identity and access
  tenancy_id  = data.oci_identity_compartment.root.id
  home_region = local.region
  region      = local.region

  # 2. General OCI
  compartment_id = module.compartment_working.id
  label_prefix   = var.ENVIRONMENT_NAME

  # 3. SSH Keys
  ssh_public_key_path  = "REDACTED.pub"
  ssh_private_key_path = "REDACTED"

  # 4. OCI Networking
  lockdown_default_seclist = false

  # vedi https://www.terraform.io/docs/language/functions/cidrsubnet.html
  subnets = {
    bastion = {
      netnum  = 0,
      newbits = 6
    }
    operator = {
      netnum  = 1,
      newbits = 6
    }
    cp = {
      netnum  = 2,
      newbits = 6
    }
    int_lb = {
      netnum  = 2,
      newbits = 3
    }
    pub_lb = {
      netnum  = 3,
      newbits = 3
    }
    fss = {
      netnum  = 4,
      newbits = 3
    }
    workers = {
      netnum  = 1,
      newbits = 1
    }
    pods = {
      netnum  = 2,
      newbits = 2
    }
  }

  vcn_cidrs = oci_core_vcn.REDACTED_vcn.cidr_blocks

  # 5. Reuse VCN
  create_vcn                   = false
  vcn_display_name             = oci_core_vcn.REDACTED_vcn.display_name
  vcn_id                       = oci_core_vcn.REDACTED_vcn.id
  ig_route_table_display_name  = oci_core_route_table.REDACTED_ig.display_name
  ig_route_table_id            = oci_core_route_table.REDACTED_ig.id
  nat_route_table_display_name = oci_core_route_table.REDACTED_nat.display_name
  nat_route_table_id           = oci_core_route_table.REDACTED_nat.id

  # 6. Bastion Host
  bastion_state = "STOPPED"
  bastion_type  = "private"

  # 8. Operator Host
  create_operator                    = false
  enable_operator_instance_principal = false

  # 9. Availability Domain
  availability_domains = { bastion = 1 }

  # 10. Tagging
  defined_tags = {
    vcn = var.DEFINED_TAGS
    oke = {
      cluster           = var.DEFINED_TAGS
      persistent_volume = var.DEFINED_TAGS
      service_lb        = var.DEFINED_TAGS
      node_pool         = var.DEFINED_TAGS
      node              = var.DEFINED_TAGS
    }
  }

  # 11. OKE
  allow_worker_ssh_access = true
  cluster_name            = "oke"
  control_plane_type      = "private"
  dashboard_enabled       = true
  kubernetes_version      = "v1.23.4"

  # 13. Node pools
  node_pools = {
    small  = { shape = "VM.Standard.E4.Flex", ocpus = 4, memory = 32, node_pool_size = 2, label = {}, placement_ads = [1] }
    medium = { shape = "VM.Standard.E4.Flex", ocpus = 8, memory = 64, node_pool_size = 0, label = {}, placement_ads = [1] }
    large  = { shape = "VM.Standard.E4.Flex", ocpus = 16, memory = 128, node_pool_size = 0, label = {}, placement_ads = [1] }
  }

  providers = {
    oci      = oci
    oci.home = oci
  }
}

---

Moved blocks:

moved {
  from = module.oke.module.oke.oci_containerengine_cluster.k8s_cluster
  to   = module.oke.module.cluster[0].oci_containerengine_cluster.k8s_cluster
}

moved {
  from = module.oke.module.oke.oci_containerengine_node_pool.nodepools
  to   = module.oke.module.workers[0].oci_containerengine_node_pool.workers
}

moved {
  from = module.oke.module.bastion[0].oci_core_subnet.bastion
  to   = module.oke.module.network.oci_core_subnet.oke["bastion"]
}

moved {
  from = module.oke.module.network.oci_core_subnet.cp
  to   = module.oke.module.network.oci_core_subnet.oke["cp"]
}

moved {
  from = module.oke.module.network.oci_core_subnet.pub_lb[0]
  to   = module.oke.module.network.oci_core_subnet.oke["pub_lb"]
}

moved {
  from = module.oke.module.network.oci_core_subnet.workers
  to   = module.oke.module.network.oci_core_subnet.oke["workers"]
}

---

New config:

module "oke" {
  source = "../../terraform-oci-oke"

  # 1. Provider
  tenancy_id  = data.oci_identity_compartment.root.id
  home_region = local.region
  region      = local.region

  # 2. Identity
  compartment_id = module.compartment_working.id
  state_id       = var.ENVIRONMENT_NAME

  use_defined_tags = true
  defined_tags = {
    cluster           = var.DEFINED_TAGS
    persistent_volume = var.DEFINED_TAGS
    service_lb        = var.DEFINED_TAGS
    workers           = var.DEFINED_TAGS
    bastion           = var.DEFINED_TAGS
    operator          = var.DEFINED_TAGS
    vcn               = var.DEFINED_TAGS
  }

  # 3. SSH
  ssh_public_key_path  = "REDACTED.pub"
  ssh_private_key_path = "REDACTED"

  # 4. Bastion
  # bastion_state = "STOPPED" # TODO missing in 5.x
  bastion_is_public           = false
  bastion_availability_domain = "sQTi:EU-FRANKFURT-1-AD-1" # TODO let me specify an ad number like in every other part of this module

  # 5. Operator
  create_operator = false

  # ??. Networking
  create_vcn         = false
  vcn_id             = oci_core_vcn.REDACTED_vcn.id
  ig_route_table_id  = oci_core_route_table.REDACTED_ig.id
  nat_route_table_id = oci_core_route_table.REDACTED_nat.id

  lockdown_default_seclist = false

  # vedi https://www.terraform.io/docs/language/functions/cidrsubnet.html
  subnets = {
    bastion = {
      netnum  = 0,
      newbits = 6
    }
    operator = {
      netnum  = 1,
      newbits = 6
    }
    cp = {
      netnum  = 2,
      newbits = 6
    }
    int_lb = {
      netnum  = 2,
      newbits = 3
    }
    pub_lb = {
      netnum  = 3,
      newbits = 3
    }
    fss = {
      netnum  = 4,
      newbits = 3
    }
    workers = {
      netnum  = 1,
      newbits = 1
    }
    pods = {
      netnum  = 2,
      newbits = 2
    }
  }

  vcn_cidrs = oci_core_vcn.REDACTED_vcn.cidr_blocks

  # 6. Cluster
  allow_worker_ssh_access = true
  cluster_name            = "${var.ENVIRONMENT_NAME}-oke"
  control_plane_is_public = false
  # dashboard_enabled       = true # TODO missing in 5.x, will you guys use the new addon thing?
  kubernetes_version = "v1.23.4"

  # 7. Workers
  worker_pools = {
    small  = { shape = "VM.Standard.E4.Flex", ocpus = 4, memory = 32, size = 2, label = {}, placement_ads = [1] }
    medium = { shape = "VM.Standard.E4.Flex", ocpus = 8, memory = 64, size = 0, label = {}, placement_ads = [1] }
    large  = { shape = "VM.Standard.E4.Flex", ocpus = 16, memory = 128, size = 0, label = {}, placement_ads = [1] }
  }

  providers = {
    oci      = oci
    oci.home = oci
  }
}

[lodotek]

With the deprecation of var. label_prefix, how do we avoid the names of our resources changing?

I noticed the introduction of var.internet_gateway_display_name, var.service_gateway_display_name, etc, but when I try to specify values for these, they are not recognized by the parent module, ie:

just a portion of this block for brevity:

module "oke" {
  source  = "oracle-terraform-modules/oke/oci"
  version = "5.0.0-RC4"
  service_gateway_display_name = "someValue"
  internet_gateway_display_name = "someValue"
}

Yields errors like An argument named "internet_gateway_display_name" is not expected here.

Please advise.

Thank you