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

interpolate environment for services in script checks #6916

Merged
merged 1 commit into from
Jan 9, 2020

Conversation

tgross
Copy link
Member

@tgross tgross commented Jan 8, 2020

Fixes #6836

In 0.10.2 (specifically #6586 / 387b016) we added interpolation to group service blocks and centralized the logic for task environment interpolation. This wasn't also added to script checks, which caused a regression where the IDs for script checks for services w/ interpolated fields (ex. the service name) didn't match the service ID that was registered with Consul.

This changeset calls the same taskenv interpolation logic during script_check configuration, and adds tests to reduce the risk of future regressions by comparing the IDs of service hook and the check hook.


In addition to the added unit test, I've exercised this with the following jobspec. If you run nomad job run example.nomad with this you'll see nginx-0 as the service name and the checks work, and if you update the value of PROJECT_VERSION that'll be properly reflected in the service name.

job "example" {
  datacenters = ["dc1"]

  group "webservers" {
    count = 1
    task "nginx" {
      driver = "docker"

      config {
        image = "0x74696d/nginx-with-curl:latest"
        port_map = {
          http = 80
        }
      }

      env {
        PROJECT_VERSION = "0"
      }

      service {
        name = "nginx-${PROJECT_VERSION}"
        port = "http"

        check {
          type     = "script"
          command = "/usr/bin/curl"
          args = ["--fail", "http://localhost/"]
          interval = "5s"
          timeout  = "3s"
        }
      }

      resources {
        memory = 128

        network {
          mbits = 10
          port "http" {}
        }
      }
    }
  }
}

cc @jorgemarey

In 0.10.2 (specifically 387b016) we added interpolation to group
service blocks and centralized the logic for task environment
interpolation. This wasn't also added to script checks, which caused a
regression where the IDs for script checks for services w/
interpolated fields (ex. the service name) didn't match the service ID
that was registered with Consul.

This changeset calls the same taskenv interpolation logic during
`script_check` configuration, and adds tests to reduce the risk of
future regressions by comparing the IDs of service hook and the check hook.
@tgross tgross added this to the 0.10.3 milestone Jan 8, 2020
@tgross tgross added this to Triaged in Nomad - Community Issues Triage via automation Jan 8, 2020
@tgross tgross moved this from Triaged to In Review in Nomad - Community Issues Triage Jan 8, 2020
@tgross tgross removed this from In Review in Nomad - Community Issues Triage Jan 8, 2020
@tgross tgross merged commit f31482a into master Jan 9, 2020
@tgross tgross deleted the b-script-check-interpolation branch January 9, 2020 13:12
tgross added a commit that referenced this pull request Jan 9, 2020
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Script checks fails to update TTL
2 participants