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

panic when creating google_dataflow_flex_template_job #16799

Open
n-oden opened this issue Dec 14, 2023 · 11 comments
Open

panic when creating google_dataflow_flex_template_job #16799

n-oden opened this issue Dec 14, 2023 · 11 comments

Comments

@n-oden
Copy link

n-oden commented Dec 14, 2023

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.
  • If an issue is assigned to the modular-magician user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

v1.5.3

Affected Resource(s)

  • google_dataflow_flex_template_job

Terraform Configuration Files

resource "google_dataflow_flex_template_job" "ingest" {
  for_each = toset([var.name])

  name     = "ingest-${var.name}-${random_id.suffix[each.key].dec}"
  region   = var.region
  provider = google-beta
  project  = var.project

  skip_wait_on_job_termination = local.ingest_config[each.key].skip_wait_on_job_termination

  container_spec_gcs_path = local.ingest_config[each.key].container_spec_gcs_path
  parameters              = local.ingest_config[each.key].parameters
  labels                  = local.ingest_config[each.key].labels
  num_workers             = var.num_workers
  machine_type = (
    var.enable_prime == true
    ? null
    : var.machine_type
  )
  additional_experiments = (
    var.enable_prime == true
    ? ["enable_prime"]
    : ["enable_streaming_engine", "enable_windmill_service"]
  )

  lifecycle {
    create_before_destroy = true
    // waiting on https://github.com/hashicorp/terraform-provider-google/issues/9291 :(
    ignore_changes = [
      parameters["output_topic"]
    ]
  }

  on_delete = "drain"
}

Debug Output

Panic Output

Expected Behavior

  • create a dataflow flex template job

Actual Behavior

  • Panic! at the dataflow :(

Steps to Reproduce

  1. terraform apply

References

b/316686467

@n-oden n-oden added the bug label Dec 14, 2023
@github-actions github-actions bot added forward/review In review; remove label to forward service/dataflow labels Dec 14, 2023
@melinath
Copy link
Collaborator

@melinath melinath added this to the Post-5.0.0 milestone Dec 14, 2023
@melinath
Copy link
Collaborator

@n-oden I notice your configuration uses additional_experiments to set enable_streaming_engine - but it's also available as an (undocumented) top-level field on the resource:

resource "google_dataflow_flex_template_job" "ingest" {
    // ...
    enable_streaming_engine = true
}

Does that fix the issue for you?

@n-oden
Copy link
Author

n-oden commented Dec 14, 2023

@melinath moving that value to the resource field does in fact work around the panic!

@melinath
Copy link
Collaborator

Interesting! I did some more testing and was about to say that wouldn't fix the problem for you 😂 I was able to get things to work just fine whether enable_streaming_engine was set with the top-level field or using the additional_experiments field. But if I set parameters.enableStreamingEngine to true (the boolean value) then I get this panic, because it gets cast to a string "true" and that can't just be cast back to a boolean with .(bool).

Parameters were actually also previously being forced to be strings - we just weren't previously trying to cast that back into a boolean. I guess the API must accept either booleans or strings, so it worked fine back then?

The fix here would either be to make the casting back to a boolean work properly, or just always send a boolean to the API, or to try to do something fancier with the parameters (instead of converting it to a string). I'm not sure what kinds of values we should expect so I'm not sure what the best fix would be.

So: I think this is definitely still a valid issue & I'll forward it to the service team for resolution - but also I'm glad that there's a workaround that fixes it for you!

@melinath melinath removed the forward/review In review; remove label to forward label Dec 14, 2023
@melinath
Copy link
Collaborator

Additionally, it would be great to get the enable_streaming_engine documented - since this is a handwritten resource that would need to happen following the "Handwritten" instructions here: https://googlecloudplatform.github.io/magic-modules/develop/resource/#add-documentation

@n-oden
Copy link
Author

n-oden commented Dec 15, 2023

Yeah, somewhat ironically I'm the person who filed this issue some time ago, which seems relevant:

#9291

@damondouglas
Copy link

Thank you @n-oden for raising this issue. Is there a local.ingest_config[each.key].parameters with a value being set to "true" versus true (or "false" versus false)? The panic: interface conversion: interface {} is string, not bool error results from something like what is demonstrated in https://go.dev/play/p/zRL6M4_T9AI.

@melinath
Copy link
Collaborator

@damondouglas see #16799 (comment) for details on why this is sometimes being cast to a string.

@damondouglas
Copy link

But if I set parameters.enableStreamingEngine to true (the boolean value) then I get this panic, because it gets cast to a string "true" and that can't just be cast back to a boolean with .(bool).

The error parameters.enableStreamingEngine=true (boolean) makes sense because the point of parameters is to forward them as --option=value to the template specific parameters. See parameters field, quoting here: "Template specific Key/Value pairs to be forwarded to the pipeline's options; keys are case-sensitive based on the language on which the pipeline is coded, mostly Java. Note: do not configure Dataflow options here in parameters.".

@melinath
Copy link
Collaborator

melinath commented Jan 6, 2025

@damondouglas the error here is happening at the Terraform level when trying to communicate with the API, not from the API trying to communicate with the pipeline language. It is definitely an error in the provider code.

@melinath
Copy link
Collaborator

melinath commented Jan 6, 2025

Even if this were somehow a correct situation for the API to return an error, errors from the API shouldn't result in a panic, and that would still need to be fixed on the provider side. (But that's not what's happening since we're not even getting to the API before panicking.)

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

No branches or pull requests

5 participants