Skip to content

Fix #6508: Add resource request/limit support for init and sidecar containers - #6721

Open
641-git641 wants to merge 2 commits into
apache:mainfrom
641-git641:feature/6508-init-container-resources
Open

Fix #6508: Add resource request/limit support for init and sidecar containers#6721
641-git641 wants to merge 2 commits into
apache:mainfrom
641-git641:feature/6508-init-container-resources

Conversation

@641-git641

Copy link
Copy Markdown

The existing positional format (name;image;command) continues to work unchanged.

Changes

containerTask struct: added 4 new fields
parseSingleTask(): parses key=value resource keys with resource.ParseQuantity validation
configureContainers(): sets corev1.ResourceRequirements when resource fields are present
6 new unit tests covering full resources, partial resources, invalid values, end-to-end apply, and backward compatibility

Fixes #6508

AI Generated with Claude Code on behalf of 641-git641

…car containers

Co-Authored-By: Claude Code on behalf of 641-git641
@github-actions

Copy link
Copy Markdown
Contributor

✔️ Unit test coverage report - coverage increased from 63.1% to 63.2% (+0.1%)

@squakez squakez 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.

LGTM. Just fix the lint warnings. Also, it would be good to clarify what happens when you have mixed arguments and provide a test for it, for example my-task;my-image;my-command:request-cpu=123m

if len(strings.TrimSpace(seg)) == 0 {
continue
}
if strings.Contains(seg, "=") {

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.

Feel free to add a nolint statement to avoid the warn

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

OK

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed: added //nolint:nestif, fixed the if-else chain, and added tests for mixed format (my-task;my-image;my-command;request-cpu=123m). PTAL.

- Replace if-else chain with switch (gocritic)
- Add //nolint:nestif for key=value parsing block
- Add tests for mixed positional + key=value format
  (e.g. my-task;my-image;my-command;request-cpu=123m)
@641-git641
641-git641 force-pushed the feature/6508-init-container-resources branch from f32cde5 to 39f83c2 Compare July 28, 2026 14:10
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.

Allow specifying CPU/Memory resources for Init and Sidecar Containers

2 participants