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

allow specifying check id for service #740

Closed
blalor opened this issue Feb 26, 2015 · 17 comments
Closed

allow specifying check id for service #740

blalor opened this issue Feb 26, 2015 · 17 comments
Labels
type/enhancement Proposed improvement or new feature

Comments

@blalor
Copy link
Contributor

blalor commented Feb 26, 2015

I'd like to be able to specify the id for a check associated with a service. The notes field is useful for human-readable text about the check, but it would be useful to have a more structured id to use for alerting. For example, I have two checks for mysql: one for aliveness and another for replication on slaves. The generated id for the replication check is service:mysql:2 but I'd like to have service:mysql:replication.

Check IDs for a service are of the form service:<service_id>[:<index>] and the id property is ignored when reading the service definition from a file. By specifying the check id it would be possible to have service:<service_id>:<check_id>, with a fallback to the other format.

@blalor
Copy link
Contributor Author

blalor commented Feb 26, 2015

It looks like this can be accomplished by providing the checks separately, giving them their own id, and providing service_id.

@pepov
Copy link

pepov commented Mar 3, 2015

👍 for supporting the id field in service check configs

@armon
Copy link
Member

armon commented Mar 4, 2015

@blalor So I am a bit confused, I think this is already possible right?

@blalor
Copy link
Contributor Author

blalor commented Mar 4, 2015

It's possible when defining a standalone check. It is not possible to define the check id for a check embedded in a service definition.

@armon
Copy link
Member

armon commented Mar 4, 2015

Ohh I see. Okay, tagging as enhancement.

@armon armon added the type/enhancement Proposed improvement or new feature label Mar 4, 2015
@pepov
Copy link

pepov commented Mar 4, 2015

Id and name to be precise. Mrantime a pointer to this in the documentation would be great.

@cruatta
Copy link
Contributor

cruatta commented Mar 10, 2015

👍 This would be incredibly useful.

@cruatta
Copy link
Contributor

cruatta commented Mar 31, 2015

I was looking into this and the issue is the way ServiceDefinition is defined.

type ServiceDefinition struct {
    ID      string
    Name    string
    Tags    []string
    Address string
    Port    int
    Check   CheckType
    Checks  CheckTypes
}
type CheckType struct {
    Script   string
    HTTP     string
    Interval time.Duration

    Timeout time.Duration
    TTL     time.Duration

    Notes string
}

ServiceDefinition takes a slice of CheckTypes rather than full blown HealthDefinitions so you don't get fields like ID and Name. You could add ID and/or Name to CheckType and I think that would solve this but then you might as well just have a slice of HealthDefinitions inside of ServiceDefinition. The way I am currently avoiding this problem is just defining a Service in one file and then in a separate file defining a list of checks that are all associated with that one service which makes config management easy. It's not as elegant as what @blalor suggested but it works.

@aconrad
Copy link

aconrad commented Jul 11, 2015

Same here, I have two health checks for a given service, but one of the Consul API consumers is only interested in one of the two, and there is not easy way to know which is which -- currently it's trying to parse the output of the first, and if parsing fails, it tries the output of the other one.

It would be nice to have something more structured that would be exposed via the API (for instance /v1/health/checks/<service>).

@ryanuber
Copy link
Member

@aconrad the endpoint /v1/health/checks/<service> does currently exist. Does it not function how you would expect?

I looked at this a while back and found exactly the same as @cruatta did - the changes actually go pretty deep to support passing in the CheckID, which is unfortunate. I would probably just use a file with separate definitions like some have hinted at above, all in a single file for each service, like this:

{
    "service": {
        "name": "myservice"
    },
    "checks": [
        {
            "id": "check1",
            "service_id": "myservice",
            "ttl": "30s"
        },
        {
            "id": "check2",
            "service_id": "myservice",
            "ttl": "30s"
        }
    ]
}

This is going to be a somewhat low-priority enhancement since we have a reasonable work-around, but we definitely want to get it fixed for the embedded definitions.

@aconrad
Copy link

aconrad commented Jul 13, 2015

@ryanuber the endpoint works (and I'm using it) but I have no way to know which check item in the list of checks is the one I care about since I can't identify them by name -- so as I explained, I try to detect the check I care about by parsing its output.

@ryanuber
Copy link
Member

@aconrad using a configuration like the sample above, you should be able to pass in the check ID to your liking. Then you can refer to the health check much more easily from your own app my just checking the ID field in responses. Does that make sense, or am I missing something?

@aconrad
Copy link

aconrad commented Jul 14, 2015

@ryanuber ah got it. The person that manages our cluster applied the work-around, works great. Thanks!

@n2taylor
Copy link

n2taylor commented Jul 6, 2016

Encountering this issue as well, and the workaround doesn't work for me. No matter how I structure the config file, I always get the generated service name and id when I access health check status via v1/agent/checks or the v1/health/checks endpoints

@ryanuber
Copy link
Member

@n2taylor can you paste your service + check config? This should work perfectly with the example config above. You can verify by saving the example config as config.json and running:

$ consul agent -dev -config=config.json
$ curl localhost:8500/v1/health/checks/myservice?pretty

@n2taylor
Copy link

Never mind, I figured it out... I was missing the service_id attribute on the check itself. Thanks!

@slackpad
Copy link
Contributor

This was fixed in #3051 while closing out #3047.

duckhan pushed a commit to duckhan/consul that referenced this issue Oct 24, 2021
To enable a client agent outside of the k8s cluster to join the
datacenter, you would need to enable server.exposeGossipAndRPCPorts,
client.exposeGossipPorts, and set server.ports.serflan.port to a port
not being used on the host. Since client.exposeGossipPorts uses the
hostPort 8301, server.ports.serflan.port must be set to something other
than 8301.

The client agent VM outside of the k8s cluster would need to be able to
route to the private IP of the VMs in the k8s cluster to join the
datacenter and the VMs in the k8s cluster would need to be able to route
to the client agent VM outside the k8s cluster as well on its advertised
IP.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Proposed improvement or new feature
Projects
None yet
Development

No branches or pull requests

9 participants