Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor validation of pool members / idempotency #42

Open
sbrinkerhoff opened this issue Mar 7, 2017 · 2 comments
Open

Poor validation of pool members / idempotency #42

sbrinkerhoff opened this issue Mar 7, 2017 · 2 comments

Comments

@sbrinkerhoff
Copy link
Contributor

sbrinkerhoff commented Mar 7, 2017

When attempting to add nodes to a bigip_ltm_pool there appears to be limited validation and error checking which results in incorrect output, and subsequent runs attempt to correct and silently fail.

This code will suggest that it added two nodes..

Invalid Names

No errors are thrown for either node below, the first one is invalid for no domain, neither are real nodes. Additionally they are missing service nodes which makes them ineligible to be added.

variable "pool_name" { default="test-pool" }
variable "pool_nodes" { type="list" default=["no-domain","/Common/not-a-real-node"]}
variable "allow_snat" { default = true }
variable "allow_nat" { default = true }
variable "monitors" { type = "list", default = []}

resource "bigip_ltm_pool" "pool" {
  name = "${var.pool_name}"
  nodes = "${var.pool_nodes}"
  allow_snat = "${var.allow_snat}"
  allow_nat = "${var.allow_nat}"
  monitors = "${var.monitors}"
}
  nodes.#:             "1" => "2"
  nodes.2161144047:    "" => "no-domain"
  nodes.596047430:     "" => "/Common/not-a-real-node"

Service Port Missing

Nodes can be defined without a service port, and the bigip_ltm_pool provider does not return a failure in adding them, similar to the error above.

Idempotency

Even when valid node names are used, the ID seems to change each run and causes Terraform to state that the nodes were updated (and likely were disruptively added/removed from the pool.

Note the F5 seemingly is returning node names without the domain below, which Terraform seemingly removes and replaces with the names containing a domain prefix.

  monitors.2541227442: "" => "http"
  monitors.576276785:  "/Common/http" => ""
  nodes.1615781422:    "dc4plat-xxxxxxx:80" => ""
  nodes.2161144047:    "" => "/Common/dc4plat-xxxxxxx:80"
  nodes.3273152647:    "dc4plat-xxxxxxx:80" => ""
  nodes.596047430:     "" => "/Common/dc4plat-xxxxxxx:80"
@jakauppila
Copy link
Contributor

@sbrinkerhoff What do you mean by the following?

first one is invalid for no domain

Just that it doesn't include the partition on it?

In terms of idempotency, I have a PR on the upstream library to address this, scottdware/go-bigip#31. I'll get the required changes in here once it's accepted there.

@mattcanty
Copy link

@jakauppila do you have a binary available of the bigip provider with this idempotency fix? I've been struggling to build the solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants