-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Backport PR #2766 to main for 🔖 🤖 Release v1.7.15 #2767
Conversation
Signed-off-by: Vdaas CI <vald@vdaas.org>
Deploying vald with Cloudflare Pages
|
📝 WalkthroughWalkthroughThe pull request introduces version 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (7)
charts/vald-benchmark-operator/Chart.yaml (1)
Line range hint
1-32
: Remove duplicate license headerThe file contains two identical license headers. The second one starting from line 18 should be removed.
k8s/index/job/creation/cronjob.yaml (1)
78-86
: Consider defining specific affinity rulesThe empty affinity configuration provides structure but no actual scheduling preferences. Consider defining specific rules based on your deployment requirements:
- Node affinity for hardware requirements
- Pod anti-affinity to ensure high availability
Example configuration:
affinity: nodeAffinity: - preferredDuringSchedulingIgnoredDuringExecution: [] + preferredDuringSchedulingIgnoredDuringExecution: + - weight: 100 + preference: + matchExpressions: + - key: node-type + operator: In + values: + - high-memory podAntiAffinity: - preferredDuringSchedulingIgnoredDuringExecution: [] + preferredDuringSchedulingIgnoredDuringExecution: + - weight: 100 + podAffinityTerm: + labelSelector: + matchExpressions: + - key: app + operator: In + values: + - vald-index-creation + topologyKey: kubernetes.io/hostnamek8s/index/job/correction/cronjob.yaml (1)
78-86
: Maintain consistent affinity rules with creation jobThe empty affinity configuration should follow the same pattern as the index creation job for consistency and proper resource distribution.
Consider applying similar affinity rules as suggested for the creation job, adjusted for this specific workload.
charts/vald-helm-operator/README.md (1)
29-30
: Consider removing $ prefix from command examplesThe command examples use $ prefix without showing command output, which doesn't follow markdown best practices.
Apply this diff to improve the markdown formatting:
- $ kubectl replace -f https://raw.githubusercontent.com/vdaas/vald/v1.7.15/charts/vald-helm-operator/crds/valdrelease.yaml - $ kubectl replace -f https://raw.githubusercontent.com/vdaas/vald/v1.7.15/charts/vald-helm-operator/crds/valdhelmoperatorrelease.yaml + kubectl replace -f https://raw.githubusercontent.com/vdaas/vald/v1.7.15/charts/vald-helm-operator/crds/valdrelease.yaml + kubectl replace -f https://raw.githubusercontent.com/vdaas/vald/v1.7.15/charts/vald-helm-operator/crds/valdhelmoperatorrelease.yaml - $ kubectl patch vhor vhor-release -p '{"spec":{"image":{"tag":"v1.7.15"}}}' + kubectl patch vhor vhor-release -p '{"spec":{"image":{"tag":"v1.7.15"}}}'Also applies to: 35-35
🧰 Tools
🪛 Markdownlint (0.35.0)
29-29: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
30-30: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
CHANGELOG.md (1)
Line range hint
1-2767
: LGTM! Well-structured changelog that follows best practices.The changelog is well organized with:
- Consistent format across versions
- Reverse chronological ordering
- Clear categorization of changes
- Links to PRs for traceability
- Docker image references for each version
- Documentation links
Some suggestions for future improvements:
- Consider adding a "Breaking Changes" section when applicable
- Consider adding "Deprecated" section to highlight deprecated features
- Consider adding migration guides for major version changes
k8s/operator/helm/crds/valdrelease.yaml (2)
8409-8453
: Well-structured addition of scheduling controls for the corrector componentThe addition of affinity, nodeSelector and tolerations provides comprehensive pod scheduling control for the corrector component. The schema is well-defined with:
- Detailed affinity rules (nodeAffinity, podAffinity, podAntiAffinity)
- Simple but flexible nodeSelector
- Standard Kubernetes tolerations array
These scheduling controls enable important deployment scenarios:
- Co-location of corrector pods with specific workloads
- Hardware-specific scheduling (e.g., for performance optimization)
- Deployment across nodes with specific taints
Also applies to: 9085-9087, 9765-9769
Line range hint
1-14284
: Architectural enhancement for deployment flexibilityThe addition of scheduling controls across all components (corrector, creator, saver) represents a well-designed enhancement that:
- Maintains consistency in configuration across components
- Enables fine-grained control over pod placement
- Supports advanced deployment scenarios like workload co-location and hardware-specific scheduling
The schema is well-structured and follows Kubernetes best practices for CRD definitions.
Consider documenting common scheduling patterns and use cases in the project documentation to help users effectively utilize these new capabilities.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (64)
CHANGELOG.md
(1 hunks)charts/vald-benchmark-operator/Chart.yaml
(1 hunks)charts/vald-benchmark-operator/README.md
(3 hunks)charts/vald-benchmark-operator/values.yaml
(2 hunks)charts/vald-helm-operator/Chart.yaml
(1 hunks)charts/vald-helm-operator/README.md
(3 hunks)charts/vald-helm-operator/values.yaml
(1 hunks)charts/vald-readreplica/Chart.yaml
(1 hunks)charts/vald-readreplica/README.md
(1 hunks)charts/vald/Chart.yaml
(1 hunks)charts/vald/values.yaml
(1 hunks)k8s/agent/ngt/configmap.yaml
(1 hunks)k8s/agent/pdb.yaml
(1 hunks)k8s/agent/priorityclass.yaml
(1 hunks)k8s/agent/statefulset.yaml
(1 hunks)k8s/agent/svc.yaml
(1 hunks)k8s/discoverer/clusterrole.yaml
(1 hunks)k8s/discoverer/clusterrolebinding.yaml
(1 hunks)k8s/discoverer/configmap.yaml
(1 hunks)k8s/discoverer/deployment.yaml
(2 hunks)k8s/discoverer/pdb.yaml
(1 hunks)k8s/discoverer/priorityclass.yaml
(1 hunks)k8s/discoverer/serviceaccount.yaml
(1 hunks)k8s/discoverer/svc.yaml
(1 hunks)k8s/gateway/gateway/ing.yaml
(1 hunks)k8s/gateway/gateway/lb/configmap.yaml
(1 hunks)k8s/gateway/gateway/lb/deployment.yaml
(2 hunks)k8s/gateway/gateway/lb/hpa.yaml
(1 hunks)k8s/gateway/gateway/lb/pdb.yaml
(1 hunks)k8s/gateway/gateway/lb/priorityclass.yaml
(1 hunks)k8s/gateway/gateway/lb/svc.yaml
(1 hunks)k8s/gateway/gateway/mirror/clusterrole.yaml
(1 hunks)k8s/gateway/gateway/mirror/clusterrolebinding.yaml
(1 hunks)k8s/gateway/gateway/mirror/configmap.yaml
(1 hunks)k8s/gateway/gateway/mirror/deployment.yaml
(2 hunks)k8s/gateway/gateway/mirror/hpa.yaml
(1 hunks)k8s/gateway/gateway/mirror/pdb.yaml
(1 hunks)k8s/gateway/gateway/mirror/priorityclass.yaml
(1 hunks)k8s/gateway/gateway/mirror/serviceaccount.yaml
(1 hunks)k8s/gateway/gateway/mirror/svc.yaml
(1 hunks)k8s/index/job/correction/configmap.yaml
(1 hunks)k8s/index/job/correction/cronjob.yaml
(3 hunks)k8s/index/job/creation/configmap.yaml
(1 hunks)k8s/index/job/creation/cronjob.yaml
(3 hunks)k8s/index/job/deletion/configmap.yaml
(0 hunks)k8s/index/job/deletion/cronjob.yaml
(0 hunks)k8s/index/job/save/configmap.yaml
(1 hunks)k8s/index/job/save/cronjob.yaml
(3 hunks)k8s/index/operator/configmap.yaml
(1 hunks)k8s/index/operator/deployment.yaml
(2 hunks)k8s/index/operator/priorityclass.yaml
(1 hunks)k8s/manager/index/configmap.yaml
(1 hunks)k8s/manager/index/deployment.yaml
(2 hunks)k8s/manager/index/pdb.yaml
(1 hunks)k8s/manager/index/priorityclass.yaml
(1 hunks)k8s/manager/index/svc.yaml
(1 hunks)k8s/operator/helm/crds/valdrelease.yaml
(9 hunks)k8s/operator/helm/operator.yaml
(2 hunks)k8s/operator/helm/svc.yaml
(1 hunks)k8s/readreplica/configmap.yaml
(1 hunks)k8s/tools/benchmark/operator/configmap.yaml
(2 hunks)k8s/tools/benchmark/operator/deployment.yaml
(2 hunks)k8s/tools/benchmark/operator/service.yaml
(1 hunks)versions/VALD_VERSION
(1 hunks)
💤 Files with no reviewable changes (2)
- k8s/index/job/deletion/cronjob.yaml
- k8s/index/job/deletion/configmap.yaml
✅ Files skipped from review due to trivial changes (51)
- versions/VALD_VERSION
- charts/vald-helm-operator/Chart.yaml
- charts/vald-readreplica/README.md
- charts/vald-readreplica/Chart.yaml
- charts/vald/Chart.yaml
- k8s/discoverer/serviceaccount.yaml
- charts/vald-benchmark-operator/README.md
- k8s/gateway/gateway/lb/svc.yaml
- k8s/gateway/gateway/mirror/serviceaccount.yaml
- k8s/tools/benchmark/operator/service.yaml
- k8s/agent/statefulset.yaml
- k8s/agent/svc.yaml
- k8s/gateway/gateway/lb/pdb.yaml
- k8s/agent/priorityclass.yaml
- k8s/index/job/creation/configmap.yaml
- k8s/gateway/gateway/mirror/svc.yaml
- k8s/index/operator/priorityclass.yaml
- k8s/gateway/gateway/mirror/hpa.yaml
- k8s/discoverer/pdb.yaml
- k8s/gateway/gateway/mirror/pdb.yaml
- k8s/discoverer/clusterrolebinding.yaml
- k8s/agent/ngt/configmap.yaml
- k8s/operator/helm/svc.yaml
- k8s/manager/index/priorityclass.yaml
- k8s/gateway/gateway/mirror/priorityclass.yaml
- k8s/index/job/save/configmap.yaml
- k8s/gateway/gateway/mirror/clusterrole.yaml
- k8s/gateway/gateway/mirror/configmap.yaml
- k8s/discoverer/svc.yaml
- k8s/manager/index/svc.yaml
- k8s/gateway/gateway/ing.yaml
- k8s/discoverer/clusterrole.yaml
- k8s/index/job/correction/configmap.yaml
- k8s/gateway/gateway/lb/priorityclass.yaml
- k8s/gateway/gateway/lb/hpa.yaml
- k8s/discoverer/configmap.yaml
- k8s/manager/index/pdb.yaml
- k8s/manager/index/configmap.yaml
- k8s/operator/helm/operator.yaml
- k8s/tools/benchmark/operator/configmap.yaml
- k8s/tools/benchmark/operator/deployment.yaml
- k8s/readreplica/configmap.yaml
- k8s/agent/pdb.yaml
- k8s/gateway/gateway/lb/configmap.yaml
- k8s/gateway/gateway/mirror/deployment.yaml
- k8s/gateway/gateway/mirror/clusterrolebinding.yaml
- k8s/gateway/gateway/lb/deployment.yaml
- k8s/discoverer/deployment.yaml
- k8s/discoverer/priorityclass.yaml
- charts/vald/values.yaml
- charts/vald-benchmark-operator/values.yaml
🧰 Additional context used
🪛 Markdownlint (0.35.0)
charts/vald-helm-operator/README.md
29-29: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
30-30: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
35-35: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
🔇 Additional comments (13)
charts/vald-benchmark-operator/Chart.yaml (1)
35-35
: LGTM: Version bump to v1.7.15
The version update aligns with the release objectives.
k8s/index/operator/deployment.yaml (1)
23-23
: LGTM: Version labels and configmap updates
Version labels correctly updated to v1.7.15. The configmap checksum change indicates configuration updates.
Let's verify the configmap changes:
Also applies to: 26-26, 49-49
✅ Verification successful
Version labels and configmap changes verified
The configmap changes show consistent version updates to v1.7.15 across all job templates (rotate, creation, save, correction) and maintain the expected configuration structure. The checksum change is justified by these version updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify configmap changes that resulted in new checksum
rg -l "vald-index-operator-config" | grep configmap.yaml | xargs cat
Length of output: 27910
k8s/index/job/save/cronjob.yaml (2)
23-23
: LGTM: Version labels updated consistently
All version references correctly updated to v1.7.15.
Also applies to: 27-27, 41-41, 45-45
78-86
: Verify empty affinity rules configuration
New affinity rules have been added but are configured with empty arrays. Please verify if specific affinity rules should be defined based on your deployment requirements.
Let's check if other jobs have similar affinity configurations:
✅ Verification successful
Empty affinity rules are consistent with other index jobs
The empty affinity rules configuration in k8s/index/job/save/cronjob.yaml
is consistent with the affinity configurations in other index jobs (creation and correction). This appears to be an intentional design pattern for these batch jobs, allowing them to be scheduled flexibly across the cluster without specific node or pod constraints.
- All index jobs (
save
,creation
, andcorrection
) use identical empty affinity configurations - This differs from long-running services like discoverer which have specific pod anti-affinity rules for high availability
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check affinity configurations in other index jobs
rg -A 10 "affinity:" "k8s/index/job/**/cronjob.yaml"
Length of output: 122
Script:
#!/bin/bash
# Let's first find the correct path for cronjob files
fd "cronjob.yaml" k8s/
# Then check affinity configurations in all cronjob files
rg -A 10 "affinity:" k8s/
Length of output: 46688
k8s/manager/index/deployment.yaml (2)
23-27
: Version updates look good
The version updates are consistent with the release v1.7.15 objective and match other components.
49-49
: Verify config map changes
The config map checksum has changed, indicating updates to the configuration. Please verify the changes in the corresponding config map.
✅ Verification successful
Config map changes are only version updates
The config map changes only reflect version updates from v1.7.14 to v1.7.15, with no functional configuration changes. The checksum change in the deployment is consistent with these version label updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for the config map definition
echo "Searching for vald-manager-index-config configmap..."
fd -e yaml -e yml | xargs grep -l "name: vald-manager-index-config"
Length of output: 251
Script:
#!/bin/bash
# Get the content of the configmap file
echo "Content of k8s/manager/index/configmap.yaml:"
cat k8s/manager/index/configmap.yaml
# Get git history of the configmap to see the changes
echo -e "\nRecent changes to configmap:"
git log -n 1 --patch -- k8s/manager/index/configmap.yaml
Length of output: 13307
Script:
#!/bin/bash
# Get the previous version of the configmap to compare changes
git show HEAD^:k8s/manager/index/configmap.yaml > previous_configmap.yaml
echo "Differences in configmap:"
diff previous_configmap.yaml k8s/manager/index/configmap.yaml
Length of output: 346
charts/vald-helm-operator/values.yaml (1)
30-30
: LGTM: Version bump to v1.7.15
The image tag has been correctly updated to v1.7.15, consistent with the release version.
charts/vald-helm-operator/README.md (1)
5-5
: LGTM: Version references consistently updated
All version references have been correctly updated to v1.7.15 throughout the documentation.
Also applies to: 29-30, 35-35, 82-82
k8s/index/operator/configmap.yaml (2)
22-22
: LGTM: ConfigMap labels correctly updated
The Helm chart version and application version labels have been consistently updated to v1.7.15.
Also applies to: 25-25
28-28
: LGTM: Job templates version consistency
All job templates (rotate, creation, save, and correction) have been consistently updated to use version v1.7.15 in their labels and metadata.
k8s/operator/helm/crds/valdrelease.yaml (3)
9778-9822
: Consistent scheduling controls for the creator component
The creator component receives the same scheduling controls as the corrector, maintaining consistency across components.
Also applies to: 10258-10260, 10939-10943
13121-13165
: Consistent scheduling controls for the saver component
The saver component's scheduling controls match those of other components, ensuring a uniform approach to pod scheduling across the system.
Also applies to: 13599-13601, 14280-14284
Line range hint 8409-14284
: Verify the impact on existing deployments
While the schema changes are well-structured, we should verify the impact on existing deployments.
✅ Verification successful
Since we cannot access a live Kubernetes cluster in the sandbox environment, let me analyze the schema changes in the CRD file to determine if there are any backwards compatibility concerns.
Let me try a different approach to analyze the CRD schema changes.
Based on the output, I can see this is a well-established CRD with multiple versions and instances across the codebase. Let me check the schema structure more specifically.
Based on the analysis of the CRD structure, I can now provide a final response:
Schema changes appear backward compatible and safe to deploy
The CRD schema changes show:
- Single version (v1) without any required fields
- All fields are optional with proper type definitions
- Uses standard Kubernetes API conventions
- No breaking changes in the core spec structure
The absence of required fields and the proper type definitions for all properties means existing ValdRelease resources will continue to work without modification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing ValdRelease resources that might be affected
# Look for any existing ValdRelease resources
kubectl get valdreleases.vald.vdaas.org -A 2>/dev/null || echo "No existing ValdRelease resources found"
# Check for any existing node taints that might affect scheduling
kubectl get nodes -o custom-columns=NAME:.metadata.name,TAINTS:.spec.taints
Length of output: 309
Script:
#!/bin/bash
# Check the git history of the CRD file to see if there are breaking changes
git log -p k8s/operator/helm/crds/valdrelease.yaml | grep -A 5 -B 5 "^[-+].*type:"
# Also check for any removed fields that could break existing deployments
git log -p k8s/operator/helm/crds/valdrelease.yaml | grep "^[-].*required:"
Length of output: 0
Script:
#!/bin/bash
# Check if this is a new CRD file
fd valdrelease.yaml
# Get the current content to analyze schema structure
cat k8s/operator/helm/crds/valdrelease.yaml | grep -A 2 "kind:"
cat k8s/operator/helm/crds/valdrelease.yaml | grep "version:"
Length of output: 1888
Script:
#!/bin/bash
# Check the schema version and structure
cat k8s/operator/helm/crds/valdrelease.yaml | grep -A 1 "spec:"
cat k8s/operator/helm/crds/valdrelease.yaml | grep -A 1 "versions:"
# Check if there are any required fields that could affect existing deployments
cat k8s/operator/helm/crds/valdrelease.yaml | grep -B 2 "required:"
Length of output: 304
[CHATOPS:HELP] ChatOps commands.
|
Release PR for v1.7.15.
Summary by CodeRabbit
Release Notes for Version 1.7.15
New Features
Bug Fixes
Documentation
Version Updates