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

Schema.Set adds an empty value when attempting to Patch #1248

Open
BBBmau opened this issue Sep 14, 2023 · 1 comment
Open

Schema.Set adds an empty value when attempting to Patch #1248

BBBmau opened this issue Sep 14, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@BBBmau
Copy link
Contributor

BBBmau commented Sep 14, 2023

SDK version

github.com/hashicorp/terraform-plugin-sdk/v2@6a82eb66c240000ef148c6defa6f77ac1029128c

Relevant provider source code

	if d.HasChange(prefix + "metric") {
		ops = append(ops, &ReplaceOperation{
			Path:  pathPrefix + "/metrics",
			Value: expandV2Beta2Metrics(d.Get(prefix + "metric").(*schema.Set).List()),
		})
	}

Terraform Configuration Files

resource "kubernetes_deployment" "my_app_deployment" {
	metadata {
		name = "my-app-deployment"
		labels = {
			app = "my-app"
		}
	}

	spec {
		revision_history_limit = 3

		selector {
			match_labels = {
				app = "my-app"
			}
		}

		template {
			metadata {
				labels = {
					app = "my-app"
				}
			}

			spec {
				container {
					name = "my-app"
					image = "nginx"
					image_pull_policy = "Always"

					port {
						container_port = 80
					}

					resources {
						requests = {
							cpu = "200m"
							memory = "200Mi"
						}
						limits = {
							cpu    = "600m"
							memory = "600Mi"
						}
					}
				}
			}
		}
	}
}

resource "kubernetes_service" "my_app_service" {
	metadata {
		name = "my-app-service"
	}

	spec {
		type = "ClusterIP"
		selector = {
			app = "my-app"
		}

		port {
			protocol = "TCP"
			port = 80
			target_port = 80
		}
	}
}

resource "kubernetes_horizontal_pod_autoscaler_v2beta2" "test" {
  metadata {
    name = "my-app-hpa"

    annotations = {
      test = "test"
    }

    labels = {
      test = "test"
    }
  }

  spec {
    max_replicas = 10

    scale_target_ref {
      kind = "Deployment"
      name = "TerraformAccTest"
    }

    behavior {
      scale_down {
        stabilization_window_seconds = 300
        select_policy                = "Min"

        policy {
          period_seconds = 120
          type           = "Pods"
          value          = 1
        }

        policy {
          period_seconds = 310
          type           = "Percent"
          value          = 100
        }
      }

      scale_up {
        stabilization_window_seconds = 600
        select_policy                = "Max"

        policy {
          period_seconds = 180
          type           = "Percent"
          value          = 100
        }

        policy {
          period_seconds = 600
          type           = "Pods"
          value          = 5
        }
      }
    }

    metric {
      type = "Resource"
      resource {
        name = "test"
        target {
          type                = "Utilization"
          average_utilization = 1
        }
      }
    }

    metric {
      type = "External"
      external {
        metric {
          name = "queue_size"
          selector {
            match_labels = {
              queue_name = "test"
            }
          }
        }
        target {
          type  = "Value"
          value = "10"
        }
      }
    }

    metric {
      type = "Pods"
      pods {
        metric {
          name = "packets-per-second"
        }
        target {
          type          = "AverageValue"
          average_value = "1k"
        }
      }
    }

    metric {
      type = "Object"
      object {
        metric {
          name = "requests-per-second"
        }
        described_object {
          kind        = "Ingress"
          name        = "main-route"
          api_version = "networking.k8s.io/v1beta1"
        }
        target {
          type          = "AverageValue"
          average_value = "2k"
        }
      }
    }
  }
}

# resource "kubernetes_horizontal_pod_autoscaler_v2beta2" "test" {
#   metadata {
#     name = "my-app-hpa"

#     annotations = {
#       test = "test"
#     }

#     labels = {
#       test = "test"
#     }
#   }

#   spec {
#     min_replicas = 50
#     max_replicas = 100

#     scale_target_ref {
#       kind = "Deployment"
#       name = "TerraformAccTest"
#     }

#     behavior {
#       scale_down {
#         stabilization_window_seconds = 100
#         select_policy                = "Max"

#         policy {
#           period_seconds = 120
#           type           = "Pods"
#           value          = 10
#         }
#       }

#       scale_up {
#         select_policy = "Disabled"

#         policy {
#           period_seconds = 60
#           type           = "Pods"
#           value          = 1
#         }
#       }
#     }

#     metric {
#       type = "External"
#       external {
#         metric {
#           name = "latency"
#           selector {
#             match_labels = {
#               lb_name = "test"
#             }
#           }
#         }
#         target {
#           type  = "Value"
#           value = "100"
#         }
#       }
#     }
#   }
# }

The Error message comes in when you comment the first horizontal pod autoscaler resource and then comment out the second one causing a Patch

Debug Output


│ Error: Failed to update horizontal pod autoscaler: HorizontalPodAutoscaler.autoscaling "my-app-hpa" is invalid: [spec.metrics[0].type: Required value: must specify a metric source type, spec.metrics[0].type: Unsupported value: "": supported values: "ContainerResource", "External", "Object", "Pods", "Resource"]

│ with kubernetes_horizontal_pod_autoscaler_v2beta2.test,
│ on main.tf line 235, in resource "kubernetes_horizontal_pod_autoscaler_v2beta2" "test":
│ 235: resource "kubernetes_horizontal_pod_autoscaler_v2beta2" "test" {

Expected Behavior

Should Patch normally and have only one metric being the External Type

Actual Behavior

We get an error that says a type is invalid due to their being an empty string for type, when looking at the trace you can see that it actually attempts to Patch with two metric blocks. One metric config from the tfconfig and another metric that's completely empty and not part of the tfconfig. The empty metric addition is what's causing this error. Their should only be one metric being Patch and not two.

Steps to Reproduce

  1. terraform apply
  2. comment out the first horizontal pod resource then uncomment the second horizontal pod resource in the tfconfig
  3. terraform apply

References

@BBBmau BBBmau added the bug Something isn't working label Sep 14, 2023
@ziyeqf
Copy link

ziyeqf commented Nov 8, 2023

Just to adding some more context as it might be the same issue.
When the Set schema contains a Map element and the state contains that scheme but tfconfig does not contain, call d.Get() on a set, there will be a zero value element in the returned set.

here is some diagnostic context:

  1. in the above case, d.get() is returned in this road:
    d.get() <- d.multiReader.ReadFieldMerge() <- DiffFieldReader.ReadField() <- DiffFieldReader.readSet()
  2. inside DiffFieldReader.readSet(), it does not ignore the *.% field code link (might be the root cause?)
  3. then, the following code will read the already removed set, which will get zero values with Exists==true
  4. the 3. result is different from the zero value check of d.get(), then the bug happens.

I'm not sure the diagnostic is 100% correct but just hope it could help with bugfix.

ziyeqf added a commit to ziyeqf/terraform-provider-azurerm that referenced this issue Nov 8, 2023
osfidelity pushed a commit to fidelity-contributions/hashicorp-terraform-provider-azurerm-archive that referenced this issue Nov 21, 2023
…hashicorp#23821)

* update test to sequential run

* typo

* update exists func

* update test case

* adding workaround for hashicorp/terraform-plugin-sdk#1248
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants