-
Notifications
You must be signed in to change notification settings - Fork 197
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
Add framework for managing startup dependencies and shutdown #153
Conversation
Adds a "Service interface" that when implemented by embedded component services allows generic management of service startup (incl. waiting on dependencies to become ready), readiness, and cancellation. Adds such a generic service manager that works based on expressing service dependencies as directed acyclic graph. Implements signaling readiness to systemd and cancellation of embedded services after interrupts from the system. Refactors the first two services (etcd, kube-apiserver) to illustrate the pattern, using a GenericService implementation to call the legacy code for bringing up non-ported services. Signed-off-by: Frank A. Zdarsky <fzdarsky@redhat.com>
The PR starts all the embedded services, but some of them haven't been refactored yet and don't implement cancellation (they just execute forever until MicroShift's process terminates). As a result, the behavior (incl. when receiving interrupts) should still be the same. To get a feeling for the UX after refactoring, comment out L80-91 and L95-110 of pkg/cmd/run.go and run. You may need to wipe |
Signed-off-by: Frank A. Zdarsky <fzdarsky@redhat.com>
} | ||
} | ||
|
||
m.services = append(m.services, s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need a mutex here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review, @rootfs. You mean for hedging against AddService()
being called concurrently?
Note that at least currently, the order of adding services is significant (to enforce the invariant of dependencies being added before dependents), so AddService()
shouldn't be called concurrently. Otherwise, we'd have to enable topo sorting. See also the comments in AddService()
and Run()
.
How should we proceed? Add mutex proactively or add note in method description?
this looks good, please finish it up and also run some benchmark to see if the new way accelerates the startup as well. |
As long as the services other than etcd and kube-apiserver (who have a startup dependency already) haven't been ported, I don't expect to see much diff (other than seeing a few less error messages in the log). |
let's merge this one first and please go ahead with another PR for the rest of the services. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rootfs The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Adds a "Service interface" that when implemented by embedded component services allows generic management of service startup (incl. waiting on dependencies to become ready), readiness, and cancellation.
Adds such a generic service manager that works based on expressing service dependencies as directed acyclic graph.
Implements signaling readiness to systemd and cancellation of embedded services after interrupts from the system.
Refactors the first two services (etcd, kube-apiserver) to illustrate the pattern, using a GenericService implementation to call the legacy code for bringing up non-ported services.
Signed-off-by: Frank A. Zdarsky fzdarsky@redhat.com
Closes #152