-
Notifications
You must be signed in to change notification settings - Fork 826
feat(configmapset): enables zero-downtime configuration updates #2149
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
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @Placeboy! It looks like this is your first PR to openkruise/kruise 🎉 |
31eac8f
to
14c9794
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2149 +/- ##
==========================================
- Coverage 44.82% 42.50% -2.33%
==========================================
Files 318 326 +8
Lines 32245 34016 +1771
==========================================
+ Hits 14455 14457 +2
- Misses 16370 18139 +1769
Partials 1420 1420
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
// +optional | ||
Partition *intstr.IntOrString `json:"partition,omitempty"` // 旧版本的比例, 多个旧版本的情况不做区分 | ||
// +optional | ||
RestartInjectedContainers bool `json:"restartInjectedContainers,omitempty"` // 开启该选项后, 被注入最新配置的容器发生重启 |
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.
plz consider extract the RestartInjectedContainers field to a separate field in the ConfigMapSetSpec, maybe called "LoadStrategy", with three possible option
- RestartAppContainers
- RestartReloaderContainer
- None
@@ -130,3 +167,324 @@ func (h *PodCreateHandler) Handle(ctx context.Context, req admission.Request) ad | |||
} | |||
return admission.PatchResponseFromRaw(original, marshaled) | |||
} | |||
|
|||
func (h *PodCreateHandler) injectSidecar4Pod(ctx context.Context, pod *corev1.Pod, cms *appsv1alpha1.ConfigMapSet, revisionKeys *configmapset.RevisionKeys) error { |
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.
consider move the pod mutating logic of comfigmapset into a sperate file e.g. configmapset.go
// 创建sidecar容器 | ||
newSidecar := corev1.Container{ | ||
Name: sidecarName, | ||
Image: "hub.bilibili.co/infra-caster/kratos-demo-wuhao18:reload-sidecar", |
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.
how to config the image and possible container configuration such as poststarthook? Maybe we can add another CRD called ReloaderClass.
Ⅰ. Describe what this PR does
This PR implements ConfigMapSet, a sidecar-based multi-version configuration manager that decouples configuration updates from image updates.
For instances of the same workload, while image versions are managed by CloneSet, configuration versions can be managed independently via ConfigMapSet.
This feature is enabled by injecting a sidecar container into each Pod. When the configuration version changes, the sidecar container restarts and rewrites the corresponding configuration version to the mounted path.
Ⅱ. Does this pull request fix one issue?
NONE
Ⅲ. Describe how to verify it
Ⅳ. Special notes for reviews
N/A