Backport PR #2775 to release/v1.7 for Update libs dependency #2785
Conversation
* [create-pull-request] automated change Signed-off-by: vdaas-ci <vald@vdaas.org> * 🔧 Fix build error Signed-off-by: vankichi <kyukawa315@gmail.com> * Added output for Go Module with fixed Version (#2784) * fix Signed-off-by: kpango <kpango@vdaas.org> * fix Signed-off-by: kpango <kpango@vdaas.org> --------- Signed-off-by: kpango <kpango@vdaas.org> Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com> * 🔧 Update go deps Signed-off-by: vankichi <kyukawa315@gmail.com> * 🔧 Update deps Signed-off-by: vankichi <kyukawa315@gmail.com> * ✅ Fix race condition Signed-off-by: vankichi <kyukawa315@gmail.com> * Revert "🔧 Update deps" This reverts commit 4622c52. * 🔧 Update deps Signed-off-by: vankichi <kyukawa315@gmail.com> * 🔧 Update deps Signed-off-by: vankichi <kyukawa315@gmail.com> * Revert "✅ Fix race condition" This reverts commit eb049c4. * 🔧 Update deps Signed-off-by: vankichi <kyukawa315@gmail.com> * ✅ Fix unit test Signed-off-by: vankichi <kyukawa315@gmail.com> * Update apis/grpc/v1/agent/sidecar/sidecar_vtproto.pb.go Co-authored-by: Yusuke Kato <kpango@vdaas.org> Signed-off-by: Kiichiro YUKAWA <kyukawa315@gmail.com> --------- Signed-off-by: vdaas-ci <vald@vdaas.org> Signed-off-by: vankichi <kyukawa315@gmail.com> Signed-off-by: kpango <kpango@vdaas.org> Signed-off-by: Kiichiro YUKAWA <kyukawa315@gmail.com> Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com> Co-authored-by: Yusuke Kato <kpango@vdaas.org>
Deploying vald with
|
| Latest commit: |
5241e5a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://37a52ef9.vald.pages.dev |
| Branch Preview URL: | https://backport-release-v1-7-create.vald.pages.dev |
📝 WalkthroughWalkthroughThis pull request involves comprehensive version updates across multiple files in the Vald project. The changes primarily focus on updating Kubernetes version from v1.31.4 to v1.32.0, upgrading various dependencies in Go modules, modifying Custom Resource Definitions (CRDs), and updating interceptor configurations in JSON schemas. The updates span across GitHub templates, Makefiles, charts, and core algorithm implementations. Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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.
|
github-actions
bot
added
area/internal
type/ci
area/helm
area/makefile
size/XXXL
labels
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
k8s/tools/benchmark/operator/crds/valdbenchmarkjob.yaml (2)
Line range hint
378-602: Consider optimizing schema to reduce duplicationThe health check configurations for liveness, readiness, and startup probes share identical structures. Consider defining a common schema definition that can be reused.
Example approach:
# Define a common probe schema definitions: probe: type: object properties: enabled: type: boolean port: maximum: 65535 minimum: 0 type: integer # ... other common properties # Reference the common schema healths: properties: liveness: $ref: "#/definitions/probe" readiness: $ref: "#/definitions/probe" startup: $ref: "#/definitions/probe"
Line range hint
1-823: Consider implementing OpenAPI schema optimizationsWhile the CRD schema is functionally complete, consider the following architectural improvements:
- Define common schema patterns in a
definitionssection to reduce duplication- Use schema references (
$ref) to reuse common patterns- Consider extracting complex nested objects into separate Custom Resources for better maintainability
These changes would make the schema more maintainable and reduce the risk of inconsistencies when updating similar structures.
hack/go.mod.default (1)
134-134: Keep the comment updated.
The comment references GitHub issue #529 for “go-json.” Make sure that this pinned version addresses the linked issue and that the pinned comment is still relevant.example/client/go.mod (1)
36-36: Updated indirect reference for golang.org/x/net.
Ensure the indirect version aligns with the direct version at lines 12–13 to avoid dependency confusion in certain build configurations.go.mod (2)
426-436: Additional cloud references
No major concern, but watch for any newly introduced environment variable dependencies.
532-532: google.golang.org/api v0.211.0 // indirect
Potential misalignment with direct references. Confirm that everything is pinned consistently.Would you like a script to compare direct vs. indirect references for google.golang.org/api?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
example/client/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumrust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.github/ISSUE_TEMPLATE/bug_report.md(1 hunks).github/ISSUE_TEMPLATE/security_issue_report.md(1 hunks).github/PULL_REQUEST_TEMPLATE.md(1 hunks)Makefile.d/dependencies.mk(1 hunks)charts/vald-benchmark-operator/values.schema.json(2 hunks)charts/vald/values.schema.json(17 hunks)example/client/go.mod(2 hunks)go.mod(14 hunks)hack/go.mod.default(3 hunks)internal/core/algorithm/ngt/ngt_test.go(22 hunks)k8s/tools/benchmark/operator/crds/valdbenchmarkjob.yaml(27 hunks)k8s/tools/benchmark/operator/crds/valdbenchmarkoperatorrelease.yaml(27 hunks)k8s/tools/benchmark/operator/crds/valdbenchmarkscenario.yaml(1 hunks)versions/KUBECTL_VERSION(1 hunks)versions/PROMETHEUS_STACK_VERSION(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- versions/KUBECTL_VERSION
- versions/PROMETHEUS_STACK_VERSION
- .github/ISSUE_TEMPLATE/security_issue_report.md
- .github/ISSUE_TEMPLATE/bug_report.md
- .github/PULL_REQUEST_TEMPLATE.md
🔇 Additional comments (62)
charts/vald/values.schema.json (3)
3720-3720: LGTM! Schema updates are consistent across all components.
The addition of "MetricInterceptor" to the gRPC client interceptors enum is consistently applied across all relevant sections of the schema.
Also applies to: 7056-7059, 7419-7419, 7771-7774, 9651-9654, 10002-10005, 10354-10357, 12240-12240, 14296-14299, 14647-14650, 15013-15013, 16708-16711, 17059-17062, 18760-18763, 19111-19114, 22491-22494, 22842-22845
3720-3720: Verify version compatibility for backport.
Since this is a backport to release/v1.7, we should ensure the MetricInterceptor feature is compatible with other v1.7 dependencies.
✅ Verification successful
MetricInterceptor addition is compatible with v1.7 release
Based on the verification results:
- The changes only add "MetricInterceptor" as an allowed value in the gRPC client interceptors enum
- The Chart version is v1.7.15, confirming this is within the v1.7.x release line
- The change is purely additive to the schema definition and doesn't modify existing functionality
- The MetricInterceptor feature is already part of the core functionality in v1.7, this just enables its configuration in values.schema.json
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check version compatibility in go.mod and related files
# Check go.mod for version constraints
echo "Checking go.mod versions..."
rg -t go "require|replace" go.mod
# Look for version-specific imports or constraints
echo "Checking for version-specific code..."
rg -t go "v1\.7\."
Length of output: 236
Script:
#!/bin/bash
# Let's check the changes in values.schema.json and related files
# Check the actual changes in values.schema.json
echo "Checking changes in values.schema.json..."
git diff HEAD^1 charts/vald/values.schema.json
# Look for MetricInterceptor related configurations
echo "Checking for MetricInterceptor configurations..."
rg -A 5 -B 5 "MetricInterceptor" charts/vald/values.schema.json
# Check Chart.yaml for version information
echo "Checking Chart.yaml version..."
cat charts/vald/Chart.yaml
# Check for any version constraints in related files
echo "Checking version constraints in related files..."
rg -g '*.yaml' -g '*.json' "version" charts/vald/
Length of output: 47032
3720-3720: Verify MetricInterceptor implementation.
Since this is a backport PR adding MetricInterceptor support, we should verify that the corresponding implementation is also included.
✅ Verification successful
Based on the search results, I can see that the MetricInterceptor implementation exists in the codebase:
-
The implementation is present in both client and server packages:
internal/net/grpc/interceptor/client/metric/metric.go(ClientMetricInterceptors)internal/net/grpc/interceptor/server/metric/metric.go(MetricInterceptors)
-
The interceptor is properly integrated into the server and client configurations:
- It's listed as a valid interceptor option in values schema
- It's referenced in various configuration files and CRDs
- The CHANGELOG mentions its implementation for continuous benchmark job (Implement client metrics interceptor for continuous benchmark job #2477)
-
The interceptor is properly registered and can be enabled through configuration:
- Server side:
internal/servers/server/option.gohandles "metricinterceptor" or "metric" case - Client side:
internal/net/grpc/option.gohandles "metricinterceptor" or "metric" case
- Server side:
The MetricInterceptor implementation is properly included and integrated in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify MetricInterceptor implementation exists and is properly imported
# Search for MetricInterceptor implementation
echo "Searching for MetricInterceptor implementation..."
rg -t go "type MetricInterceptor" -A 5
# Search for MetricInterceptor usage in interceptor registration
echo "Searching for MetricInterceptor registration..."
rg -t go "NewMetricInterceptor|RegisterMetricInterceptor" -A 5
Length of output: 319
Script:
#!/bin/bash
# Let's try a broader search for metric interceptor related code
# Search for any mentions of metric interceptor (case insensitive)
echo "Searching for metric interceptor mentions..."
rg -i "metric.*interceptor"
# Search for metric-related interceptor implementations
echo "Searching for metric-related implementations..."
rg -t go "type.*Interceptor.*metric|func.*Interceptor.*metric" -A 5
# Check for metric-related files in interceptor directories
echo "Searching for metric-related files in interceptor paths..."
fd -t f -i metric.*interceptor
# Check values schema for metric interceptor configuration
echo "Checking values schema for metric interceptor config..."
cat charts/vald/values.schema.json | grep -A 10 -i "metric.*interceptor"
Length of output: 40849
k8s/tools/benchmark/operator/crds/valdbenchmarkscenario.yaml (2)
69-72: LGTM: Type definitions and constraints are properly defined.
The type definitions and constraints for the dataset properties are correctly specified:
- String type for
groupandname - Integer type with minimum value of 0 for
indexes - Integer type with minimum value of 1 for range
startandend
Also applies to: 77-77, 83-83, 85-85, 89-89
97-97: LGTM: Object structure and type definitions are well-defined.
The schema structure is properly defined with:
- Object type for dataset with required fields
- Array type for jobs with proper object items
- Object type for target with required host and port fields
- Integer type with proper constraints for port (0-65535)
Also applies to: 102-102, 107-107, 111-111, 115-116
k8s/tools/benchmark/operator/crds/valdbenchmarkoperatorrelease.yaml (1)
704-712: LGTM: gRPC interceptor configuration is comprehensive.
The gRPC server interceptors are well-defined with a complete set of options:
- RecoverInterceptor for panic recovery
- AccessLogInterceptor for access logging
- TraceInterceptor for tracing
- MetricInterceptor for metrics collection
charts/vald-benchmark-operator/values.schema.json (2)
167-170: LGTM: Client interceptor configuration is properly defined.
The gRPC client interceptors schema is correctly defined with:
- TraceInterceptor for tracing
- MetricInterceptor for metrics collection
704-712: LGTM: Server interceptor configuration matches CRD definition.
The gRPC server interceptors schema matches the CRD definition with all required interceptors:
- RecoverInterceptor
- AccessLogInterceptor
- TraceInterceptor
- MetricInterceptor
k8s/tools/benchmark/operator/crds/valdbenchmarkjob.yaml (4)
Line range hint 169-212: Network configuration structure looks good!
The network configuration is well-structured with comprehensive options for DNS, socket options, and TLS settings.
Line range hint 316-350: Job configuration and validation parameters are well-defined!
The job types cover all necessary operations, and the RPS/replica configurations have appropriate boundary validations.
Line range hint 602-714: GRPC server configuration is comprehensive and well-structured!
The configuration includes all necessary settings for managing GRPC connections, streams, and interceptors.
Line range hint 714-786: REST server configuration is well-defined!
The configuration includes all necessary HTTP server settings and timeouts.
Makefile.d/dependencies.mk (1)
56-56: Ensure filtering logic meets expectations.
The filter removes lines that contain "upgrade", "latest", "master", or "main", which might drop lines you actually want if a version tag or dependency reference unexpectedly includes those words. For instance, some repositories might name their branches "master" or "main" in the semantic version. Confirm this won't exclude legitimate dependencies and consider a fallback or an explicit allowlist approach.
Would you like a script to scan for references to "upgrade", "latest", "master", or "main" in the rest of the codebase to ensure no needed dependencies are excluded?
hack/go.mod.default (4)
344-344: Controller-runtime update to v0.19.3.
This minor patch update usually provides bug fixes. Confirm that any version-specific code for sidecar injection or custom controllers is still compatible.
347-347: Structured-merge-diff upgraded to v4.5.0.
No immediate issues, but keep an eye out for new field merging behaviors if you handle CRDs.
16-16: Check the pinned version for cloud.google.com/go/storage.
Downgrading or pinning to an older version might help with the referenced issue, but be aware that any upstream bug fixes will not be present here. Verify whether the pinned version is still maintained and tested.
Run the following script to confirm no references require a newer version:
✅ Verification successful
The pinned version v1.47.0 of cloud.google.com/go/storage is appropriate and actively used.
The codebase shows consistent usage of cloud.google.com/go/storage v1.47.0 across:
- Direct import in
internal/db/storage/blob/cloudstorage/urlopener/urlopener.go - Pinned version in both
go.modandhack/go.mod.default - Matching go.sum entry confirming the version
The pinning is intentional to address a specific issue (googleapis/google-cloud-go#11283) and the version is relatively recent, being only a few minor versions behind the latest required version (v1.43.0) in the dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching references to storage usage
rg -A 5 "storage.NewClient"
Length of output: 27
Script:
#!/bin/bash
# Let's search for any imports or usage of cloud.google.com/go/storage
rg -A 3 "cloud\.google\.com/go/storage"
# Also check for any direct storage package imports
rg -A 3 '"storage"'
# Let's also check go.mod files for any requirements of this package
fd -e mod -x cat {}
Length of output: 40459
332-341: Kubernetes dependencies upgraded to v0.32.0.
These changes appear consistent. Check any code that depends on older Kubernetes APIs. The upgrade typically indicates new features, but also potential deprecations.
example/client/go.mod (3)
11-12: Dependency upgrades to golang.org/x/crypto and golang.org/x/net.
These minor updates can contain important security patches. Ensure you review any cryptographic or network-level changes to avoid regressions.
14-18: Google and gRPC dependencies bumped.
These new versions can introduce subtle breaking changes. Double-check any custom gRPC interceptors or usage of newly available features.
39-41: Google Genproto & Protobuf transitions.
No obvious issues with these indirect upgrades. Just confirm that your code generation scripts or proto messages are consistent with these versions.
go.mod (20)
6-16: Cloud library version updates and pinned storage library downgrade.
Confirm that none of the updated cloud libraries require a minimum version higher than the pinned storage v1.47.0.
Line range hint 18-39: bytefmt & cloudsql-proxy updated.
Check if the cloudsql-proxy version update changes the authentication or connection approach.
82-85: cncf/xds & go-md2man/v2 versions
These packages rarely break usage but verify that release notes don’t mention new environment or CLI flags that are now required.
134-134: Synced goccy/go-json to v0.10.3
Matching with hack/go.mod.default ensures consistency across modules.
211-211: Upgraded mailru/easyjson v0.9.0
Be aware that some new versions can introduce reflection-based changes that might affect performance. Consider running benchmarks if performance is critical.
278-288: OpenTelemetry dependencies updated to v1.33.0
This is a major step to keep instrumentation aligned, but confirm that your custom trace and metric exporters have not changed significantly.
[approve]
302-304: golang.org/x/mobile => v0.0.0-20241213221354-a87c1cf6cf46
Check if you rely on mobile or cross-compilation features. The main or master references may have changed in this new version.
317-324: google.golang.org/api and genproto updates
All appear consistent with your pinned storage library version. No immediate issues.
332-341: K8s dependencies to v0.32.0
Same comment as in hack/go.mod.default. Ensure your CRDs or controllers follow the new standard.
344-347: controller-runtime & structured-merge-diff
If you rely heavily on mutating webhooks or server-side apply, test thoroughly with this combination.
Line range hint 352-395: buf.build/gen & additional OTEL references
These unify your code generation pipeline and telemetry instrumentation. No immediate red flags.
410-413: Google gRPC bumped to v1.69.2
Potential changes to load balancing or service config defaults. Confirm existing interceptors are unaffected.
415-418: K8s updates
Consistent with other lines. Good to see the alignment across modules.
451-451: envoyproxy/go-control-plane v0.13.1
Should be compatible with protoc-gen-validate v1.1.0.
477-477: google/pprof => v0.0.0-20241210010833-40e02aabc2ad
Likely internal optimizations or bug fixes. Recommended to keep your profiling tools up to date.
484-484: gregjones/httpcache
No immediate issue. Mostly a maintenance update.
494-494: moby/spdystream
Infrequent updates. Check Docker or container runtime references for regressions.
518-519: OTEL auto instrumentation changes
If you do not use auto instrumentation, these are likely harmless. Otherwise, check for new config fields.
521-521: go.opentelemetry.io/proto/otlp => v1.4.0
Ensure your pipeline does not rely on an older version of OTLP messages.
539-542: kube-openapi & kustomize versions
Potential for CRD schema changes, especially if you generate openapi definitions. Double-check the kustomize patch syntax.
internal/core/algorithm/ngt/ngt_test.go (22)
2309-2310: No issues found with these lines.
They correctly add "vec" and "poolSize" parameters in the InsertCommit test case.
2325-2326: Looks good.
Defining "vec" with all zeroes and specifying the "poolSize" is consistent with the test pattern.
2345-2345: Works as intended.
Using the "poolSize" parameter for the min-int test case aligns with boundary testing.
2364-2364: No concerns here.
Testing with max-uint8 values and specifying "poolSize" looks consistent.
2379-2380: Clear addition of parameters.
Applying "poolSize" to float-based insertion tests maintains consistency.
2395-2396: All-zero float vector test is fine.
No issues with adding "poolSize" for this scenario.
2415-2415: Appropriate boundary coverage.
Including "poolSize" for the smallest float checks is coherent.
2434-2434: Max float boundary test looks good.
No issues found with adding the "poolSize" parameter here.
2449-2450: Dimension mismatch test updated correctly.
Including a separate "poolSize" parameter for this scenario is consistent.
2902-2902: Pool size parameter usage is fine.
Adding "poolSize" for BulkInsertCommit tests aligns with the updated signature.
2926-2926: No problems spotted.
The "poolSize" addition is coherent with bulk insertion testing.
2947-2947: Consistent usage of "poolSize".
This change follows the established pattern for BulkInsertCommit tests.
2970-2970: No issues here.
The block properly specifies "poolSize" in the test arguments.
2994-2994: Parameter addition looks correct.
All references to "poolSize" for float tests are well-structured.
3018-3018: Looks good.
Applying "poolSize" consistently for multiple vector insertion tests.
3039-3039: Checked thoroughly.
Nothing problematic about adding the "poolSize" argument here.
3062-3062: No objections.
The test case properly enumerates "poolSize" to finalize BulkInsertCommit.
3192-3192: Change aligns with added poolSize usage.
No functional issues identified for CreateAndSaveIndex.
3233-3233: Parameter addition is coherent.
Including "poolSize" in test fields ensures coverage of index creation logic.
3293-3293: No issues spotted.
Consistent with other lines specifying "poolSize" in index creation tests.
3339-3339: Checks out.
This newly added "poolSize" detail doesn't present any obvious concerns.
3455-3455: Confirmed correctness.
The updated code referencing "poolSize" is consistent with the rest of the test suite.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/v1.7 #2785 +/- ##
===============================================
Coverage ? 23.93%
===============================================
Files ? 546
Lines ? 54544
Branches ? 0
===============================================
Hits ? 13053
Misses ? 40704
Partials ? 787 ☔ View full report in Codecov by Sentry. |
Automated pull request to update Dependencies.
Summary by CodeRabbit
New Features
v1.32.0.Bug Fixes
KUBECTL_VERSIONandPROMETHEUS_STACK_VERSION.Chores