search

airbnb / nerve

A service registration daemon that performs health checks; companion to airbnb/synapse
MIT License
942 stars 151 forks source link

Remove port requirement from nerve #87

Closed yagnik closed 8 years ago

yagnik commented 8 years ago

Problem: Nerve currently requires all services to define a port and fails hard if port is not set. Some "services" (lack of better word) are batch jobs which don't require a port but we'd like to register them through nerve as we would like to build auto deploy and fail over using our service discovery layer.

Solution: Remove the hard dependancy on port in service watcher.

Debate: Should nerve really be responsible for registering non talkative (one which do not latch onto a port) services? At snapdeal we list all our services in yaml files and use https://github.com/yagnik/dendrite to build nerve/synapse config. We'd like to also list any batch jobs in there too.

@igor47 @jolynch

jolynch commented 8 years ago

I don't see any issues with removing this requirement other than typically, people want a port so validating it's there is useful for catching configuration errors.

We work around this by registering some bogus port and having synapse just ignore it (we use synapse file output for those services/batch services).

edit: there might be an issue with check logic assuming there is a port. hrm I wonder how we make them pass healthchecks ... (I'll check and get back to you)

yagnik commented 8 years ago

That's what I did here but as I wrote it I found the idea not as elegant as I like https://github.com/yagnik/dendrite/pull/2

If you think port should still stay here then feel free to close the pr, I'll merge the changes in dendrite

On Monday, 23 May 2016, Joseph Lynch notifications@github.com wrote:

I don't see any issues with removing this requirement other than typically, people want a port so validating it's there is useful for catching configuration errors.

We work around this by registering some bogus port and having synapse just ignore it (we use synapse file output for those services/batch services).

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/airbnb/nerve/pull/87#issuecomment-221008562

yagnik commented 8 years ago

For now will update it on my side, might revisit this.