Skip to content

fix(aws-batch): job lifecycle, scheduling policies, job definition type check and job state-change events - #1648

Merged
Satyam-Trivedi-ZS merged 4 commits into
stackshy:developmentfrom
Satyam-Trivedi-ZS:fix/aws-batch-parity-1496
Oct 23, 2026
Merged

Satyam-Trivedi-ZS merged 4 commits into
stackshy:developmentfrom
Satyam-Trivedi-ZS:fix/aws-batch-parity-1496

Conversation

@Satyam-Trivedi-ZS

@Satyam-Trivedi-ZS Satyam-Trivedi-ZS commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Part of #1496. Closes all four AWS Batch items:

  • Adds the job lifecycle: SubmitJob, DescribeJobs, ListJobs, CancelJob and TerminateJob, with jobs moving SUBMITTED → (PENDING) → RUNNABLE → STARTING → RUNNING → SUCCEEDED/FAILED on the provider clock.
  • Adds scheduling-policy CRUD and validates schedulingPolicyArn on CreateJobQueue/UpdateJobQueue.
  • Validates the RegisterJobDefinition type enum.
  • Publishes "Batch Job State Change" events to the EventBridge default bus from the shared serve scheduler.

Issue items

Item Status Evidence
(High) Job lifecycle Fixed providers/aws/batch/jobs.go, job_lifecycle.go. Gap confirmed on the base: services/batch/driver/driver.go had no job operations and server/aws/batch/handler.go did not route /v1/submitjob (it was not in knownOps).
(Medium) SchedulingPolicy CRUD, schedulingPolicyArn free text Fixed providers/aws/batch/scheduling_policy.go, job_queue.go. On the base, CreateJobQueue stored in.SchedulingPolicyARN verbatim and no scheduling-policy operations existed.
(Medium) RegisterJobDefinition type enum Fixed providers/aws/batch/job_definition.go (validateJobDefinitionType). On the base only type == "" was rejected, so type=batch registered. The pinned SDK (service/batch v1.74.0, types/enums.go) models only container and multinode.
(Low) Job/JobQueue/ComputeEnvironment state-change events Fixed for jobs; not a real gap for queues and compute environments Reuses internal/awsevents (the same seam EC2/ECS/SFN/Glue/SSM/SageMaker use), wired in providers/aws/aws.go wireLifecycleEvents. AWS documents only "Batch Job State Change" (plus "Batch Job Queue Blocked", which cannot happen here because jobs never wait for capacity). It documents no compute-environment or job-queue state-change event, so none is emitted.

What real AWS does (sources)

  • SubmitJob: jobName up to 128 characters, first alphanumeric, then letters/numbers/-/_; jobQueue by name or ARN; jobDefinition as name, name:revision or ARN with or without the revision, where no revision means the latest ACTIVE one; array size 2 to 10,000; at most 20 dependencies; SEQUENTIAL without a job ID and N_TO_N for array jobs; shareIdentifier required on a fair-share queue and not allowed on a FIFO queue; schedulingPriorityOverride 0 to 9999; timeout at least 60 seconds; at most 50 tags.
  • CreateJobQueue state: a DISABLED queue accepts no new jobs. Its schedulingPolicyArn text says a fair-share policy "can be replaced but can't be removed", and a FIFO queue "can't have a fair-share scheduling policy added".
  • Job states: SUBMITTED → PENDING (only while dependencies are unfinished) → RUNNABLE → STARTING → RUNNING → SUCCEEDED/FAILED. An array parent is PENDING while its children run.
  • CancelJob: cancels SUBMITTED, PENDING or RUNNABLE jobs (to FAILED). STARTING or RUNNING jobs are not cancelled, but the call still succeeds. reason is required, at most 1024 characters. TerminateJob: STARTING or RUNNING jobs go to FAILED, and jobs that haven't reached STARTING are cancelled.
  • DescribeJobs: up to 100 job IDs.
  • ListJobs: exactly one of jobQueue, arrayJobId or multiNodeJobId. jobStatus defaults to RUNNING and is ignored when a filter is used, except SHARE_IDENTIFIER. Only one filter at a time. JOB_NAME is case-insensitive with a trailing * prefix match. JOB_DEFINITION takes a name (any revision), a prefix*, or an ARN (exact revision). BEFORE/AFTER_CREATED_AT take epoch milliseconds. Results are newest first. maxResults is at least 1, with up to 1000 per page by status and 100 with a filter.
  • CreateSchedulingPolicy, DescribeSchedulingPolicies (arns required, up to 100), ListSchedulingPolicies (maxResults 1 to 100), UpdateSchedulingPolicy, DeleteSchedulingPolicy ("You can't delete a scheduling policy that's used in any job queues"). FairsharePolicy: computeReservation 0 to 99, shareDecaySeconds up to 604800. ShareAttributes: shareIdentifier is required and up to 255 characters plus an optional *; weightFactor is 0.0001 to 999.9999. Only one of fairsharePolicy and quotaSharePolicy can be set, and the kind can't be changed.
  • RegisterJobDefinition type: container | multinode. container requires containerProperties, ecsProperties or eksProperties; multinode requires nodeProperties.
  • TagResource: jobs and scheduling policies are taggable, and array or MNP child job ARNs aren't. At most 50 tags, keys 1 to 128 characters, values up to 256. The aws: prefix is reserved (tag naming rules).
  • Job state change events: source aws.batch, detail-type Batch Job State Change, resources [jobArn], and a detail shaped like the DescribeJobs JobDetail. "Events aren't created for the initial job submission."
  • Every operation's documented errors are ClientException (400) and ServerException (500). All new client errors map to ClientException.

Changes

  • Driver (services/batch/driver): 10 new operations (scheduling policies and jobs) with their types.
  • Provider (providers/aws/batch):
    • Job lifecycle (job_lifecycle.go):
      • A job stores its submission time and a phase length. Its status, statusReason, startedAt and stoppedAt are derived on every read from the provider clock, the way internal/settle works. Nothing runs on goroutines or timers.
      • With --async-settle (config.WithAsyncSettle) SUBMITTED, RUNNABLE and STARTING last 1s each (settle.DefaultBatchJobPhase) and RUNNING lasts 5s (settle.DefaultBatchJobRunDwell), so a real client can terminate a running job. Without it a job is SUCCEEDED as soon as its dependencies are, which is the same default as every other settling resource.
      • dependsOn holds a job in PENDING until its dependencies finish, and fails it with Dependent Job failed if one of them failed.
      • Array jobs have derived children (<id>:<index>). SEQUENTIAL runs the children one at a time, N_TO_N pairs child indexes, and the parent reports PENDING, then SUCCEEDED or FAILED, with statusSummary.
      • SUCCEEDED jobs report Essential container in task exited and container exitCode: 0. containerOverrides are merged into the definition's containerProperties: environment by name, resourceRequirements by type, and every other field replaced. ecsPropertiesOverride and eksPropertiesOverride are merged per container (matched by name), and nodeOverrides replace numNodes and merge into the node ranges their targetNodes cover. An override the job definition can't take is a ClientException.
    • Cancel and terminate (jobs.go):
      • CancelJob and TerminateJob decide against the job's state at call time. Cancel affects only SUBMITTED, PENDING and RUNNABLE jobs.
      • Terminate affects any unfinished job, and before STARTING it counts as a cancel (isCancelled). A terminated running job keeps its startedAt.
      • On an array parent, both apply to every child they affect.
    • SubmitJob checks that the queue exists and is ENABLED, and that the job definition revision exists and is ACTIVE (a bare name resolves to the latest ACTIVE revision). It also checks dependencies, shareIdentifier, the array size, priority, timeout and tags.
    • ListJobs takes all the documented filters, for queue listings and array-child listings alike. The page token is a cursor on the submission sequence (or the child index), not an offset, so jobs submitted or changing status between pages never make a page skip or repeat an entry.
    • Scheduling policies (scheduling_policy.go): full CRUD with the FairsharePolicy and ShareAttributes bounds. A queue must reference an existing policy, a policy can be replaced but not removed, a FIFO queue can't gain one, and an in-use policy can't be deleted. Binding a policy to a queue and deleting a policy are serialized, so a queue can't end up bound to a deleted policy.
    • RegisterJobDefinition enforces the container/multinode enum and the properties each type requires. platformCapabilities is stored as sent; no EC2 default is echoed (that made Terraform multinode definitions drift).
    • Tags (tags.go): jobs and scheduling policies are taggable. All create calls and TagResource now enforce the 50/128/256 limits and the reserved aws: prefix. A TagResource that would push the merged set past 50 is rejected without changing anything.
    • Events (events.go):
      • "Batch Job State Change" is published once per status a job passes through, never for the submission itself. A cancelled or failed job publishes FAILED.
      • Publishing runs on the shared serve scheduler: the Batch mock implements config.Tickable and is registered in Provider.Tickables(). A job nobody polls still reports each transition. Batch job API calls also publish, so library mode without a ticker still sees events.
      • Emission happens outside jobsMu, so an event target that calls back into Batch can't deadlock (covered by TestConcurrentJobOperations under -race).
    • Snapshots: scheduling policies, job records and the job sequence are in the snapshot. A restored job resumes its lifecycle.
  • Wire (server/aws/batch): REST-JSON routes for the 10 operations, translation only.
  • Docs: docs/coverage/nongoals/batch.md lists the limits below, and go generate ./... output is committed.
  • Auth: Batch is authorized at service level under --enforce-auth (the IAMService() == "batch" path, as for the other REST services without per-operation checks), so the new operations are covered without extra wiring. Per-operation IAM for Batch would be a separate change, as it was for Lambda in feat(aws-lambda): op-level IAM authorization under --enforce-auth (AUTHZ-X1f) #1530.

Provider Coverage

  • AWS
  • Azure (n/a: AWS Batch-specific operations)
  • GCP (n/a)
  • OCI (n/a)

Checklist

  • All tests pass (go test -p 2 ./...)
  • Linter: no new findings in touched packages (golangci-lint run --new-from-rev=upstream/development)
  • Every provider the change applies to implements the same behavior (AWS-only service)
  • Integration tests added to cloudemu_test.go (no AWS Batch section there; covered by provider and SDK wire tests)
  • Unit tests added to provider test files
  • Regenerated docs (go generate ./...) and committed the result

Test Plan

Provider tests (providers/aws/batch, FakeClock plus config.WithAsyncSettle). These don't compile on the base, where the operations don't exist:

  • TestSubmitJobSyncSucceedsImmediately, TestJobLifecycleAdvancesWithClock (SUBMITTED→RUNNABLE→STARTING 1s each, RUNNING 5s, then SUCCEEDED)
  • TestSubmitJobValidation, TestSubmitJobUsesLatestActiveRevision, TestDependentJobWaitsAndFailsWithDependency
  • TestCancelVersusTerminate, TestArrayJobChildren, TestListJobs, TestListJobsCursorIsStable
  • TestJobStateChangeEvents, TestJobEventsFollowAsyncLifecycle, TestTickPublishesUnpolledTransitions
  • TestSchedulingPolicyCRUDAndQueueBinding, TestRegisterJobDefinitionTypeValidation, TestTagValidation
  • TestJobsAndPoliciesSurviveSnapshot, TestConcurrentJobOperations
  • Review follow-up: TestSubmitJobAppliesEcsPropertiesOverride, TestSubmitJobAppliesEksPropertiesOverride, TestSubmitJobAppliesNodeOverrides, TestSubmitJobRejectsUnusableOverrides, TestSubmitJobRejectsInactiveRevision, TestReplayedEventsCarryNoExitCode, TestListArrayChildrenAppliesFilters, TestSchedulingPolicyBindingIsAtomic, TestMultinodeJobDefinitionEchoesNoDefaultPlatform, and SDK TestSDKSubmitJobEcsPropertiesOverride
  • providers/aws TestTickablesRegistersTimeDrivenServices (updated for Batch)

SDK wire tests (server/aws/batch, real aws-sdk-go-v2 service/batch v1.74.0). These fail on the base, which does not serve these operations:

  • TestSDKJobLifecycle, TestSDKArrayJob, TestSDKSchedulingPolicies, TestSDKRegisterJobDefinitionInvalidType
  • TestSDKListJobsPaginatorAndFilters (SDK ListJobsPaginator), TestSDKReservedTagPrefixRejected

Results:

  • go build ./...: ok.
  • go vet on the touched packages: ok.
  • go test -p 2 ./...: all packages pass.
  • go test -race -count=1 ./providers/aws/batch/ ./server/aws/batch/: ok. TestConcurrentJobOperations also passes with -race -count=2.
  • golangci-lint run --concurrency 2 --allow-parallel-runners --new-from-rev=upstream/development on providers/aws/batch, server/aws/batch, services/batch, internal/settle and providers/aws: 0 issues.
  • go generate ./... leaves no diff after the commit.

Real-user run (AWS CLI 1.x with botocore 1.42, against cloudemu serve --providers aws --aws-port 4601 --async-settle)

The setup created an EventBridge rule ({"source":["aws.batch"],"detail-type":["Batch Job State Change"]}) with an SQS target, then a Fargate compute environment, queue q and job definition jd.

Job lifecycle:

  • submit-job --job-name hello --container-overrides '{"command":["echo","bye"]}', then describe-jobs once a second, returned SUBMITTED, RUNNABLE, STARTING, then SUCCEEDED with Essential container in task exited, exitCode 0, command ["echo","bye"], and the queue and job definition as ARNs. RUNNING fell between two polls because of CLI latency.
  • --depends-on jobId=<parent>: the parent was STARTING while the child was PENDING, and both later SUCCEEDED.

Cancel vs terminate:

  • cancel-job on a RUNNABLE job returned FAILED, user cancel, isCancelled: true.
  • cancel-job on a RUNNING job had no effect (exit 0).
  • terminate-job on the same RUNNING job returned FAILED, kill it, isTerminated: true, with startedAt and stoppedAt set.
  • terminate-job on a SUBMITTED job returned FAILED, isCancelled: true.
  • cancel-job --job-id does-not-exist returned ClientException.

Array jobs:

  • --array-properties size=3: the parent was PENDING with statusSummary STARTING: 3.
  • list-jobs --array-job-id <id> --job-status SUCCEEDED listed <id>:0..2 with their indexes.

list-jobs:

  • --job-status SUCCEEDED listed the jobs newest first. --job-status FAILED showed tocancel user cancel.
  • --filters name=JOB_NAME,values=HEL* returned hello. name=JOB_DEFINITION,values=jd --max-items 2 paginated.
  • These returned ClientException: --job-status DONE, two filters at once, and both --job-queue and --array-job-id.

Validation errors (all ClientException, exit 255):

  • submit-job with:
    • a missing queue
    • a missing job definition
    • jd:99
    • --job-name=-x
    • --job-name "has space"
    • --array-properties size=1
    • --depends-on jobId=nope
    • --share-identifier A on a FIFO queue
    • --tags aws:team=x
    • a DISABLED queue
  • register-job-definition with:
    • --type batch
    • --type CONTAINER
    • --type multinode without nodeProperties

Scheduling policies:

  • create-scheduling-policy --name fair --fairshare-policy {...} --tags k=v succeeded.
  • These returned ClientException:
    • creating fair again
    • computeReservation: 100
    • weightFactor: 1000
    • update-scheduling-policy on a missing ARN
    • delete-scheduling-policy on a missing ARN
    • tag-resource --tags aws:x=y
  • describe-scheduling-policies and list-scheduling-policies round-tripped the policy. describe-scheduling-policies with a missing ARN returned [].
  • update-scheduling-policy updated it.
  • create-job-queue --scheduling-policy-arn .../nope returned ClientException. With the real ARN it succeeded.
  • update-job-queue --job-queue q --scheduling-policy-arn ... on the FIFO queue returned ClientException.
  • On the fair-share queue, submit-job without --share-identifier returned ClientException, and with --share-identifier A1 it succeeded.
  • delete-scheduling-policy while the policy was in use returned ClientException.

EventBridge (received through the SQS target):

  • hello: RUNNABLE, STARTING, RUNNING, SUCCEEDED, with no event for the submission.
  • child: PENDING, RUNNABLE, STARTING, RUNNING, SUCCEEDED.
  • tocancel: FAILED.
  • arr: PENDING, SUCCEEDED.
  • Every event had source aws.batch, detail-type Batch Job State Change, and resources [jobArn]. The events for jobs nobody was polling arrived through the scheduler tick.

Persistence: with serve --persist --state-file state.json, I submitted a job tagged t=1 and created policy sp, stopped the server with SIGINT, and restarted it. list-jobs, list-tags-for-resource (t=1) and list-scheduling-policies all returned the same data.

I stopped the server afterwards, and nothing is listening on 4601.

Terraform was not run by me; the reviewer ran hashicorp/aws 6.66.0 against it.

Review follow-up run (same CLI, serve --async-settle on 4601, then plain serve for the event check): ecs-properties-override, eks-properties-override and node-overrides show up in describe-jobs; an ECS override on a containerProperties definition and an unknown container name return ClientException; submit-job --job-definition jd:1 after deregistering it returns ClientException while jd:2 works; polling caught a job in RUNNING and terminate-job gave FAILED with isTerminated: true and startedAt kept; list-jobs --array-job-id X --filters name=JOB_NAME,values=nomatch returns 0 children; a multinode definition without platformCapabilities describes as null; and through EventBridge to SQS without --async-settle the RUNNABLE, STARTING and RUNNING events carry no exitCode while SUCCEEDED carries exitCode: 0.

Known limits and risks

  • No workload runs: jobs always succeed unless they are cancelled, terminated or a dependency fails. attempts is empty, and retryStrategy and timeout are stored and echoed but never applied.
  • CancelJob on a PENDING job takes effect at once. Real Batch waits for the dependencies first.
  • Multi-node parallel jobs run as one job, report no per-node nodeDetails, and ListJobs --multi-node-job-id returns an empty list. Overrides aren't checked against compute-environment platform rules (for example nodeOverrides on Fargate). Array children emit no events of their own.
  • A failed job's event stream has only FAILED, without the earlier steps.
  • Fair-share policies never change the order jobs run in, so "Batch Job Queue Blocked" never fires.
  • Error message texts are written by the emulator. The exception type and status follow the documented ones.
  • TagResource and the create calls now reject an aws: tag key and more than 50 tags on compute environments, job queues and job definitions too. That matches the TagResource reference, but it is stricter than before for those three resources.
  • Without --async-settle a job is SUCCEEDED at submission, the same as other settling resources. Users who want to watch the progression need --async-settle (CLOUDEMU_ASYNC_SETTLE).

Related Issues

Part of #1496

@NitinKumar004 NitinKumar004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This lands the whole Batch job surface: SubmitJob/DescribeJobs/ListJobs/CancelJob/TerminateJob, scheduling-policy CRUD with queue binding, the job-definition type enum, and "Batch Job State Change" events. The design is right. Job state is derived at read time from SubmittedAt + Phase on the provider clock, with no goroutine per job. That is the same model as internal/settle. Events go out through the shared config.Tickable scheduler and are emitted outside jobsMu. Cancel and terminate are decided against the observed state at call time. The ListJobs cursor is keyed on the submission sequence, so it stays stable. Snapshots carry jobs, policies and the job sequence. The wire layer only translates. I found no blocking correctness bug. The points below are fidelity gaps, and one of them is a silent drop worth fixing in this PR.

Verified

  • go build ./... ok. go vet on the touched packages ok. gofmt clean.
  • go test -race -count=1 passes for providers/aws/batch, server/aws/batch and internal/settle. providers/aws (Tickables) and persist also pass.
  • golangci-lint --new-from-rev=base on the touched packages: 0 issues. go generate leaves no diff.
  • I ran a concurrent Tick + Restore + submit probe under -race -count=3. No race.
  • Terraform (hashicorp/aws 6.66.0, real provider, default endpoint override, serve --async-settle):
    • aws_batch_compute_environment (Fargate), aws_batch_scheduling_policy (fair share), two aws_batch_job_queue (fair-share and FIFO), and aws_batch_job_definition (container and multinode) all applied.
    • plan -detailed-exitcode returned 0 for everything except the pre-existing multinode drift in point 5 below.
    • I then updated weight_factor and the queue priority. The apply succeeded and plan returned 0 again.
    • destroy succeeded and follow-up describes return empty.
  • AWS CLI:
    • submit → describe shows STARTING → SUCCEEDED with Essential container in task exited, exitCode 0, and retryStrategy/timeout echoed.
    • dependsOn: the child stays PENDING while the parent is RUNNING.
    • Array job (size=3): the parent is PENDING with statusSummary. :1 resolves and :3 is omitted.
    • SEQUENTIAL array, then cancel on the parent: :0 SUCCEEDED, :1 and :2 FAILED, and the parent is FAILED with isCancelled.
    • cancel-job on a SUBMITTED job gives FAILED with isCancelled.
    • These all return ClientException with the right message: fair-share queue without shareIdentifier, jd:99, size=10001, unknown job id, jobStatus DONE, a non-numeric BEFORE_CREATED_AT, deleting an in-use policy, removing a queue's policy, --type batch, and an aws: tag key.
    • JOB_NAME HEL* matches case-insensitively.
  • EventBridge rule to SQS: RUNNABLE, STARTING, RUNNING, SUCCEEDED, and no event for the submission. Source is aws.batch, resources are [jobArn], and events for unpolled jobs arrive through the ticker.
  • No AI attribution in the PR body or in any of the 3 commits.

Medium

  1. nodeOverrides, ecsPropertiesOverride and eksPropertiesOverride are accepted and silently dropped (providers/aws/batch/jobs.go:299).
    • The wire layer and the driver carry all three into SubmitJobInput, but buildJob only merges containerOverrides. It copies the job definition's node/ECS/EKS properties unchanged.
    • Probe: I submitted with ecsPropertiesOverride and DescribeJobs returned the definition's ecsProperties (null here) without the override.
    • A user who overrides the command of an ECS or EKS job, or the node ranges of an MNP job, sees the original definition echoed back. Nothing errors, and nothing says the override was ignored.
    • Fix: merge them the way containerOverrides is merged, or reject them / list the gap in docs/coverage/nongoals/batch.md. Today the drop isn't documented anywhere.

Low

  1. SubmitJob accepts an explicit INACTIVE revision (jobs.go:267).
    • The name:revision / ARN-with-revision path returns the stored definition without checking Status. Probe: after DeregisterJobDefinition jd:1, SubmitJob with jd:1 succeeded (CLI too).
    • The bare-name path does require ACTIVE. I'd expect real Batch to refuse a deregistered revision with ClientException. Please confirm against the API reference and match it.
  2. Replayed intermediate events carry the SUCCEEDED-era container (events.go:192).
    • eventDetail sets Container: j.Container from the job as rendered now. Without --async-settle (the serve default) the job is already SUCCEEDED, so the RUNNABLE, STARTING and RUNNING events all carry container.exitCode: 0. I confirmed this with a probe.
    • statusReason, startedAt and stoppedAt are already stripped for replayed statuses. exitCode should be stripped the same way.
  3. A 1s phase makes TerminateJob on a RUNNING job hard to reach from a real client (internal/settle/settle.go:85).
    • With --async-settle a job goes from STARTING to SUCCEEDED in 2s.
    • Under load I polled until STARTING and then called terminate-job. The job was already SUCCEEDED when the call landed, because CLI round-trips take more than a second. The same happened in a dependency test.
    • The provider test covers the semantics, but CLI or Terraform users can't reliably exercise terminate-vs-cancel. Consider a longer RUNNING dwell, for example 5 to 10s, or making the dwell configurable.
  4. Pre-existing, not introduced here: multinode job definitions drift in Terraform.
    • RegisterJobDefinition defaults platformCapabilities to ["EC2"] when none is sent. aws_batch_job_definition with type = "multinode" and no platform_capabilities then plans - "EC2" forever (exit 2).
    • This PR rewrites the validation in the same function, so it could fix it here or in a follow-up: only default when real AWS does, or don't echo a default the caller didn't send.
  5. ListJobs --array-job-id ignores filters (jobs.go:641). children() only applies the status check. JOB_NAME and the other filters are validated, but the child list is returned unfiltered. --filters name=JOB_NAME,values=nomatch would still list every child (this comes from reading the code; my live call used a matching name). Either apply matchesFilters there or reject filters with arrayJobId.
  6. Check-then-act on scheduling-policy binding.
    • UpdateJobQueue reads the queue (job_queue.go:106) and then applies policyARN inside a separate Update.
    • DeleteSchedulingPolicy scans the queues (scheduling_policy.go:258) and then deletes the policy, while CreateJobQueue resolves the policy on its own.
    • A concurrent delete plus create or update can leave a queue pointing at a deleted policy. It needs concurrent control-plane calls, so this is low priority, but one small mutex around policy binding would close it.

Verdict: COMMENT, close to approve. Fix point 1, either in code or as a documented non-goal. Points 2 to 7 can follow.

// buildJob assembles the static detail of a new job from the request, the
// queue and the job definition (submit-time values override the definition's).
func (m *Mock) buildJob(in *driver.SubmitJobInput, q *driver.JobQueue, jd *driver.JobDefinition) (driver.Job, error) {
container, err := mergeContainer(jd.ContainerProperties, in.ContainerOverrides)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only containerOverrides is merged here. NodeOverrides, EcsPropertiesOverride and EksPropertiesOverride come in through the wire and the driver input but are never read, so DescribeJobs echoes the unmodified definition (probe: ecsPropertiesOverride set, ecsProperties came back null). Please merge them the same way, reject them, or at least list the gap in nongoals/batch.md. Right now they are dropped silently.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 689b550. buildJob now runs applyPropertyOverrides (new providers/aws/batch/overrides.go):

  • ecsPropertiesOverride: taskProperties[i] maps onto the definition's taskProperties[i], and each container override targets a container by name (an unnamed override is accepted only when there is exactly one container). environment merges by name, resourceRequirements by type, and command replaces.
  • eksPropertiesOverride: containers and initContainers by name (env merges by name; image, command, args and resources replace), and metadata labels/annotations merge key by key.
  • nodeOverrides: numNodes replaces the count under the documented conditions (an open-ended range whose lower bound is below it, and mainNode below it), and each nodePropertyOverride merges into every node range its targetNodes covers (containerOverrides, ECS/EKS overrides, instanceTypes, consumable resources).
  • An override the definition can't take is a ClientException rather than a silent drop: ECS/EKS override without matching properties, nodeOverrides on a non-multinode definition, an unknown container name, or a targetNodes covering no range.

Tests: TestSubmitJobAppliesEcsPropertiesOverride, TestSubmitJobAppliesEksPropertiesOverride, TestSubmitJobAppliesNodeOverrides, TestSubmitJobRejectsUnusableOverrides, and SDK TestSDKSubmitJobEcsPropertiesOverride. The remaining gap (no per-node nodeDetails, no platform check such as nodeOverrides on Fargate) is now in nongoals/batch.md.

func (m *Mock) resolveJobDefinition(ref string) (driver.JobDefinition, error) {
key := nameFromARN(kindJobDefinition, ref)

if strings.Contains(key, ":") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The explicit-revision path skips the ACTIVE check that the bare-name path applies. After DeregisterJobDefinition jd:1, SubmitJob --job-definition jd:1 still succeeds (I reproduced it with the CLI). I'd expect Batch to return a ClientException for an INACTIVE revision. Can you check the API reference and add a jd.Status != JDStatusActive guard if it matches?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 689b550. The explicit-revision path (name:revision or an ARN with a revision) now requires Status == ACTIVE and returns a ClientException otherwise. The SubmitJob reference says the revision forms resolve a specific revision and a bare name uses the latest ACTIVE one, and an INACTIVE revision can't be submitted to (this is the known Step Functions failure mode after CloudFormation deregisters old revisions). Test: TestSubmitJobRejectsInactiveRevision (both jd:1 and the full ARN are rejected after deregistering, and jd:2 still works). Also confirmed with the CLI.

JobArn: j.JobARN, JobName: j.JobName, JobID: j.JobID, JobQueue: j.JobQueue,
Status: status, Attempts: []any{}, CreatedAt: j.CreatedAt,
RetryStrategy: j.RetryStrategy, DependsOn: make([]jobEventDependency, 0, len(j.DependsOn)),
JobDefinition: j.JobDefinition, Parameters: j.Parameters, Container: j.Container,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Container comes from the job as rendered now. In sync mode (the serve default) that render is already SUCCEEDED, so the replayed RUNNABLE, STARTING and RUNNING events all carry container.exitCode: 0 (confirmed with a probe). Since status != j.Status already strips the reason and times, consider stripping exitCode from the container there as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 689b550. The replay branch now strips exitCode from the container (withoutExitCode) along with the reason and times, so only the terminal event carries it. Test: TestReplayedEventsCarryNoExitCode (sync mode: RUNNABLE/STARTING/RUNNING have no exitCode, SUCCEEDED has 0). Confirmed through an EventBridge rule to SQS on plain serve.

Comment thread internal/settle/settle.go
// DefaultBatchJobPhase is how long an AWS Batch job dwells in each
// non-terminal phase (SUBMITTED, RUNNABLE, STARTING, RUNNING) before moving
// on, so a dependency-free job reaches SUCCEEDED 4 phases after submission.
DefaultBatchJobPhase = 1 * time.Second

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

With 1s phases, STARTING to SUCCEEDED takes 2s. Under normal CLI latency I couldn't land terminate-job while the job was STARTING or RUNNING: the job was already SUCCEEDED each time. A longer RUNNING dwell (5 to 10s) would let real CLI and SDK users exercise terminate vs cancel and dependency PENDING.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 689b550. Added settle.DefaultBatchJobRunDwell = 5s (additive; the other phases stay at 1s), stored per job as RunPhase so it survives snapshots. A record from an older snapshot without it falls back to the phase length. With the CLI I polled to RUNNING and terminate-job landed every time: FAILED, isTerminated: true, startedAt kept. Tests: TestJobLifecycleAdvancesWithClock (asserts stoppedAt - startedAt = 5s) and TestJobsAndPoliciesSurviveSnapshot (the dwell survives restore). I didn't make it configurable to keep the scope small; happy to add a flag later if it is wanted.

return out
}

for i := int(l.after + 1); i < rec.arraySize(); i++ {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The array-child listing applies only the status filter. filters are validated but never applied here, so --array-job-id X --filters name=JOB_NAME,values=foo returns every child. Either run matchesFilters on each child or reject filters together with arrayJobId.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 689b550. children() now runs matchesFilters on each child, the same as the queue listing. Test: TestListArrayChildrenAppliesFilters (JOB_NAME=nomatch returns 0 children, ARR* returns all 3). The CLI gives the same result.

}
}

cur, ok := m.jobQueues.Get(name)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a separate read before the Update closure, and DeleteSchedulingPolicy scans queues outside any lock as well. A concurrent delete plus UpdateJobQueue/CreateJobQueue can leave a queue bound to a policy that no longer exists. It's minor, but resolving the policy inside the closure, plus a small mutex shared with the policy delete, would close the window.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 689b550. Added policyBindMu. CreateJobQueue holds it across the policy resolution and the insert, UpdateJobQueue across the read, the resolution and the update, and DeleteSchedulingPolicy across the in-use scan and the delete. It is a leaf lock (no other mutex is taken under it), so it can't deadlock with jobsMu or registerMu. Test: TestSchedulingPolicyBindingIsAtomic races create/update/delete 50 times and checks that no queue references a deleted policy. It failed every run before the change and passes under -race -count=5 now.

@Satyam-Trivedi-ZS

Copy link
Copy Markdown
Collaborator Author

Thanks for the thorough review, @NitinKumar004. All seven points are addressed in 689b550, including the pre-existing multinode drift, since the fix was small and safe.

# Finding Status
1 (Medium) nodeOverrides / ecsPropertiesOverride / eksPropertiesOverride dropped Fixed. They are merged into the job detail (new overrides.go). An override the definition can't take is a ClientException. Tests: TestSubmitJobAppliesEcsPropertiesOverride, TestSubmitJobAppliesEksPropertiesOverride, TestSubmitJobAppliesNodeOverrides, TestSubmitJobRejectsUnusableOverrides, SDK TestSDKSubmitJobEcsPropertiesOverride.
2 Explicit INACTIVE revision accepted Fixed. name:revision and revision ARNs must be ACTIVE. Test: TestSubmitJobRejectsInactiveRevision.
3 Replayed events carry exitCode Fixed. Replayed intermediate statuses strip exitCode. Test: TestReplayedEventsCarryNoExitCode.
4 RUNNING too short to terminate from a real client Fixed. RUNNING now lasts 5s (settle.DefaultBatchJobRunDwell, an additive constant). It is stored per job and survives snapshots. Tests: TestJobLifecycleAdvancesWithClock, TestJobsAndPoliciesSurviveSnapshot.
5 (pre-existing) Multinode definitions drift in Terraform (platformCapabilities default) Fixed. platformCapabilities is stored as sent, with no EC2 echo. Evidence: the Terraform provider's acceptance tests run against real AWS and assert platform_capabilities.# = 0 both for TestAccBatchJobDefinition_basic (container) and NodeProperties_basic (multinode) when the argument is omitted. Tests: TestMultinodeJobDefinitionEchoesNoDefaultPlatform, plus TestRegisterJobDefinitionRevisionIncrements (updated).
6 ListJobs --array-job-id ignores filters Fixed. Children go through matchesFilters. Test: TestListArrayChildrenAppliesFilters.
7 Check-then-act on policy binding Fixed. policyBindMu covers the CreateJobQueue / UpdateJobQueue binding and the DeleteSchedulingPolicy scan-and-delete. It is a leaf lock. Test: TestSchedulingPolicyBindingIsAtomic, which failed every run before the change.

