feat(k8s): Add lifecycle feat in task schedule#420
feat(k8s): Add lifecycle feat in task schedule#420Spground wants to merge 1 commit intoalibaba:mainfrom
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
e2414f9 to
3835fbf
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3835fbfdc7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if task.Process != nil && task.Process.Lifecycle != nil && task.Process.Lifecycle.PostStop != nil { | ||
| klog.InfoS("Executing postStop lifecycle hook", "task", task.Name) | ||
| if err := e.execLifecycleHook(ctx, task, task.Process.Lifecycle.PostStop); err != nil { |
There was a problem hiding this comment.
Trigger postStop for tasks that already reached terminal state
postStop is only executed inside Executor.Stop, but the reconcile loop finalizes deletion of terminal tasks without calling Stop (see reconcileTasks), so any task that exits naturally (or is deleted after it already succeeded/failed) skips the hook entirely. This breaks the advertised lifecycle contract (PostStop is executed after the main process stops) and will miss cleanup for common short-lived tasks that finish before deletion.
Useful? React with 👍 / 👎.
| env: | ||
| - name: TASK_NAME | ||
| value: lifecycle-task | ||
| lifecycle: |
There was a problem hiding this comment.
Place lifecycle sample under process spec
In this sample manifest, lifecycle is defined at taskTemplate.spec.lifecycle instead of taskTemplate.spec.process.lifecycle, so the hooks are ignored by the controller conversion path that only reads process.lifecycle. Users applying this example won’t actually run preStart/postStop, which makes the feature appear broken and gives incorrect guidance.
Useful? React with 👍 / 👎.
Summary
Add task
lifecyclein k8s runtime, commen use cases: use preStart to prepare some resources e.g dir, data and etc, and clean up those resources via postStop after task terminated.Testing
Breaking Changes
Checklist