-
Notifications
You must be signed in to change notification settings - Fork 106
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
fix: Making the daemons keep up the status. #2617
Changes from all commits
fd1acd3
53e74e2
55d9e6d
79f1d14
de38992
b5b2ca2
6e62465
149b204
97edc0d
d35e318
219bf69
1ff61e9
20e5045
cea24e9
3c51be7
69a5186
1227999
2bcc53c
780fe2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,3 @@ | ||
import { types as T } from "@start9labs/start-sdk" | ||
|
||
export type Effects = T.Effects & { | ||
setMainStatus(o: { status: "running" | "stopped" }): Promise<void> | ||
} | ||
export type Effects = T.Effects |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -581,10 +581,28 @@ struct GetHostInfoParams { | |
callback: Callback, | ||
} | ||
async fn get_host_info( | ||
_: EffectContext, | ||
ctx: EffectContext, | ||
GetHostInfoParams { .. }: GetHostInfoParams, | ||
) -> Result<Value, Error> { | ||
todo!() | ||
let ctx = ctx.deref()?; | ||
Ok(json!({ | ||
"id": "fakeId1", | ||
"kind": "multi", | ||
"hostnames": [{ | ||
"kind": "ip", | ||
"networkInterfaceId": "fakeNetworkInterfaceId1", | ||
"public": true, | ||
"hostname":{ | ||
"kind": "domain", | ||
"domain": format!("{}", ctx.id), | ||
"subdomain": (), | ||
"port": (), | ||
"sslPort": () | ||
} | ||
} | ||
|
||
] | ||
})) | ||
} | ||
|
||
async fn clear_bindings(context: EffectContext, _: Empty) -> Result<Value, Error> { | ||
|
@@ -1011,21 +1029,23 @@ async fn set_configured(context: EffectContext, params: SetConfigured) -> Result | |
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize, TS)] | ||
#[serde(rename_all = "camelCase")] | ||
#[ts(export)] | ||
enum Status { | ||
enum SetMainStatusStatus { | ||
Running, | ||
Stopped, | ||
Starting, | ||
} | ||
impl FromStr for Status { | ||
impl FromStr for SetMainStatusStatus { | ||
type Err = color_eyre::eyre::Report; | ||
fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
match s { | ||
"running" => Ok(Self::Running), | ||
"stopped" => Ok(Self::Stopped), | ||
"starting" => Ok(Self::Starting), | ||
_ => Err(eyre!("unknown status {s}")), | ||
} | ||
} | ||
} | ||
impl ValueParserFactory for Status { | ||
impl ValueParserFactory for SetMainStatusStatus { | ||
type Parser = FromStrParser<Self>; | ||
fn value_parser() -> Self::Parser { | ||
FromStrParser::new() | ||
|
@@ -1037,14 +1057,15 @@ impl ValueParserFactory for Status { | |
#[command(rename_all = "camelCase")] | ||
#[ts(export)] | ||
struct SetMainStatus { | ||
status: Status, | ||
status: SetMainStatusStatus, | ||
} | ||
async fn set_main_status(context: EffectContext, params: SetMainStatus) -> Result<Value, Error> { | ||
dbg!(format!("Status for main will be is {params:?}")); | ||
let context = context.deref()?; | ||
match params.status { | ||
Status::Running => context.started(), | ||
Status::Stopped => context.stopped(), | ||
SetMainStatusStatus::Running => context.started(), | ||
SetMainStatusStatus::Stopped => context.stopped(), | ||
SetMainStatusStatus::Starting => context.stopped(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why add a varient if just treating as stopped? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To indicate a state, that it is trying to getting things started. Could be more usefull or not, but for now just not doing anything with this information until I talk with you. |
||
} | ||
Ok(Value::Null) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
import { NO_TIMEOUT, SIGTERM } from "../StartSdk" | ||
import { SDKManifest } from "../manifest/ManifestTypes" | ||
import { Effects, ValidIfNoStupidEscape } from "../types" | ||
import { MountOptions, Overlay } from "../util/Overlay" | ||
import { splitCommand } from "../util/splitCommand" | ||
import { cpExecFile } from "./Daemons" | ||
|
||
export class CommandController { | ||
private constructor( | ||
readonly runningAnswer: Promise<unknown>, | ||
readonly overlay: Overlay, | ||
readonly pid: number | undefined, | ||
) {} | ||
static of<Manifest extends SDKManifest>() { | ||
return async <A extends string>( | ||
effects: Effects, | ||
imageId: { | ||
id: Manifest["images"][number] | ||
sharedRun?: boolean | ||
}, | ||
command: ValidIfNoStupidEscape<A> | [string, ...string[]], | ||
options: { | ||
mounts?: { path: string; options: MountOptions }[] | ||
overlay?: Overlay | ||
env?: | ||
| { | ||
[variable: string]: string | ||
} | ||
| undefined | ||
cwd?: string | undefined | ||
user?: string | undefined | ||
onStdout?: (x: Buffer) => void | ||
onStderr?: (x: Buffer) => void | ||
}, | ||
) => { | ||
const commands = splitCommand(command) | ||
const overlay = options.overlay || (await Overlay.of(effects, imageId)) | ||
for (let mount of options.mounts || []) { | ||
await overlay.mount(mount.options, mount.path) | ||
} | ||
const childProcess = await overlay.spawn(commands, { | ||
env: options.env, | ||
}) | ||
const answer = new Promise<null>((resolve, reject) => { | ||
childProcess.stdout.on( | ||
"data", | ||
options.onStdout ?? | ||
((data: any) => { | ||
console.log(data.toString()) | ||
}), | ||
) | ||
childProcess.stderr.on( | ||
"data", | ||
options.onStderr ?? | ||
((data: any) => { | ||
console.error(data.toString()) | ||
}), | ||
) | ||
|
||
childProcess.on("exit", (code: any) => { | ||
if (code === 0) { | ||
return resolve(null) | ||
} | ||
return reject(new Error(`${commands[0]} exited with code ${code}`)) | ||
}) | ||
}) | ||
|
||
const pid = childProcess.pid | ||
|
||
return new CommandController(answer, overlay, pid) | ||
} | ||
} | ||
async wait() { | ||
try { | ||
return await this.runningAnswer | ||
} finally { | ||
await cpExecFile("pkill", ["-9", "-s", String(this.pid)]).catch((_) => {}) | ||
await this.overlay.destroy().catch((_) => {}) | ||
} | ||
} | ||
async term({ signal = SIGTERM, timeout = NO_TIMEOUT } = {}) { | ||
try { | ||
await cpExecFile("pkill", [ | ||
`-${signal.replace("SIG", "")}`, | ||
"-s", | ||
String(this.pid), | ||
]) | ||
|
||
if (timeout > NO_TIMEOUT) { | ||
const didTimeout = await Promise.race([ | ||
new Promise((resolve) => setTimeout(resolve, timeout)).then( | ||
() => true, | ||
), | ||
this.runningAnswer.then(() => false), | ||
]) | ||
if (didTimeout) { | ||
await cpExecFile("pkill", [`-9`, "-s", String(this.pid)]).catch( | ||
(_: any) => {}, | ||
) | ||
} | ||
} else { | ||
await this.runningAnswer | ||
} | ||
} finally { | ||
await this.overlay.destroy() | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
import { SDKManifest } from "../manifest/ManifestTypes" | ||
import { Effects, ValidIfNoStupidEscape } from "../types" | ||
import { MountOptions, Overlay } from "../util/Overlay" | ||
import { CommandController } from "./CommandController" | ||
|
||
const TIMEOUT_INCREMENT_MS = 1000 | ||
const MAX_TIMEOUT_MS = 30000 | ||
/** | ||
* This is a wrapper around CommandController that has a state of off, where the command shouldn't be running | ||
* and the others state of running, where it will keep a living running command | ||
*/ | ||
|
||
export class Daemon { | ||
private commandController: CommandController | null = null | ||
private shouldBeRunning = false | ||
private constructor(private startCommand: () => Promise<CommandController>) {} | ||
static of<Manifest extends SDKManifest>() { | ||
return async <A extends string>( | ||
effects: Effects, | ||
imageId: { | ||
id: Manifest["images"][number] | ||
sharedRun?: boolean | ||
}, | ||
command: ValidIfNoStupidEscape<A> | [string, ...string[]], | ||
options: { | ||
mounts?: { path: string; options: MountOptions }[] | ||
overlay?: Overlay | ||
env?: | ||
| { | ||
[variable: string]: string | ||
} | ||
| undefined | ||
cwd?: string | undefined | ||
user?: string | undefined | ||
onStdout?: (x: Buffer) => void | ||
onStderr?: (x: Buffer) => void | ||
}, | ||
) => { | ||
const startCommand = () => | ||
CommandController.of<Manifest>()(effects, imageId, command, options) | ||
return new Daemon(startCommand) | ||
} | ||
} | ||
|
||
async start() { | ||
if (this.commandController) { | ||
return | ||
} | ||
this.shouldBeRunning = true | ||
let timeoutCounter = 0 | ||
new Promise(async () => { | ||
while (this.shouldBeRunning) { | ||
this.commandController = await this.startCommand() | ||
await this.commandController.wait().catch((err) => console.error(err)) | ||
await new Promise((resolve) => setTimeout(resolve, timeoutCounter)) | ||
timeoutCounter += TIMEOUT_INCREMENT_MS | ||
timeoutCounter = Math.max(MAX_TIMEOUT_MS, timeoutCounter) | ||
} | ||
}).catch((err) => { | ||
console.error(err) | ||
}) | ||
} | ||
async term(termOptions?: { | ||
signal?: NodeJS.Signals | undefined | ||
timeout?: number | undefined | ||
}) { | ||
return this.stop(termOptions) | ||
} | ||
async stop(termOptions?: { | ||
signal?: NodeJS.Signals | undefined | ||
timeout?: number | undefined | ||
}) { | ||
this.shouldBeRunning = false | ||
await this.commandController | ||
?.term(termOptions) | ||
.catch((e) => console.error(e)) | ||
this.commandController = null | ||
} | ||
} |
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.
Why this change
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.
So, the reason is that a daemon is something that we start and run. It will keep trying, to run in the background.
A command is something that runs once and is done. This is doing things with a normal command and tries the cleanup that was with the old command.
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.
ok, as long as this
runCommand
isn't collecting output, we don't want to be leaking memory buffering logs