feat(injector): add support for native sidecar injection #5295
Conversation
- Implement native sidecar injection mode for Fuse - Add FUSE_SIDECAR_INJECTION_MODE environment variable - Update webhook to set sidecar injection mode - Modify injector to use the new sidecar injection mode - Implement default and unprivileged mutators for native sidecar injection Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #5295 +/- ##
==========================================
- Coverage 56.96% 56.96% -0.01%
==========================================
Files 442 442
Lines 30476 30529 +53
==========================================
+ Hits 17361 17390 +29
- Misses 11504 11523 +19
- Partials 1611 1616 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
|
/test fluid-e2e |
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
|
/test fluid-e2e |
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
|
/test fluid-e2e |
|
/retest |
1 similar comment
|
/retest |
- Display an error message if the Kubernetes version is less than 1.28.0-0 Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
|
@TrafalgarZZZ Please fix unit test. Thanks. |
There was a problem hiding this comment.
Pull Request Overview
This PR implements native sidecar injection mode support for Fuse containers in Fluid, allowing Fuse sidecars to be deployed as Kubernetes native sidecar containers (init containers with restart policy) instead of regular containers.
Key changes:
- Adds
FUSE_SIDECAR_INJECTION_MODEenvironment variable with support for "default", "legacy", and "native-sidecar" modes - Implements native sidecar injection logic that places Fuse containers in
initContainers[]withRestartPolicyAlways - Updates both default and unprivileged mutators to conditionally use native sidecar injection based on the configured mode
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/java | Removes Java SDK subproject commit reference |
| pkg/common/types.go | Defines SidecarInjectionMode type and GetSidecarInjectionMode() function to read injection mode from environment |
| pkg/common/env_names.go | Declares EnvFuseSidecarInjectionMode constant for the new environment variable |
| pkg/application/inject/fuse/mutator/mutator_unprivileged.go | Updates unprivileged mutator to use native sidecar injection when mode is set |
| pkg/application/inject/fuse/mutator/mutator_default_test.go | Adds test cases for native sidecar injection scenarios |
| pkg/application/inject/fuse/mutator/mutator_default.go | Implements defaultInjectFuseNativeSidecar() and prependFuseNativeSidecar() functions for native sidecar injection |
| pkg/application/inject/fuse/injector.go | Initializes injector with sidecar injection mode and passes it to mutator options |
| charts/fluid/fluid/values.yaml | Adds fuseSidecar.sidecarInjectionMode configuration with documentation |
| charts/fluid/fluid/templates/webhook/webhook.yaml | Sets FUSE_SIDECAR_INJECTION_MODE environment variable in webhook deployment with Kubernetes version validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- fix unit tests - fix typos Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
|
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if helper.Specs.MetaObj.Labels == nil { | ||
| helper.Specs.MetaObj.Labels = map[string]string{} | ||
| } | ||
| helper.Specs.MetaObj.Labels[containerDatasetMappingLabelKey] = fmt.Sprintf("%s_%s", helper.runtimeInfo.GetNamespace(), helper.runtimeInfo.GetName()) |
There was a problem hiding this comment.
The label value format has changed from using utils.GetDatasetId() (which includes the dataset UID) to a simple namespace_name format. This removes the UID from the label, which could cause issues if datasets with the same name exist in the same namespace but are recreated with different UIDs. Consider keeping the UID in the label value or documenting why it was removed.
| if helper.Specs.MetaObj.Labels == nil { | ||
| helper.Specs.MetaObj.Labels = map[string]string{} | ||
| } | ||
| helper.Specs.MetaObj.Labels[containerDatasetMappingLabelKey] = fmt.Sprintf("%s_%s", helper.runtimeInfo.GetNamespace(), helper.runtimeInfo.GetName()) |
There was a problem hiding this comment.
The label value format has changed from using utils.GetDatasetId() (which includes the dataset UID) to a simple namespace_name format. This removes the UID from the label, which could cause issues if datasets with the same name exist in the same namespace but are recreated with different UIDs. Consider keeping the UID in the label value or documenting why it was removed.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheyang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Ⅰ. Describe what this PR does
Ⅱ. Does this pull request fix one issue?
fixes #5294
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews