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

Adding node on start, and not on hook #1008

Merged
merged 1 commit into from
Feb 19, 2016
Merged

Adding node on start, and not on hook #1008

merged 1 commit into from
Feb 19, 2016

Conversation

subnetmarco
Copy link
Member

This change adds the node on start, and not on hook. It provides a better way of dealing with the nodes table, and also allows nginx to be the last service to start up.

Also tries to address issue #1004.

@subnetmarco subnetmarco self-assigned this Feb 19, 2016
require "kong.cli.services.nginx",
require "kong.cli.services.serf"
require "kong.cli.services.serf",
require "kong.cli.services.nginx"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you will like this @mars - although terminating the foreground process will not terminate all the other services (like < v0.6.x was erroneously doing). As opposed to #1007 which terminates the other process when the main one is terminated (but not really an issue if terminating means shutting down the whole Docker container).

Basically this would allow the older behavior to work again. Still, not a replacement of supervisor (or similar solutions).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would allow the older behavior to work again.

The old behavior being "run kong w/ Nginx daemon off" to get stderr/stdout streams?

It's an improvement indeed, as log spelunking on Docker containers makes ops difficult. Stream all the logs!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Although, like the old behavior, if you terminate nginx it will not terminate the other services, but it doesn't matter if terminating nginx means shutting down the entire container.

@subnetmarco subnetmarco added the pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) label Feb 19, 2016
subnetmarco added a commit that referenced this pull request Feb 19, 2016
Adding node on start, and not on hook
@subnetmarco subnetmarco merged commit 7095754 into release/0.7.0 Feb 19, 2016
@subnetmarco subnetmarco deleted the fix/nodes branch February 19, 2016 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants