Skip to content

feat: add eksjob plugin to run jobs as Kubernetes custom resources on EKS - #156

Closed
Shashank Mittal (shashank-iitbhu) wants to merge 5 commits into
mainfrom
feat/eksjob-plugin
Closed

Shashank Mittal (shashank-iitbhu) wants to merge 5 commits into
mainfrom
feat/eksjob-plugin

Conversation

@shashank-iitbhu

@shashank-iitbhu Shashank Mittal (shashank-iitbhu) commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Closing this PR: the plugin will be developed and validated elsewhere first, and may come back as a new PR once it is mature.

Add a small package that assumes an IAM role (auto-refreshing), describes
an EKS cluster and builds a client-go rest.Config authenticated with the
presigned STS token aws-iam-authenticator uses, refreshed before expiry.

The sparkeks health check now uses it for AssumeRole + DescribeCluster
instead of its inline copy; its behaviour is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… EKS

The plugin is CRD-agnostic: the command context names the custom resource,
its spec defaults, size profiles, cancel field and pod selector label; the
job context supplies size, labels and spec.

- creates heimdall-<job id> and attaches to it if it already exists
- polls status.phase until Succeeded/Failed; failureReason surfaces in the
  job error, with recent Warning events on stderr
- streams the run's pod logs (late pods, restarts, resumed streams)
- cancels by merge-patching spec.<cancel_field>; never deletes the resource
- health check: assume role, DescribeCluster (ACTIVE), list the resource
- New() only checks the command context's shape so a bad value fails a
  job rather than Heimdall startup

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 22, 2026 11:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@argus-pattern argus-pattern Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Argus review of 02b6696: 5 inline findings. The most important are that job spec can override any spec_defaults key, including serviceAccountName and image, and that a run is left orphaned when polling fails.

Not tied to a line: the PR's test plan says end-to-end against a real EKS cluster and CRD hasn't been run yet. The token auth path (presigned STS with X-Amz-Expires, refresh, and invalidation on 401), the dedupe on log-stream resume, and the cancel contract are only exercised against fakes. Please validate them on a real cluster before data-airflow switches its backend flag. The sparkeks health-check refactor looks behaviour-preserving.

When waitForTerminal fails without the context being canceled (ten
consecutive Get errors, for example), the Heimdall job fails but the
custom resource keeps running, and the janitor never cleans up failed
jobs. An Airflow retry then starts a second run alongside it.

Make the same best-effort cancel patch as the canceled-context path,
through a shared cancelUnfinished helper, and close the log streams
instead of draining them. A run that was deleted is not patched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every line whose timestamp was not after the last one was dropped, so a
line sharing its predecessor's timestamp, or any line after the node
clock stepped back, was silently lost.

Only a resumed stream needs deduplicating, because SinceTime has second
granularity and the stream repeats lines up to the resume point. Record
that point when the stream starts and drop lines up to it; the first
newer line ends the replay.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The plugin calls create/get/list/patch on the custom resource, list on
pods, get on pods/log and list on events. It never watches, updates or
reads the status subresource, so drop those from the table and say what
each verb is for. Also note that init containers are not streamed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants