Feature cms api operator test#2464
Open
a932846905 wants to merge 20 commits into
Open
Conversation
added 3 commits
May 21, 2026 14:42
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR introduces the ConfigMapSet feature into Kruise, wiring it end-to-end from API types/CRD through controller reconciliation, admission webhooks (mutating + validating), generated client/informer support, and new e2e/unit tests.
Changes:
- Added the
ConfigMapSetAPI type and CRD, plus generated deepcopy/client/lister/informer code. - Added a new
configmapsetcontroller and integrated it into the controller manager. - Added ConfigMapSet admission webhooks and e2e tests validating basic behavior, partitioning, rollout injection behavior, and delete validation.
Reviewed changes
Copilot reviewed 23 out of 35 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/framework/v1alpha1/configmapset_util.go | Adds e2e helper utilities for creating/updating/deleting ConfigMapSets and Pods, and polling for status/annotations. |
| test/e2e/apps/v1alpha1/configmapset.go | Adds e2e coverage for create/update behavior, partition handling, rollout-time injection behavior, and delete validation. |
| pkg/webhook/pod/mutating/pod_create_update_handler.go | Extends Pod mutating webhook to inject ConfigMapSet volumes/sidecar and annotate revision/customVersion metadata. |
| pkg/webhook/pod/mutating/configmapset_mutating_test.go | Adds unit tests for ConfigMapSet-related pod mutation helpers (conflict detection, revision annotations, sidecar name resolution). |
| pkg/webhook/configmapset/validating/webhooks.go | Registers the validating webhook endpoint for ConfigMapSet resources. |
| pkg/webhook/configmapset/validating/configmapset_create_update_handler.go | Implements ConfigMapSet validation (selector/strategy/effectPolicy/reloadSidecarConfig/history-limit/delete checks). |
| pkg/webhook/configmapset/validating/configmapset_create_update_handler_test.go | Adds unit tests for ConfigMapSet validation helpers and delete validation. |
| pkg/webhook/configmapset/validating/configmapset_conflicts.go | Adds validation-time conflict checking against existing ConfigMapSets with overlapping selectors. |
| pkg/webhook/configmapset/mutating/webhooks.go | Registers the mutating webhook endpoint for ConfigMapSet resources. |
| pkg/webhook/configmapset/mutating/configmapset_create_update_handler.go | Implements defaulting mutation for ConfigMapSet resources. |
| pkg/webhook/add_configmapset.go | Hooks ConfigMapSet webhooks into the webhook handler registry. |
| pkg/controller/controllers.go | Registers the new ConfigMapSet controller with the manager. |
| pkg/controller/configmapset/configmapset_utils.go | Adds ConfigMapSet utility functions (annotation keys, hashing helpers, pod/cms matching, grouping). |
| pkg/controller/configmapset/configmapset_utils_test.go | Adds unit tests for ConfigMapSet utility functions. |
| pkg/controller/configmapset/configmapset_event_handler.go | Adds event handler implementations intended to enqueue reconcile requests on CMS/Pod events. |
| pkg/controller/configmapset/configmapset_controller.go | Introduces the main ConfigMapSet controller reconciliation logic (revision hub configmap, distribution, rolling update behavior, status, CRR handling). |
| pkg/controller/configmapset/configmapset_controller_test.go | Adds unit tests for hashing, partition->distribution, pod selection ordering, and container-name resolution. |
| pkg/client/listers/apps/v1alpha1/expansion_generated.go | Extends lister expansion interfaces to include ConfigMapSet. |
| pkg/client/listers/apps/v1alpha1/configmapset.go | Adds generated lister for ConfigMapSet resources. |
| pkg/client/informers/externalversions/generic.go | Adds ConfigMapSet to the generic informer factory resource switch. |
| pkg/client/informers/externalversions/apps/v1alpha1/interface.go | Adds ConfigMapSet informer accessor to the apps/v1alpha1 informer interface. |
| pkg/client/informers/externalversions/apps/v1alpha1/configmapset.go | Adds generated shared informer for ConfigMapSet resources. |
| pkg/client/clientset/versioned/typed/apps/v1alpha1/generated_expansion.go | Adds typed client expansion interface for ConfigMapSet. |
| pkg/client/clientset/versioned/typed/apps/v1alpha1/fake/fake_configmapset.go | Adds generated fake typed client for ConfigMapSet. |
| pkg/client/clientset/versioned/typed/apps/v1alpha1/fake/fake_apps_client.go | Wires fake AppsV1alpha1 client to serve fake ConfigMapSets. |
| pkg/client/clientset/versioned/typed/apps/v1alpha1/configmapset.go | Adds generated typed client implementation for ConfigMapSet. |
| pkg/client/clientset/versioned/typed/apps/v1alpha1/apps_client.go | Wires real AppsV1alpha1 client to serve ConfigMapSets. |
| go.mod | Adds indirect dependencies needed by new/updated test tooling. |
| go.sum | Updates sums to match go.mod dependency changes. |
| config/webhook/manifests.yaml | Adds webhook configuration entries for ConfigMapSet mutating/validating endpoints. |
| config/rbac/role.yaml | Grants RBAC permissions for configmapsets and configmapsets/status. |
| config/crd/bases/apps.kruise.io_configmapsets.yaml | Adds the ConfigMapSet CRD definition. |
| apis/apps/v1alpha1/zz_generated.deepcopy.go | Adds generated deep-copy methods for new ConfigMapSet API types. |
| apis/apps/v1alpha1/configmapset_types.go | Adds the ConfigMapSet API types, spec/status, and related config structs/enums. |
| apis/apps/defaults/v1alpha1.go | Adds ConfigMapSet defaulting logic (partition/maxUnavailable/history limit). |
Files not reviewed (11)
- apis/apps/v1alpha1/zz_generated.deepcopy.go: Language not supported
- pkg/client/clientset/versioned/typed/apps/v1alpha1/apps_client.go: Language not supported
- pkg/client/clientset/versioned/typed/apps/v1alpha1/configmapset.go: Language not supported
- pkg/client/clientset/versioned/typed/apps/v1alpha1/fake/fake_apps_client.go: Language not supported
- pkg/client/clientset/versioned/typed/apps/v1alpha1/fake/fake_configmapset.go: Language not supported
- pkg/client/clientset/versioned/typed/apps/v1alpha1/generated_expansion.go: Language not supported
- pkg/client/informers/externalversions/apps/v1alpha1/configmapset.go: Language not supported
- pkg/client/informers/externalversions/apps/v1alpha1/interface.go: Language not supported
- pkg/client/informers/externalversions/generic.go: Language not supported
- pkg/client/listers/apps/v1alpha1/configmapset.go: Language not supported
- pkg/client/listers/apps/v1alpha1/expansion_generated.go: Language not supported
Comments suppressed due to low confidence (1)
pkg/controller/configmapset/configmapset_controller.go:1217
- Same md5.New().Sum misuse here as in rebootSidecarsByCrr: this does not compute a digest of the input. This can make the CRR lookup name inconsistent with the name used at creation, breaking the "wait" logic. Compute the digest via md5.Sum or hash.Write()+Sum(nil).
// Combine hashStr and sortedContainerNames to form a unique hash for this specific set of containers and targetRevision
combinedHashStr := hex.EncodeToString(md5.New().Sum([]byte(hashStr + "-" + strings.Join(sortedContainerNames, ","))))[:10]
crrName := fmt.Sprintf("%s-%s", pod.Name, combinedHashStr)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+180
to
+189
| func SetDefaultsConfigMapSet(obj *v1alpha1.ConfigMapSet) { | ||
| partitionValue := int32(0) | ||
| maxUnavailableValue := intstr.FromInt32(1) | ||
| if obj.Spec.UpdateStrategy.Partition == nil { | ||
| v := intstr.FromInt32(partitionValue) | ||
| obj.Spec.UpdateStrategy.Partition = &v | ||
| } | ||
| if obj.Spec.UpdateStrategy.MaxUnavailable == nil { | ||
| obj.Spec.UpdateStrategy.MaxUnavailable = &maxUnavailableValue | ||
| } |
Comment on lines
+81
to
+87
| if len(spec.UpdateStrategy.MatchLabelKeys) > 0 { | ||
| for _, key := range spec.UpdateStrategy.MatchLabelKeys { | ||
| if _, exists := spec.Selector.MatchLabels[key]; exists { | ||
| return field.Invalid(fldPath.Child("updateStrategy", "matchLabelKeys"), spec.UpdateStrategy.MatchLabelKeys, fmt.Sprintf("matchLabelKeys cannot intersect with selector matchLabels, conflict key: %s", key)) | ||
| } | ||
| } | ||
| } |
Comment on lines
+236
to
+249
| if [ -f /etc/config/.current_revision ]; then | ||
| CURRENT_REVISION=$(cat /etc/config/.current_revision) | ||
| if [ "$TARGET_REVISION" != "$CURRENT_REVISION" ]; then | ||
| exit 1 | ||
| fi | ||
| else | ||
| exit 1 | ||
| fi | ||
|
|
||
| # 2. If PostHook is configured, check for success marker | ||
| POST_HOOK_CONFIG=$(cat /etc/cms_config/post_hook_config 2>/dev/null || true) | ||
| if [ -n "$POST_HOOK_CONFIG" ] && [ "$POST_HOOK_CONFIG" != "null" ]; then | ||
| if [ ! -f "/etc/config/.post_hook_success_${TARGET_REVISION}" ]; then | ||
| exit 1 |
Comment on lines
+308
to
+313
| } else if cms.Spec.ReloadSidecarConfig.Type == appsv1alpha1.ReloadSidecarTypeSidecarSet { | ||
| klog.Infof("pod %s/%s will be injected by SidecarSet, skip full sidecar injection in ConfigMapSet webhook, just merge VolumeMounts and Env", pod.Namespace, pod.Name) | ||
|
|
||
| // find reload-sidecar in Pod (because already injected by sidecarSet) | ||
| targetSidecarName := cms.Spec.ReloadSidecarConfig.Config.SidecarSetRef.ContainerName | ||
| var container *corev1.Container |
Comment on lines
+626
to
+640
| // Strategy 1: Prioritize using CurrentRevision. If empty, it means this is the first release, so use the target version calculated from Data. | ||
| targetVersion := cms.Status.CurrentRevision | ||
| if targetVersion == "" { | ||
| hash, err := configmapset.CalculateHash(cms.Spec.Data) | ||
| if err == nil { | ||
| targetVersion = hash | ||
| } | ||
| } | ||
|
|
||
| pod.Annotations[targetRevisionKey] = targetVersion | ||
| pod.Annotations[currentRevisionKey] = targetVersion | ||
| pod.Annotations[targetCustomVersionKey] = cms.Status.CurrentCustomVersion | ||
| pod.Annotations[currentCustomVersionKey] = cms.Status.CurrentCustomVersion | ||
| pod.Annotations[currentRevisionTimestampKey] = now | ||
| pod.Annotations[updateRevisionTimestampKey] = now |
Comment on lines
+63
to
+70
| func (h *ConfigMapSetCreateUpdateHandler) getReloadSidecarName(ctx context.Context, cms *appsv1alpha1.ConfigMapSet) (string, error) { | ||
| if cms.Spec.ReloadSidecarConfig == nil { | ||
| return "", nil | ||
| } | ||
| config := cms.Spec.ReloadSidecarConfig.Config | ||
| if config == nil { | ||
| return "", nil | ||
| } |
Comment on lines
+339
to
+366
| // Verify if maxUnavailable is a percentage between 1~100, or a positive integer (>0) | ||
| func validateMaxUnavailable(maxUnavailable *intstr.IntOrString) bool { | ||
| if maxUnavailable == nil { | ||
| return false | ||
| } | ||
|
|
||
| switch maxUnavailable.Type { | ||
| case intstr.Int: | ||
| if maxUnavailable.IntVal > 0 { | ||
| return true | ||
| } | ||
| case intstr.String: | ||
| s := strings.TrimSpace(maxUnavailable.StrVal) | ||
| if strings.HasSuffix(s, "%") { | ||
| numStr := strings.TrimSuffix(s, "%") | ||
| num, err := strconv.Atoi(numStr) | ||
| if err != nil || num < 0 || num > 100 { | ||
| return false | ||
| } | ||
| return true | ||
| } | ||
| // Non-percentage case, parse as integer | ||
| num, err := strconv.Atoi(s) | ||
| if err != nil || num < 0 { | ||
| return false | ||
| } | ||
| return true | ||
| } |
Comment on lines
+353
to
+355
| // Wait for pod to be running | ||
| tester.WaitForPodAnnotation(namespace, podName, configmapset.GetConfigMapSetCurrentRevisionKey(cmsName), "") // just wait for it to be processed | ||
|
|
Comment on lines
+50
to
+57
| pods, err := getMatchedPods(e.Reader, cms) | ||
| if err != nil { | ||
| klog.Errorf("Failed to get Pods for appsv1alpha1.ConfigMapSet %s: %v", cms.Name, err) | ||
| return | ||
| } | ||
| for _, pod := range pods.Items { | ||
| q.Add(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: pod.Namespace, Name: pod.Name}}) | ||
| } |
Comment on lines
+99
to
+103
| // Calculate Hash of current Spec.Data | ||
| hash, err := configmapset.CalculateHash(spec.Data) | ||
| if err != nil { | ||
| return field.InternalError(fldPath.Child("data"), fmt.Errorf("failed to compute hash: %v", err)) | ||
| } |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2464 +/- ##
==========================================
- Coverage 48.77% 47.65% -1.13%
==========================================
Files 324 334 +10
Lines 27928 29701 +1773
==========================================
+ Hits 13623 14154 +531
- Misses 12775 13939 +1164
- Partials 1530 1608 +78
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:
|
added 11 commits
May 21, 2026 17:27
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
|
@a932846905: PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
added 5 commits
June 5, 2026 19:00
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
Signed-off-by: chrisdeng <chrisdeng@futunn.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ⅰ. Describe what this PR does
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how to verify it
Ⅳ. Special notes for reviews