-
Notifications
You must be signed in to change notification settings - Fork 19
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
Streamline deployment process further #229
Conversation
Is it a bad idea to have the keysystem run the startup script upon admission, or make admission part of the startup process? Using a timer for this seems a bit kludgy. |
Admission is part of the startup process. It is possible for the keyclient to run a script once it's done, but I'd worry that it couples the components too tightly, and has weird edge case problems with things like the autostart script being run as a child process of the keyclient, and potentially causing problems if it waits too long or exits. It's not a bad idea; I'm not convinced it's unambiguously better than this model. I also want to avoid adding more features to the already-rather-complicated keysystem. |
What about using systemd notify to notify systemd when the keyclient is ready to process requests? The other services can then be made to depend on it. I was concerned that using a timer doesn't make it explicit that the other services depend on this admission process being completed, and could easily lead to timing issues if overused. |
The keyclient doesn't process requests. It performs a set of operations, like generating keys, getting them signed by the keyserver, and downloading files from the keyserver. You're right that making the keyclient notify systemd is probably the cleanest way to do this; I didn't really want to go through the effort of making that work, especially because it complicates the keyclient more, but I suppose that I probably should. |
78f3e4c
to
dfa2f9d
Compare
I've made a bunch of changes and updated the description of this PR. Ready for review, though I'll be doing a bit more testing before I merge it. |
dfa2f9d
to
bbae1e6
Compare
#with tempfile.TemporaryDirectory() as d: | ||
# specfile = os.path.join(d, "spec.yaml") | ||
# util.writefile(specfile, configuration.get_single_kube_spec(spec_name).encode()) | ||
# access.call_kubectl(["apply", "-f", specfile], return_result=False) |
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.
commented-out code
The four main ideas here:
This almost finishes #172, except for #188. With this PR, I deployed a cluster in under 13 minutes, and it should (in theory) now be possible to add additional members to the cluster without reconfiguring all of the old nodes.