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

removing liveness_probe declaration from google_cloud_run_v2_service does not remove the probe from the service #17468

Open
breathe opened this issue Mar 3, 2024 · 3 comments
Labels
forward/linked persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work service/run size/s
Milestone

Comments

@breathe
Copy link

breathe commented Mar 3, 2024

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 a user, that user is claiming responsibility for the issue.
  • Customers working with a Google Technical Account Manager or Customer Engineer can ask them to reach out internally to expedite investigation and resolution of this issue.

Terraform Version

  • Latest terraform as installed by hashicorp/setup-terraform@v3 github action
  • using hashicorp/google v5.18.0

Affected Resource(s)

google_cloud_run_v2_service

Terraform Configuration

resource "google_cloud_run_v2_service" "service" {
  name     = local.SERVICE
  location = var.GOOGLE_CLOUD_REGION

  template {

    max_instance_request_concurrency = 250

    scaling {
      max_instance_count = 1
    }
    containers {

      resources {
        cpu_idle = false
      }

      image = var.IMAGE

      startup_probe {
        failure_threshold     = 3
        initial_delay_seconds = 10
        timeout_seconds       = 5
        period_seconds        = 5

        tcp_socket {
          port = 8080
        }
      }

      # NOTE: due to a terraform bug, removing the liveness_probe configuration doesn't actually unconfigure the liveness probe ...
      # liveness_probe {
      #   failure_threshold = 7
      #   timeout_seconds   = 10
      #   period_seconds    = 10

      #   http_get {
      #     path = "/"
      #   }
      # }
}

Debug Output

No response

Expected Behavior

No response

Actual Behavior

No response

Steps to reproduce

  1. Create a cloud run v2 service with a liveness_probe declaration: terraform apply
  2. Modify configuration to remove the liveness_probe config
  3. re-run terraform plan/apply and you'll notice that it doesn't actually remove the liveness_probe related configuration from the newly deployed cloud_run revision

Important Factoids

No response

References

No response

b/328077381

@breathe breathe added the bug label Mar 3, 2024
@github-actions github-actions bot added forward/review In review; remove label to forward service/run labels Mar 3, 2024
@breathe breathe changed the title removing liveness_probe declaration from google_cloud_run_v2_service does not remove the probe removing liveness_probe declaration from google_cloud_run_v2_service does not remove the probe from the service Mar 3, 2024
@rileykarson
Copy link
Collaborator

This field was marked as Optional+Computed (default_from_api in our code generator) which preserves the existing value when a field is removed from config (specifically is null, but that's how most users will null a field). If it were just Optional, nulling the field would indicate removal rather than preservation.

This means the behaviour is correct as written, but could very well be wrong- it was marked as such in GoogleCloudPlatform/magic-modules#6987. That's an unusual change, and I can't tell if it was intended or not, as it doesn't seem to have been discussed in review.

If there's any situation that Cloud Run will provide a liveness_probe value when one wasn't sent, marking it this way was correct and the surprise behaviour you're seeing is a side effect. However if it isn't, we should remove Computed: true in a major release so that removing the block no longer preserves the value.

@rileykarson rileykarson added persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work and removed bug forward/review In review; remove label to forward labels Mar 4, 2024
@melinath melinath added the size/s label Mar 4, 2024
@melinath melinath added this to the Goals milestone Mar 4, 2024
@breathe
Copy link
Author

breathe commented Mar 4, 2024

If there's any situation that Cloud Run will provide a liveness_probe value when one wasn't sent, marking it this way was correct and the surprise behaviour you're seeing is a side effect. However if it isn't, we should remove Computed: true in a major release so that removing the block no longer preserves the value.

I believe that cloud_run does provide a default value when one is not supplied for the startup_probe (there must be a startup_probe defined).

I'm not completely sure what the behavior is with respect to liveness_probe -- if there is one enabled by default its operation doesn't produce any visible gcloud logs -- and while google explicitly documents the default behavior of the startup_probe I don't find similar language anywhere for the liveness_probe ...

When I remove the liveness_probe configuration parameter via the ui -- nothing about it is visible in the ui any longer ...

@AndreaGiardini
Copy link

@breathe @rileykarson I think this issue can be closed?

Could you please double-check if #20753 is the same problem on a different field? Happy to open a PR if that's the case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
forward/linked persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work service/run size/s
Projects
None yet
Development

No branches or pull requests

5 participants