Each new test fails on the previous head and passes now.

Notes on the choices I made:

  • Override matching. ECS/EKS container overrides target a container by name. An unnamed override is accepted only when the definition has exactly one container. A node override applies to every node range its targetNodes covers, and numNodes follows the three conditions in the NodeOverrides reference.
  • INACTIVE revisions. The SubmitJob reference only says that a bare name resolves to the latest ACTIVE revision. The refusal for an explicit INACTIVE revision follows the widely reported real-world behaviour: Step Functions executions fail after CloudFormation marks old revisions INACTIVE.

Verification:

  • go build ./...: ok.
  • go test -race on providers/aws/batch, server/aws/batch, providers/aws, persist and internal/settle: ok. TestConcurrentJobOperations and the binding test also pass with -race -count=5.
  • Full go test -p 2 (run in chunks): all pass.
  • golangci-lint --new-from-rev=upstream/development on the touched packages: 0 issues.
  • go generate ./... output is committed. Only the Batch non-goals text changed.
  • AWS CLI against serve --async-settle on 4601:
    • All three overrides show up in describe-jobs, and the bad-override cases return ClientException.
    • jd:1 is rejected after deregistration.
    • Polling reached RUNNING and terminate-job landed (isTerminated: true, startedAt kept).
    • list-jobs --array-job-id X --filters name=JOB_NAME,values=nomatch returns 0 children.
    • A multinode definition without platformCapabilities describes as null.
  • EventBridge to SQS on plain serve: RUNNABLE/STARTING/RUNNING carry no exitCode, and SUCCEEDED carries 0.

Follow-ups (not in this PR):

  • Per-node nodeDetails for multi-node jobs.
  • Rejecting overrides that don't fit the compute environment's platform (for example nodeOverrides on Fargate).
  • A flag to configure the RUNNING dwell, if 5s turns out not to be enough.

Both of the first two are listed in nongoals/batch.md. I updated the PR description to match.

@NitinKumar004 NitinKumar004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review of 689b550 against my review of 57fdca5. Every point from that review is fixed, and the new override code holds up against the CLI and Terraform. I have two small follow-ups on the override merge and one lint note. None of them blocks this PR.

Earlier findings

  1. (Medium) nodeOverrides, ecsPropertiesOverride and eksPropertiesOverride were dropped silently. Fixed. applyPropertyOverrides in overrides.go is now called from buildJob. With the CLI:
    • An ECS override on main replaced command and merged environment by name (X=1 kept, Y=2 added). The side container was left alone.
    • An EKS override replaced command, set env and merged the metadata labels key by key.
    • nodeOverrides set numNodes 2 to 4 and merged containerOverrides into the 0: range.
    • These all return ClientException: an ECS override on a containerProperties definition, an unknown container name, an unnamed override with two containers, nodeOverrides on a container definition, numNodes: 0, and targetNodes "5:7".
    • The remaining gaps are listed in nongoals/batch.md.
  2. (Low) SubmitJob accepted an INACTIVE revision. Fixed. After deregistering jd:1, both jd:1 and the full ARN with :1 return ClientException. jd:2 and bare jd still work.
  3. (Low) Replayed events carried exitCode: 0. Fixed. On plain serve, an EventBridge rule with an SQS target received RUNNABLE, STARTING and RUNNING with no exitCode, and SUCCEEDED with exitCode: 0.
  4. (Low) terminate-job was hard to reach on a RUNNING job. Fixed. RUNNING now lasts 5s (DefaultBatchJobRunDwell, stored per job as RunPhase, and old snapshots fall back to Phase). I polled to RUNNING and terminate-job gave FAILED, kill it, isTerminated: true, with startedAt and stoppedAt set. cancel-job on a RUNNING job left it RUNNING.
  5. (Low, pre-existing) Multinode job definitions drifted in Terraform. Fixed. No EC2 default is echoed now. In Terraform, aws_batch_job_definition with type = "multinode" and no platform_capabilities plans clean.
  6. (Low) ListJobs with arrayJobId ignored filters. Fixed. --filters name=JOB_NAME,values=nomatch returns 0 children and arr* returns 3.
  7. (Low) Scheduling-policy binding could race with a delete. Fixed. policyBindMu now covers create, update and delete. TestSchedulingPolicyBindingIsAtomic passes under -race -count=2. With the CLI, deleting a policy in use returns ClientException, and once the queue is gone the delete succeeds.

What I checked

  • go build ./... ok. go vet ok on providers/aws/batch, server/aws/batch, services/batch, internal/settle and providers/aws.
  • go test -count=1 passes for those packages and for persist. go test -race -count=2 passes for providers/aws/batch, server/aws/batch and internal/settle.
  • gofmt is clean, go mod tidy leaves no diff, and go generate leaves no diff.
  • I ran golangci-lint 2.14.0 on the touched packages without --new-from-rev and compared it with a merge-base worktree. The base has 22 issues and the head has 30. Of the 8 new ones, 7 are goconst and 1 is gosec G115 (see Low).
  • Terraform with hashicorp/aws 6.66.0, against serve --async-settle. I used a Fargate compute environment, a fair-share scheduling policy, a fair-share queue, a FIFO queue, a container definition and a multinode definition.
    • apply, then plan -detailed-exitcode returned 0.
    • I updated weight_factor and the queue priority, applied, and plan returned 0 again.
    • destroy, then follow-up describes return empty.
  • AWS CLI v2, with the queue, policy and lifecycle runs described above. A fair-share queue without shareIdentifier returns ClientException, and with A the job submits.
  • CI is green and the PR is mergeable. The PR body and the new commit have no AI attribution.

High

None.

Medium

None.

Low

  1. A node override that targets part of a definition range is rejected (providers/aws/batch/overrides.go:552).
    • applyNodePropertyOverride only applies to a definition range that lies wholly inside the override's targetNodes.
    • Take a definition with targetNodes "0:" and numNodes 2. An override with targetNodes "0:0" (main node only) matches no range, so SubmitJob fails with "doesn't cover any node range".
    • Real Batch takes a sub-range override and applies it to those nodes.
    • Either apply it to any overlapping range, which fits because the emulator runs no per-node workload, or list the limit in nongoals/batch.md.
  2. containerOverrides is still dropped on ECS, EKS and multinode definitions (providers/aws/batch/jobs.go:396).
    • mergeContainer returns nil when the definition has no containerProperties, so --container-overrides on an ecsProperties definition is accepted and then disappears.
    • The new override paths reject a mismatch like this, so the same check here would keep the behavior consistent.
  3. Lint: gosec G115 on int32(idx) (providers/aws/batch/job_lifecycle.go:538), plus 7 new goconst hits in jobs.go and overrides.go (environment, resourceRequirements, containerOverrides, instanceTypes, ...). G115 can't trigger, because idx is below the array size, which is at most 10,000. A bounded conversion or a //nolint:gosec with a reason would clear it, and the strings could become constants.

_ = json.Unmarshal(r["targetNodes"], &rt)

rr, ok := parseTargetNodes(rt, def.NumNodes)
if !ok || rr.start < or.start || rr.end > or.end {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A definition range is only matched when it lies wholly inside the override's targetNodes. With a definition range 0: (numNodes 2), an override targetNodes: "0:0" matches nothing, so SubmitJob returns "doesn't cover any node range". Real Batch applies a sub-range override to those nodes. Since there are no per-node workloads here, applying it to any overlapping range (or documenting the limit in nongoals) would avoid rejecting a valid request.

// containerProperties: environment variables and resource requirements are
// merged (by name / type), every other override replaces the property.
func mergeContainer(props, overrides json.RawMessage) (json.RawMessage, error) {
if !isSet(props) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When the definition has no containerProperties (ECS, EKS or multinode), this returns nil and containerOverrides is dropped without any error. The new ECS, EKS and node override paths reject a mismatch, so a matching ClientException here would make the behavior consistent.

v = o.childViews(rec)[idx]
j.JobID = rec.Job.JobID + ":" + strconv.Itoa(idx)
j.JobARN = rec.Job.JobARN + ":" + strconv.Itoa(idx)
i := int32(idx)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

golangci-lint 2.14 flags this as gosec G115 (int to int32). It can't overflow because idx is below the array size, which is at most 10,000, but a bounded conversion or a //nolint:gosec // idx < array size <= 10000 would keep the touched packages lint-clean.

…n type check and job state-change events

Add SubmitJob, DescribeJobs, ListJobs, CancelJob and TerminateJob. Jobs
move SUBMITTED -> PENDING -> RUNNABLE -> STARTING -> RUNNING -> SUCCEEDED
or FAILED on the provider clock, honour dependsOn and array jobs, and
follow the documented cancel vs terminate rules. SubmitJob validates the
queue (exists, ENABLED), the job definition revision, dependencies, share
identifier, array size, priority, timeout and tags. ListJobs supports the
documented filters and pages with a stable cursor.

Add scheduling-policy CRUD and validate schedulingPolicyArn on
CreateJobQueue/UpdateJobQueue. Enforce the RegisterJobDefinition type enum
(container, multinode) and the properties each type requires. Jobs and
scheduling policies are taggable, and tags follow the TagResource limits
and the reserved aws: prefix.

Publish "Batch Job State Change" events to the EventBridge default bus
from the shared serve scheduler (config.Tickable) and on job API calls.
New state is included in snapshots.

Part of stackshy#1496
Parse the array child index with strconv.ParseInt(..., 10, 32) so the
value converted into ArrayProperties.Index is bounded at parse time
(CodeQL go/incorrect-integer-conversion). lookup already rejected any
index at or above the array size, so no out-of-range value reached the
conversion; this makes the bound explicit.
- SubmitJob merges ecsPropertiesOverride, eksPropertiesOverride and
  nodeOverrides into the job detail instead of dropping them, and rejects
  an override the job definition can't take (wrong properties kind,
  unknown container name, numNodes without an open-ended range,
  targetNodes covering no range).
- SubmitJob refuses an explicit INACTIVE revision (name:revision or ARN).
- Replayed intermediate "Batch Job State Change" events no longer carry
  the final container exitCode.
- With async settle a job stays RUNNING for 5s (DefaultBatchJobRunDwell)
  so a real client can terminate a running job; the dwell is stored on
  the job record and survives snapshots.
- ListJobs with arrayJobId applies the JOB_NAME and other filters to the
  children.
- Scheduling-policy binding (CreateJobQueue, UpdateJobQueue) and
  DeleteSchedulingPolicy are serialized so a queue can't end up bound to a
  deleted policy.
- RegisterJobDefinition stores platformCapabilities as sent instead of
  echoing a default EC2, which made Terraform multinode definitions drift.
@Satyam-Trivedi-ZS
Satyam-Trivedi-ZS force-pushed the fix/aws-batch-parity-1496 branch from 689b550 to db8553e Compare October 23, 2026 20:10
@Satyam-Trivedi-ZS
Satyam-Trivedi-ZS merged commit e6b4b94 into stackshy:development Oct 23, 2026
23 checks passed
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.

3 participants