Skip to content

using configmap to inject runtime and csi sideacr#219

Merged
furykerry merged 1 commit intoopenkruise:masterfrom
BH4AWS:feat/auto_inject_csi_runtime_config
Mar 27, 2026
Merged

using configmap to inject runtime and csi sideacr#219
furykerry merged 1 commit intoopenkruise:masterfrom
BH4AWS:feat/auto_inject_csi_runtime_config

Conversation

@BH4AWS
Copy link
Copy Markdown
Contributor

@BH4AWS BH4AWS commented Mar 24, 2026

Ⅰ. Describe what this PR does

The purpose of this PR is to use ConfigMap as the data source to automatically inject CSI mount or agent runtime configurations into the sandbox.

// ShouldInjectCsiMount is the annotation key for inject csi mount plugin container.
// If set, the csi sidecar will be injected into the pod when the sandbox is created.
// The csi mount sidecar is used to mount the remote oss/nas storage to the sandbox container.
ShouldInjectCsiMount = "agents.kruise.io/should-inject-csi-mount-plugin"

// ShouldInjectAgentRuntime is the annotation key for inject agent runtime sidecar in init container.
// If set, the agent runtime sidecar will be injected into the pod when the sandbox is created.
// Some binary tools which are contained in the init agent runtime container. These are the basic tools for sandbox running.
ShouldInjectAgentRuntime = "agents.kruise.io/should-inject-agent-runtime"

TODO:
This is the first patch. In next, for the second path, i will modify the code in sandbox-manager.

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how to verify it

Ⅳ. Special notes for reviews

@BH4AWS BH4AWS force-pushed the feat/auto_inject_csi_runtime_config branch from 3cf2c3c to 8cb8853 Compare March 24, 2026 14:27
Comment on lines +55 to +153
// parseCSIMountConfig parses the CSI sidecar injection configuration from raw ConfigMap data.
// It extracts the value associated with KEY_CSI_INJECTION_CONFIG key and unmarshals it into
// a SidecarInjectConfig struct. If the key is not present in the configRaw map, it returns
// an empty SidecarInjectConfig without error. The configuration includes the main container
// settings, CSI sidecar containers list, and volumes to be injected.
//
// Parameters:
// - ctx: context.Context for the operation (currently unused but kept for future extensibility)
// - configRaw: map[string]string containing the raw ConfigMap data with potential CSI config
//
// Returns:
// - SidecarInjectConfig: The parsed configuration containing main container, sidecars, and volumes
// - error: An error if JSON unmarshaling fails, nil otherwise
func parseCSIMountConfig(ctx context.Context, configRaw map[string]string) (SidecarInjectConfig, error) {
log := logf.FromContext(ctx)
csiMountConfig := SidecarInjectConfig{}

configValue, exists := configRaw[KEY_CSI_INJECTION_CONFIG]
if !exists || configValue == "" {
log.Info("csi container config not found or empty, using default configuration")
return csiMountConfig, nil
}

err := json.Unmarshal([]byte(configRaw[KEY_CSI_INJECTION_CONFIG]), &csiMountConfig)
if err != nil {
log.Error(err, "failed to json unmarshal csi mount config")
return csiMountConfig, err
}
return csiMountConfig, nil
}

// parseAgentRuntimeConfig parses the agent runtime injection configuration from raw ConfigMap data.
// It extracts the value associated with KEY_RUNTIME_INJECTION_CONFIG key and unmarshals it into
// a SidecarInjectConfig struct. If the key is not present in the configRaw map, it returns
// an empty SidecarInjectConfig without error. The configuration includes the main container
// settings with lifecycle hooks, init runtime containers list, and volumes to be injected.
//
// Parameters:
// - ctx: context.Context for the operation (currently unused but kept for future extensibility)
// - configRaw: map[string]string containing the raw ConfigMap data with potential runtime config
//
// Returns:
// - SidecarInjectConfig: The parsed configuration containing main container, sidecars (init containers), and volumes
// - error: An error if JSON unmarshaling fails, nil otherwise
func parseAgentRuntimeConfig(ctx context.Context, configRaw map[string]string) (SidecarInjectConfig, error) {
log := logf.FromContext(ctx)
csiMountConfig := SidecarInjectConfig{}

configValue, exists := configRaw[KEY_RUNTIME_INJECTION_CONFIG]
if !exists || configValue == "" {
log.Info("agent runtime config not found or empty, using default configuration")
return csiMountConfig, nil
}

err := json.Unmarshal([]byte(configValue), &csiMountConfig)
if err != nil {
log.Error(err, "failed to json unmarshal agent runtime config")
return csiMountConfig, err
}
return csiMountConfig, nil
}

// setCSIMountContainer injects CSI mount configurations into the SandboxTemplate's pod spec.
// It configures the main container (first container in the spec) with CSI sidecar settings,
// appends additional CSI sidecar containers, and mounts shared volumes.
// Volumes are only added if they don't already exist in the template.
func setCSIMountContainer(ctx context.Context, obj *corev1.PodTemplateSpec, config SidecarInjectConfig) {
log := logf.FromContext(ctx)

// set main container, the first container is the main container
if len(obj.Spec.Containers) == 0 {
log.Info("no found the template containers")
return
}

mainContainer := &obj.Spec.Containers[0]
setMainContainerWhenInjectCSISidecar(mainContainer, config)

// set csi sidecars
for _, csiSidecar := range config.Sidecars {
obj.Spec.Containers = append(obj.Spec.Containers, csiSidecar)
}

// set share volume
if len(config.Volumes) > 0 {
if obj.Spec.Volumes == nil {
obj.Spec.Volumes = make([]corev1.Volume, 0, len(config.Volumes))
}
for _, vol := range config.Volumes {
if findVolumeByName(obj.Spec.Volumes, vol.Name) {
continue
}
obj.Spec.Volumes = append(obj.Spec.Volumes, vol)
}
}
}

// setMainContainerWhenInjectCSISidecar configures the main container with environment variables and volume mounts from the CSI sidecar configuration.
// It appends environment variables and volume mounts to the main container, skipping any that already exist (matched by name) to avoid duplicates.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

plz merge two parse function into one to avoid code duplication

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

parseCSIMountConfig and parseAgentRuntimeConfig ---> parseInjectConfig function

obj.Spec.Volumes = append(obj.Spec.Volumes, config.Volumes...)
}

func setMainContainerConfigWhenInjectRuntimeSidecar(mainContainer *corev1.Container, config SidecarInjectConfig) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

plz merge two set function to avoid code duplication

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The function is implemented as a sub-function and is not exposed for external calls; thus, it remains in its current implementation solely to simplify the implementation of the main function.

