Backport PR #2758 to release/v1.7 for add affinity to jobTemplate #2760
Conversation
Deploying vald with
|
| Latest commit: |
d09f0d4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5ac2a86a.vald.pages.dev |
| Branch Preview URL: | https://backport-release-v1-7-featur-imvm.vald.pages.dev |
📝 WalkthroughWalkthroughThe pull request introduces significant modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[CHATOPS:HELP] ChatOps commands.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
charts/vald/values.schema.json (2)
15230-15233: Consider enhancing nodeSelector schema validationThe nodeSelector schema is correctly implemented but could benefit from additional validation rules. Consider adding:
- Pattern validation for keys (must be valid Kubernetes label keys)
- Pattern validation for values (must be valid Kubernetes label values)
Example enhancement:
"nodeSelector": { "type": "object", "description": "node selector", + "additionalProperties": { + "type": "string", + "pattern": "^[a-zA-Z0-9][-._a-zA-Z0-9]*$" + }, + "propertyNames": { + "pattern": "^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\\/)?[a-z0-9]([-a-z0-9]*[a-z0-9])?$" + } }Also applies to: 17280-17283, 23051-23054
16452-16456: Consider enhancing tolerations schema validationThe tolerations schema is correctly implemented but could benefit from additional validation rules to match Kubernetes API specifications.
Example enhancement:
"tolerations": { "type": "array", "description": "tolerations", - "items": { "type": "object" } + "items": { + "type": "object", + "properties": { + "key": { "type": "string" }, + "operator": { + "type": "string", + "enum": ["Equal", "Exists"] + }, + "value": { "type": "string" }, + "effect": { + "type": "string", + "enum": ["NoSchedule", "PreferNoSchedule", "NoExecute"] + }, + "tolerationSeconds": { "type": "integer" } + } + } }Also applies to: 18502-18506, 24273-24277
charts/vald/values.yaml (3)
3430-3453: LGTM! Consider adding examples in comments for common affinity use cases.The affinity configuration for the corrector job looks good and follows Kubernetes best practices. To improve usability, consider adding comment examples showing common affinity rules like:
- Co-locating with specific services
- Spreading across zones
- Hardware requirements
3566-3589: LGTM! Consider adding examples in comments for common affinity use cases.The affinity configuration for the creator job looks good and follows Kubernetes best practices. To improve usability, consider adding comment examples showing common affinity rules like:
- Co-locating with specific services
- Spreading across zones
- Hardware requirements
3696-3719: LGTM! Consider adding examples in comments for common affinity use cases.The affinity configuration for the saver job looks good and follows Kubernetes best practices. To improve usability, consider adding comment examples showing common affinity rules like:
- Co-locating with specific services
- Spreading across zones
- Hardware requirements
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
charts/vald-helm-operator/crds/valdrelease.yaml(9 hunks)charts/vald/templates/_helpers.tpl(2 hunks)charts/vald/values.schema.json(9 hunks)charts/vald/values.yaml(3 hunks)
🔇 Additional comments (9)
charts/vald/templates/_helpers.tpl (3)
899-902: Addition of affinity settings to the job template is appropriate
The affinity field is correctly added to the job specification when .Job.affinity is provided, enabling node affinity configurations for job scheduling.
936-939: Addition of nodeSelector to the job template is appropriate
The nodeSelector field is properly included in the job specification when .Job.nodeSelector is defined, allowing for selection of nodes matching specified labels.
940-943: Addition of tolerations to the job template is appropriate
The tolerations field is correctly added to the job specification when .Job.tolerations is provided, allowing the job to tolerate specific node taints.
charts/vald/values.schema.json (2)
14074-14128: LGTM! Well-structured affinity configurations
The affinity schema definitions for corrector, creator, and saver components are well-structured and correctly follow the Kubernetes pod scheduling API. The schema includes all necessary affinity types (nodeAffinity, podAffinity, podAntiAffinity) with proper descriptions and type definitions.
Also applies to: 16471-16525, 22246-22300
Line range hint 14074-24277: Verify configuration examples in documentation
The schema changes for adding affinity, nodeSelector, and tolerations to the jobTemplate are well-implemented and consistent across all components (corrector, creator, and saver). Since this is a backport to release/v1.7, ensure that:
- The documentation includes examples of these new configuration options
- The changes are reflected in the default values.yaml
- Any upgrade notes mention these new configuration capabilities
charts/vald-helm-operator/crds/valdrelease.yaml (4)
8409-8453: LGTM: Affinity configuration follows Kubernetes standards
The affinity configuration structure correctly implements the standard Kubernetes affinity schema with all required fields:
- nodeAffinity with preferred/required scheduling rules
- podAffinity with preferred/required scheduling rules
- podAntiAffinity with preferred/required scheduling rules
9085-9087: LGTM: NodeSelector configuration follows Kubernetes standards
The nodeSelector configuration is properly defined as an object with x-kubernetes-preserve-unknown-fields to allow arbitrary label selectors.
9765-9769: LGTM: Tolerations configuration follows Kubernetes standards
The tolerations configuration is correctly defined as an array of objects with x-kubernetes-preserve-unknown-fields to allow standard Kubernetes toleration rules.
Line range hint 8409-14284: Verify consistent implementation across components
The affinity, nodeSelector and tolerations configurations are consistently implemented across the corrector, creator and saver components.
✅ Verification successful
Based on the output from the shell scripts, I can see that the affinity configuration is consistently implemented across all components in the CRD, with identical structure for nodeAffinity, podAffinity and podAntiAffinity fields. Let me verify the nodeSelector and tolerations with a modified script.
Based on the verification results, I can now provide the final response:
Scheduling controls are consistently implemented across components
The affinity, nodeSelector, and tolerations configurations show consistent implementation across all components:
- All affinity configurations follow the same structure with nodeAffinity, podAffinity, and podAntiAffinity
- All nodeSelector fields are defined as objects with x-kubernetes-preserve-unknown-fields
- All tolerations are defined as arrays of objects with x-kubernetes-preserve-unknown-fields
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent implementation of scheduling controls across components
# Check for consistent affinity structure
echo "Checking affinity structure..."
ast-grep --pattern 'affinity:
type: object
properties:
nodeAffinity:
$$$
podAffinity:
$$$
podAntiAffinity:
$$$'
# Check for consistent nodeSelector structure
echo "Checking nodeSelector structure..."
ast-grep --pattern 'nodeSelector:
type: object
x-kubernetes-preserve-unknown-fields: true'
# Check for consistent tolerations structure
echo "Checking tolerations structure..."
ast-grep --pattern 'tolerations:
type: array
items:
type: object
x-kubernetes-preserve-unknown-fields: true'
Length of output: 66762
Script:
#!/bin/bash
# Check for consistent nodeSelector structure
echo "Checking nodeSelector structure..."
rg -A 1 'nodeSelector:' charts/vald-helm-operator/crds/valdrelease.yaml
# Check for consistent tolerations structure
echo "Checking tolerations structure..."
rg -A 2 'tolerations:' charts/vald-helm-operator/crds/valdrelease.yaml
Length of output: 2242
kmrmt
deleted the
backport/release/v1.7/feature/manager-index/add-affinities-to-job-fix
branch
Description
Add affinity, nodeSelector and torelations to jobTemplate in manager/index (corrector, creator, saver).
Related Issue
Versions
Checklist
Special notes for your reviewer
Summary by CodeRabbit
Release Notes
New Features
corrector,creator, andsavercomponents.nodeSelectorandtolerationsoptions for more granular pod scheduling control.Improvements
values.yamlfile for better resource management and deployment strategies.affinity,nodeSelector, andtolerations.