mainContainer.VolumeMounts = make([]corev1.VolumeMount, 0, len(config.MainContainer.VolumeMounts))
}
for _, volMount := range config.MainContainer.VolumeMounts {
if findVolumeMountByName(mainContainer.VolumeMounts, volMount.Name) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

also compare the mount path?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we will modify the implementation function in the next patch.

@BH4AWS BH4AWS force-pushed the feat/auto_inject_csi_runtime_config branch from 8cb8853 to 521d1d7 Compare March 25, 2026 07:33
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 25, 2026

Codecov Report

❌ Patch coverage is 92.06349% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.47%. Comparing base (4d5825c) to head (5e6ce43).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
pkg/controller/sandbox/core/util.go 55.00% 6 Missing and 3 partials ⚠️
...g/controller/sandbox/core/sidecar_config_inject.go 99.04% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #219      +/-   ##
==========================================
+ Coverage   61.80%   62.47%   +0.67%     
==========================================
  Files         104      106       +2     
  Lines        6694     6827     +133     
==========================================
+ Hits         4137     4265     +128     
- Misses       2290     2294       +4     
- Partials      267      268       +1     
Flag Coverage Δ
unittests 62.47% <92.06%> (+0.67%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@BH4AWS BH4AWS force-pushed the feat/auto_inject_csi_runtime_config branch from 521d1d7 to 54481a5 Compare March 25, 2026 07:57
// ShouldInjectCsiMount is the annotation key for inject csi mount plugin container.
// If set, the csi sidecar will be injected into the pod when the sandbox is created.
// The csi mount sidecar is used to mount the remote oss/nas storage to the sandbox container.
ShouldInjectCsiMount = "agents.kruise.io/should-inject-csi-mount-plugin"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

agents.kruise.io/should-inject-csi-mount-plugin -> agents.kruise.io/inject-csi-plugin

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

// ShouldInjectAgentRuntime is the annotation key for inject agent runtime sidecar in init container.
// If set, the agent runtime sidecar will be injected into the pod when the sandbox is created.
// Some binary tools which are contained in the init agent runtime container. These are the basic tools for sandbox running.
ShouldInjectAgentRuntime = "agents.kruise.io/should-inject-agent-runtime"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

agents.kruise.io/should-inject-agent-runtime -> agents.kruise.io/inject-agent-runtime

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

}, config)
if err != nil {
if errors.IsNotFound(err) {
logger.Info("no found the injection configuration, using default")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no found the injection configuration, using default -> injection configuration not found, skip injection

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

// - error: An error if JSON unmarshaling fails, nil otherwise or when config key is missing/empty
func parseInjectConfig(ctx context.Context, configKey string, configRaw map[string]string) (SidecarInjectConfig, error) {
log := logf.FromContext(ctx)
csiMountConfig := SidecarInjectConfig{}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

csiMountConfig -> sidecarConfig

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done


err := json.Unmarshal([]byte(configRaw[configKey]), &csiMountConfig)
if err != nil {
log.Error(err, "failed to json unmarshal csi mount config")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

failed to unmarshal sidecar config for
log.Errorf(err, "failed to unmarshal sidecar config for the %v", configKey)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done


// set main container, the first container is the main container
if len(obj.Spec.Containers) == 0 {
log.Info("no found the template containers")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

	log.Info("no container found in sidecar template")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

}

mainContainer := &obj.Spec.Containers[0]
setMainContainerWhenInjectCSISidecar(mainContainer, config)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should csi sidecar should also be a native sidecar ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TODO?

obj.Spec.InitContainers = append(obj.Spec.InitContainers, config.Sidecars...)

if len(obj.Spec.Containers) == 0 {
log.Info("no found the template container, default main container is the first container")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

log.Info("no container found in sidecar template for agent runtime")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

// setAgentRuntimeContainer injects agent runtime configurations into the SandboxTemplate's pod spec.
// It appends agent runtime containers as init containers and configures the main container (first container) with runtime settings.
// The init containers run before the main containers to prepare the runtime environment.
func setAgentRuntimeContainer(ctx context.Context, obj *corev1.PodTemplateSpec, config SidecarInjectConfig) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

plz rename obj to template

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done


// using config to override the main container lifecycle post start
if config.MainContainer.Lifecycle != nil && config.MainContainer.Lifecycle.PostStart != nil {
mainContainer.Lifecycle.PostStart = config.MainContainer.Lifecycle.PostStart
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what if main container already have a post starthook, override the poststarthook may cause agent runs in correctly. Instead of silently overside postarthook, we should at least log an error and abort the injection

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

DONE

Signed-off-by: jicheng.sk <jicheng.sk@alibaba-inc.com>
@BH4AWS BH4AWS force-pushed the feat/auto_inject_csi_runtime_config branch from 54481a5 to 5e6ce43 Compare March 27, 2026 02:45
Copy link
Copy Markdown
Member

@furykerry furykerry left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@kruise-bot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: furykerry

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@furykerry furykerry merged commit 0566fe6 into openkruise:master Mar 27, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants