diff --git a/apis/apps/v1alpha1/poddecoration_types.go b/apis/apps/v1alpha1/poddecoration_types.go index a5bcd749..cdaf6b08 100644 --- a/apis/apps/v1alpha1/poddecoration_types.go +++ b/apis/apps/v1alpha1/poddecoration_types.go @@ -50,7 +50,7 @@ type PodDecorationPodTemplate struct { Metadata []*PodDecorationPodTemplateMeta `json:"metadata,omitempty"` // InitContainers is the init containers needs to be attached to a pod. - // If there is a container with the same name, PodDecoration will override it entirely. + // If there is a container with the same name, PodDecoration will retain old Container. InitContainers []*corev1.Container `json:"initContainers,omitempty"` // Containers is the containers need to be attached to a pod. @@ -63,6 +63,7 @@ type PodDecorationPodTemplate struct { PrimaryContainers []*PrimaryContainerPatch `json:"primaryContainers,omitempty"` // Volumes will be attached to a pod spec volume. + // If there is a volume with the same name, new volume will replace it. Volumes []corev1.Volume `json:"volumes,omitempty"` // If specified, the pod's scheduling constraints @@ -152,7 +153,6 @@ type PodDecorationInjectStrategy struct { // Group provides the name of the group this PodDecoration belongs to. // Only one PodDecoration is active when multiple PodDecorations share the same group value. Group string `json:"group,omitempty"` - // Weight indicates the priority to apply for a group of PodDecorations with same group value. // The greater one has higher priority to apply. // Default value is 0. diff --git a/apis/apps/v1alpha1/resourcecontext_types.go b/apis/apps/v1alpha1/resourcecontext_types.go index aad8a135..28c5cb6f 100644 --- a/apis/apps/v1alpha1/resourcecontext_types.go +++ b/apis/apps/v1alpha1/resourcecontext_types.go @@ -74,6 +74,15 @@ func (cd *ContextDetail) Put(key, value string) { cd.Data[key] = value } +// Get is used to get the specified key from Data. +func (cd *ContextDetail) Get(key string) (string, bool) { + if cd.Data == nil { + return "", false + } + val, ok := cd.Data[key] + return val, ok +} + // Remove is used to remove the specified key from Data . func (cd *ContextDetail) Remove(key string) { if cd.Data == nil { diff --git a/config/crd/bases/apps.kusionstack.io_poddecorations.yaml b/config/crd/bases/apps.kusionstack.io_poddecorations.yaml index edc14b56..873ff11b 100644 --- a/config/crd/bases/apps.kusionstack.io_poddecorations.yaml +++ b/config/crd/bases/apps.kusionstack.io_poddecorations.yaml @@ -2336,7 +2336,7 @@ spec: initContainers: description: InitContainers is the init containers needs to be attached to a pod. If there is a container with the same name, - PodDecoration will override it entirely. + PodDecoration will retain old Container. items: description: A single application container that you want to run within a pod. @@ -3782,7 +3782,9 @@ spec: type: object type: array volumes: - description: Volumes will be attached to a pod spec volume. + description: Volumes will be attached to a pod spec volume. If + there is a volume with the same name, new volume will replace + it. items: description: Volume represents a named volume in a pod that may be accessed by any container in the pod. diff --git a/pkg/controllers/collaset/collaset_controller.go b/pkg/controllers/collaset/collaset_controller.go index 7d8bc83c..20464cdc 100644 --- a/pkg/controllers/collaset/collaset_controller.go +++ b/pkg/controllers/collaset/collaset_controller.go @@ -172,11 +172,11 @@ func (r *CollaSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c UpdatedRevision: updatedRevision, NewStatus: newStatus, } - resources.PodDecorations, resources.OldRevisionDecorations, err = utils.GetEffectiveDecorationsByCollaSet(ctx, r.Client, instance) + resources.PDGetter, err = utils.NewPodDecorationGetter(ctx, r.Client, instance.Namespace) if err != nil { return ctrl.Result{}, fmt.Errorf("fail to get effective pod decorations by CollaSet %s: %s", key, err) } - for _, pd := range resources.PodDecorations { + for _, pd := range resources.PDGetter.GetLatestDecorations() { if pd.Status.ObservedGeneration != pd.Generation { logger.Info("wait for PodDecoration ObservedGeneration", "CollaSet", key, "PodDecoration", commonutils.ObjectKeyString(pd)) return ctrl.Result{}, nil diff --git a/pkg/controllers/collaset/podcontext/podcontext.go b/pkg/controllers/collaset/podcontext/podcontext.go index dc638754..d7e896a1 100644 --- a/pkg/controllers/collaset/podcontext/podcontext.go +++ b/pkg/controllers/collaset/podcontext/podcontext.go @@ -32,8 +32,9 @@ import ( ) const ( - OwnerContextKey = "Owner" - RevisionContextDataKey = "Revision" + OwnerContextKey = "Owner" + RevisionContextDataKey = "Revision" + PodDecorationRevisionKey = "PodDecorationRevisions" ) func AllocateID(c client.Client, instance *appsv1alpha1.CollaSet, defaultRevision string, replicas int) (map[int]*appsv1alpha1.ContextDetail, error) { diff --git a/pkg/controllers/collaset/synccontrol/sync_control.go b/pkg/controllers/collaset/synccontrol/sync_control.go index 24432169..560ce3b3 100644 --- a/pkg/controllers/collaset/synccontrol/sync_control.go +++ b/pkg/controllers/collaset/synccontrol/sync_control.go @@ -203,23 +203,43 @@ func (r *RealSyncControl) Scale( // scale out new Pods with updatedRevision // TODO use cache - pod, err := collasetutils.NewPodFrom(cls, metav1.NewControllerRef(cls, appsv1alpha1.GroupVersion.WithKind("CollaSet")), revision) + pod, err := collasetutils.NewPodFrom( + cls, + metav1.NewControllerRef(cls, appsv1alpha1.GroupVersion.WithKind("CollaSet")), + revision, + func(in *corev1.Pod) (localErr error) { + in.Labels[appsv1alpha1.PodInstanceIDLabelKey] = fmt.Sprintf("%d", availableIDContext.ID) + revisionsInfo, ok := availableIDContext.Get(podcontext.PodDecorationRevisionKey) + var pds map[string]*appsv1alpha1.PodDecoration + if !ok { + // get default PodDecorations if no revision in context + pds, localErr = resources.PDGetter.GetLatestDecorationsByTargetLabel(ctx, in.Labels) + if localErr != nil { + return localErr + } + } else { + // upgrade by recreate pod case + infos, marshallErr := utilspoddecoration.UnmarshallFromString(revisionsInfo) + if marshallErr != nil { + return marshallErr + } + var revisions []string + for _, info := range infos { + revisions = append(revisions, info.Revision) + } + pds, localErr = resources.PDGetter.GetDecorationByRevisions(ctx, revisions...) + if localErr != nil { + return localErr + } + } + logger.Info("get pod effective decorations before create it", "EffectivePodDecorations", utilspoddecoration.BuildInfo(pds)) + return utilspoddecoration.PatchListOfDecorations(in, pds) + }, + ) if err != nil { return fmt.Errorf("fail to new Pod from revision %s: %s", revision.Name, err) } newPod := pod.DeepCopy() - // allocate new Pod a instance ID - newPod.Labels[appsv1alpha1.PodInstanceIDLabelKey] = fmt.Sprintf("%d", availableIDContext.ID) - - // get PodDecorations which selected newPod - podDecorations := utilspoddecoration.GetPodEffectiveDecorations(newPod, resources.PodDecorations, resources.OldRevisionDecorations) - // patch pod with PodDecorations - if patchErr := utilspoddecoration.PatchListOfDecorations(newPod, podDecorations); patchErr != nil { - msg := fmt.Sprintf("fail to patch pod %s by PodDecoration, %v", commonutils.ObjectKeyString(newPod), patchErr) - logger.Error(patchErr, msg) - r.recorder.Eventf(cls, corev1.EventTypeWarning, "PodDecorationPatch", msg) - } - logger.V(1).Info("try to create Pod with revision of collaSet", "revision", revision.Name) if pod, err = r.podControl.CreatePod(newPod); err != nil { return err @@ -401,8 +421,10 @@ func (r *RealSyncControl) Update( logger := r.logger.WithValues("collaset", commonutils.ObjectKeyString(cls)) var recordedRequeueAfter *time.Duration // 1. scan and analysis pods update info - podUpdateInfos := attachPodUpdateInfo(podWrappers, resources) - + podUpdateInfos, err := attachPodUpdateInfo(ctx, podWrappers, resources) + if err != nil { + return false, nil, fmt.Errorf("fail to attach pod update info, %v", err) + } // 2. decide Pod update candidates podToUpdate := decidePodToUpdate(cls, podUpdateInfos) @@ -464,7 +486,13 @@ func (r *RealSyncControl) Update( needUpdateContext = true ownedIDs[podInfo.ID].Put(podcontext.RevisionContextDataKey, resources.UpdatedRevision.Name) } - + if podInfo.PodDecorationChanged { + decorationStr := utilspoddecoration.GetDecorationInfoString(podInfo.UpdatedPodDecorations) + if val, ok := ownedIDs[podInfo.ID].Get(podcontext.PodDecorationRevisionKey); !ok || val != decorationStr { + needUpdateContext = true + ownedIDs[podInfo.ID].Put(podcontext.PodDecorationRevisionKey, decorationStr) + } + } if podInfo.IsUpdatedRevision && !podInfo.PodDecorationChanged { continue } diff --git a/pkg/controllers/collaset/synccontrol/update.go b/pkg/controllers/collaset/synccontrol/update.go index 71bea3da..c62184a3 100644 --- a/pkg/controllers/collaset/synccontrol/update.go +++ b/pkg/controllers/collaset/synccontrol/update.go @@ -26,6 +26,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" appsv1alpha1 "kusionstack.io/operating/apis/apps/v1alpha1" @@ -47,24 +48,45 @@ type PodUpdateInfo struct { // indicates effected PodDecorations changed PodDecorationChanged bool - //OldPodDecorations + CurrentPodDecorations map[string]*appsv1alpha1.PodDecoration UpdatedPodDecorations map[string]*appsv1alpha1.PodDecoration // indicates the PodOpsLifecycle is started. isDuringOps bool } -func attachPodUpdateInfo(pods []*collasetutils.PodWrapper, resource *collasetutils.RelatedResources) []*PodUpdateInfo { +func attachPodUpdateInfo(ctx context.Context, pods []*collasetutils.PodWrapper, resource *collasetutils.RelatedResources) ([]*PodUpdateInfo, error) { podUpdateInfoList := make([]*PodUpdateInfo, len(pods)) for i, pod := range pods { updateInfo := &PodUpdateInfo{ PodWrapper: pod, } + currentPDs, err := resource.PDGetter.GetCurrentDecorationsOnPod(ctx, pod.Pod) + if err != nil { + return nil, err + } + updatedPDs, err := resource.PDGetter.GetUpdatedDecorationsByOldPod(ctx, pod.Pod) + if err != nil { + return nil, err + } - decorations := utilspoddecoration.GetPodEffectiveDecorations(pod.Pod, resource.PodDecorations, resource.OldRevisionDecorations) - updateInfo.UpdatedPodDecorations = decorations - updateInfo.PodDecorationChanged = utilspoddecoration.ShouldUpdateDecorationInfo(pod.Pod, decorations) + if len(currentPDs) != len(updatedPDs) { + updateInfo.PodDecorationChanged = true + } else { + revisionSets := sets.NewString() + for rev := range currentPDs { + revisionSets.Insert(rev) + } + for rev := range updatedPDs { + if !revisionSets.Has(rev) { + updateInfo.PodDecorationChanged = true + break + } + } + } + updateInfo.CurrentPodDecorations = currentPDs + updateInfo.UpdatedPodDecorations = updatedPDs // decide this pod current revision, or nil if not indicated if pod.Labels != nil { @@ -90,7 +112,7 @@ func attachPodUpdateInfo(pods []*collasetutils.PodWrapper, resource *collasetuti podUpdateInfoList[i] = updateInfo } - return podUpdateInfoList + return podUpdateInfoList, nil } func decidePodToUpdate(cls *appsv1alpha1.CollaSet, podInfos []*PodUpdateInfo) []*PodUpdateInfo { @@ -105,6 +127,7 @@ func decidePodToUpdateByLabel(_ *appsv1alpha1.CollaSet, podInfos []*PodUpdateInf for i := range podInfos { if _, exist := podInfos[i].Labels[appsv1alpha1.CollaSetUpdateIndicateLabelKey]; exist { podToUpdate = append(podToUpdate, podInfos[i]) + continue } if podInfos[i].PodDecorationChanged { podToUpdate = append(podToUpdate, podInfos[i]) @@ -197,52 +220,31 @@ func (u *inPlaceIfPossibleUpdater) AnalyseAndGetUpdatedPod( updatedRevision *appsv1.ControllerRevision, podUpdateInfo *PodUpdateInfo) ( inPlaceUpdateSupport bool, onlyMetadataChanged bool, updatedPod *corev1.Pod, err error) { + // 1. build pod from current and updated revision ownerRef := metav1.NewControllerRef(u.collaSet, appsv1alpha1.GroupVersion.WithKind("CollaSet")) // TODO: use cache - currentPod, err := collasetutils.NewPodFrom(u.collaSet, ownerRef, podUpdateInfo.CurrentRevision) + currentPod, err := collasetutils.NewPodFrom(u.collaSet, ownerRef, podUpdateInfo.CurrentRevision, func(in *corev1.Pod) error { + return utilspoddecoration.PatchListOfDecorations(in, podUpdateInfo.CurrentPodDecorations) + }) if err != nil { err = fmt.Errorf("fail to build Pod from current revision %s: %v", podUpdateInfo.CurrentRevision.Name, err) return } // TODO: use cache - updatedPod, err = collasetutils.NewPodFrom(u.collaSet, ownerRef, updatedRevision) + updatedPod, err = collasetutils.NewPodFrom(u.collaSet, ownerRef, updatedRevision, func(in *corev1.Pod) error { + return utilspoddecoration.PatchListOfDecorations(in, podUpdateInfo.UpdatedPodDecorations) + }) if err != nil { err = fmt.Errorf("fail to build Pod from updated revision %s: %v", updatedRevision.Name, err) return } - // 2.1 patch PodDecorations on current pod - if podUpdateInfo.PodDecorationChanged { - var notFound bool - var currentPodDecorations map[string]*appsv1alpha1.PodDecoration - notFound, currentPodDecorations, err = utilspoddecoration.GetPodDecorationsByPodAnno(u.ctx, u.Client, podUpdateInfo.Pod) - - if err != nil { - return false, false, nil, err - } - // if NotFound PD, recreate pod. - if notFound { - return false, false, nil, err - } - if err = utilspoddecoration.PatchListOfDecorations(currentPod, currentPodDecorations); err != nil { - return false, false, nil, err - } - } else { - if err = utilspoddecoration.PatchListOfDecorations(currentPod, podUpdateInfo.UpdatedPodDecorations); err != nil { - return false, false, nil, err - } - } - // 2.1 patch PodDecorations on updated pod - if err = utilspoddecoration.PatchListOfDecorations(updatedPod, podUpdateInfo.UpdatedPodDecorations); err != nil { - return false, false, nil, err - } - - // 3. compare current and updated pods. Only pod image and metadata are supported to update in-place + // 2. compare current and updated pods. Only pod image and metadata are supported to update in-place // TODO: use cache inPlaceUpdateSupport, onlyMetadataChanged = u.diffPod(currentPod, updatedPod) - // 4. if pod has changes more than metadata and image + // 3. if pod has changes more than metadata and image if !inPlaceUpdateSupport { return false, onlyMetadataChanged, nil, nil } @@ -291,10 +293,6 @@ func (u *inPlaceIfPossibleUpdater) AnalyseAndGetUpdatedPod( return } -func (u *inPlaceIfPossibleUpdater) patchPodDecorations() { - -} - func (u *inPlaceIfPossibleUpdater) diffPod(currentPod, updatedPod *corev1.Pod) (inPlaceSetUpdateSupport bool, onlyMetadataChanged bool) { if len(currentPod.Spec.Containers) != len(updatedPod.Spec.Containers) { return false, false diff --git a/pkg/controllers/collaset/utils/pod.go b/pkg/controllers/collaset/utils/pod.go index 80826390..dbfb2121 100644 --- a/pkg/controllers/collaset/utils/pod.go +++ b/pkg/controllers/collaset/utils/pod.go @@ -69,7 +69,7 @@ func GetPodInstanceID(pod *corev1.Pod) (int, error) { return int(id), nil } -func NewPodFrom(owner metav1.Object, ownerRef *metav1.OwnerReference, revision *appsv1.ControllerRevision) (*corev1.Pod, error) { +func NewPodFrom(owner metav1.Object, ownerRef *metav1.OwnerReference, revision *appsv1.ControllerRevision, updateFn ...func(*corev1.Pod) error) (*corev1.Pod, error) { pod, err := GetPodFromRevision(revision) if err != nil { return pod, err @@ -82,12 +82,17 @@ func NewPodFrom(owner metav1.Object, ownerRef *metav1.OwnerReference, revision * pod.Labels[appsv1.ControllerRevisionHashLabelKey] = revision.Name utils.ControllByKusionStack(pod) + for _, fn := range updateFn { + if err = fn(pod); err != nil { + return pod, err + } + } return pod, nil } func GetPodRevisionPatch(revision *appsv1.ControllerRevision) ([]byte, error) { var raw map[string]interface{} - if err := json.Unmarshal([]byte(revision.Data.Raw), &raw); err != nil { + if err := json.Unmarshal(revision.Data.Raw, &raw); err != nil { return nil, err } @@ -115,7 +120,7 @@ func ApplyPatchFromRevision(pod *corev1.Pod, revision *appsv1.ControllerRevision return clone, nil } -// PatchToPod Use three way merge to get a updated pod. +// PatchToPod Use three-way merge to get a updated pod. func PatchToPod(currentRevisionPod, updateRevisionPod, currentPod *corev1.Pod) (*corev1.Pod, error) { currentRevisionPodBytes, err := json.Marshal(currentRevisionPod) if err != nil { diff --git a/pkg/controllers/collaset/utils/poddecoration.go b/pkg/controllers/collaset/utils/poddecoration.go index 4a130ef9..85f0fb86 100644 --- a/pkg/controllers/collaset/utils/poddecoration.go +++ b/pkg/controllers/collaset/utils/poddecoration.go @@ -21,52 +21,168 @@ import ( "fmt" appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" appsv1alpha1 "kusionstack.io/operating/apis/apps/v1alpha1" utilspoddecoration "kusionstack.io/operating/pkg/controllers/utils/poddecoration" + "kusionstack.io/operating/pkg/utils" ) -func GetEffectiveDecorationsByCollaSet( - ctx context.Context, - c client.Client, - colla *appsv1alpha1.CollaSet, -) ( - podDecorations []*appsv1alpha1.PodDecoration, oldRevisions map[string]*appsv1alpha1.PodDecoration, err error) { +type PodDecorationGetter interface { + GetLatestDecorations() []*appsv1alpha1.PodDecoration + GetCurrentDecorationsOnPod(ctx context.Context, pod *corev1.Pod) (map[string]*appsv1alpha1.PodDecoration, error) + GetDecorationByRevisions(ctx context.Context, revisions ...string) (map[string]*appsv1alpha1.PodDecoration, error) + GetLatestDecorationsByTargetLabel(ctx context.Context, labels map[string]string) (map[string]*appsv1alpha1.PodDecoration, error) + GetUpdatedDecorationsByOldPod(ctx context.Context, pod *corev1.Pod) (map[string]*appsv1alpha1.PodDecoration, error) + GetUpdatedDecorationsByOldRevisions(ctx context.Context, labels map[string]string, oldPDRevisions map[string]string) (map[string]*appsv1alpha1.PodDecoration, error) +} - pdList := &appsv1alpha1.PodDecorationList{} - if err = c.List(ctx, pdList, &client.ListOptions{Namespace: colla.Namespace}); err != nil { - return +func NewPodDecorationGetter(ctx context.Context, c client.Client, namespace string) (PodDecorationGetter, error) { + getter := &podDecorationGetter{ + namespace: namespace, + Client: c, + latestPodDecorationNames: sets.NewString(), + revisions: map[string]*appsv1alpha1.PodDecoration{}, } - for i := range pdList.Items { - if isAffectedCollaSet(&pdList.Items[i], colla) { - podDecorations = append(podDecorations, &pdList.Items[i]) + return getter, getter.getLatest(ctx) +} + +type podDecorationGetter struct { + client.Client + namespace string + + latestPodDecorations []*appsv1alpha1.PodDecoration + latestPodDecorationNames sets.String + revisions map[string]*appsv1alpha1.PodDecoration +} + +func (p *podDecorationGetter) GetLatestDecorations() []*appsv1alpha1.PodDecoration { + return p.latestPodDecorations +} + +func (p *podDecorationGetter) GetCurrentDecorationsOnPod(ctx context.Context, pod *corev1.Pod) (map[string]*appsv1alpha1.PodDecoration, error) { + infos := utilspoddecoration.GetDecorationRevisionInfo(pod) + var revisions []string + for _, info := range infos { + revisions = append(revisions, info.Revision) + } + return p.GetDecorationByRevisions(ctx, revisions...) +} + +func (p *podDecorationGetter) GetDecorationByRevisions(ctx context.Context, revisions ...string) (map[string]*appsv1alpha1.PodDecoration, error) { + res := map[string]*appsv1alpha1.PodDecoration{} + var err error + for _, rev := range revisions { + if pd, ok := p.revisions[rev]; ok { + res[rev] = pd + continue + } + pd, localErr := p.getByRevision(ctx, rev) + if localErr != nil { + err = utils.Join(err, localErr) + continue } + res[rev] = pd } - oldRevisions = map[string]*appsv1alpha1.PodDecoration{} - for _, pd := range podDecorations { - if pd.Status.CurrentRevision != "" && pd.Status.CurrentRevision != pd.Status.UpdatedRevision { - revision := &appsv1.ControllerRevision{} - if err = c.Get(ctx, types.NamespacedName{Namespace: colla.Namespace, Name: pd.Status.CurrentRevision}, revision); err != nil { - return nil, nil, fmt.Errorf("fail to get PodDecoration ControllerRevision %s/%s: %v", colla.Namespace, pd.Status.CurrentRevision, err) - } - oldPD, err := utilspoddecoration.GetPodDecorationFromRevision(revision) - if err != nil { - return nil, nil, err + return res, err +} + +// GetLatestDecorationsByTargetLabel used to get PodDecorations for a given pod's label. +func (p *podDecorationGetter) GetLatestDecorationsByTargetLabel(ctx context.Context, labels map[string]string) (map[string]*appsv1alpha1.PodDecoration, error) { + updatedRevisions, stableRevisions := utilspoddecoration.GetEffectiveRevisionsFormLatestDecorations(p.latestPodDecorations, labels) + return p.GetDecorationByRevisions(ctx, append(updatedRevisions.List(), stableRevisions.List()...)...) +} + +func (p *podDecorationGetter) GetUpdatedDecorationsByOldPod(ctx context.Context, pod *corev1.Pod) (map[string]*appsv1alpha1.PodDecoration, error) { + infos := utilspoddecoration.GetDecorationRevisionInfo(pod) + oldRevisions := map[string]string{} + for _, info := range infos { + oldRevisions[info.Name] = info.Revision + } + return p.GetUpdatedDecorationsByOldRevisions(ctx, pod.Labels, oldRevisions) +} + +func (p *podDecorationGetter) GetUpdatedDecorationsByOldRevisions(ctx context.Context, labels map[string]string, oldPDRevisions map[string]string) (map[string]*appsv1alpha1.PodDecoration, error) { + updatedRevisions, _ := utilspoddecoration.GetEffectiveRevisionsFormLatestDecorations(p.latestPodDecorations, labels) + // key: Group name, value: PodDecoration name + effectiveGroup := map[string]string{} + updatedPDs, err := p.GetDecorationByRevisions(ctx, updatedRevisions.List()...) + if err != nil { + return nil, err + } + // delete updated PodDecorations in old revisions + for _, pd := range updatedPDs { + if pd.Spec.InjectStrategy.Group != "" { + effectiveGroup[pd.Spec.InjectStrategy.Group] = pd.Name + } + delete(oldPDRevisions, pd.Name) + } + + var oldStableRevisions []string + for _, revision := range oldPDRevisions { + oldStableRevisions = append(oldStableRevisions, revision) + } + + // get old stable PodDecorations + oldStablePDs, err := p.GetDecorationByRevisions(ctx, oldStableRevisions...) + if err != nil { + return nil, err + } + // delete updated group in old stable PodDecorations + var shouldDeleteRevisions []string + for rev, pd := range oldStablePDs { + group := pd.Spec.InjectStrategy.Group + if group != "" { + if _, ok := effectiveGroup[group]; ok { + shouldDeleteRevisions = append(shouldDeleteRevisions, rev) } - oldRevisions[pd.Status.CurrentRevision] = oldPD } } - return + for _, rev := range shouldDeleteRevisions { + delete(oldStablePDs, rev) + } + + for rev, pd := range oldStablePDs { + if p.latestPodDecorationNames.Has(pd.Name) { + updatedPDs[rev] = pd + } + } + return updatedPDs, nil } -func isAffectedCollaSet(pd *appsv1alpha1.PodDecoration, colla *appsv1alpha1.CollaSet) bool { - if pd.Status.IsEffective == nil || !*pd.Status.IsEffective { - return false +func (p *podDecorationGetter) getByRevision(ctx context.Context, rev string) (*appsv1alpha1.PodDecoration, error) { + if pd, ok := p.revisions[rev]; ok { + return pd, nil } - sel, _ := metav1.LabelSelectorAsSelector(pd.Spec.Selector) - return sel.Matches(labels.Set(colla.Spec.Template.Labels)) + revision := &appsv1.ControllerRevision{} + if err := p.Get(ctx, types.NamespacedName{Namespace: p.namespace, Name: rev}, revision); err != nil { + return nil, fmt.Errorf("fail to get PodDecoration ControllerRevision %s/%s: %v", p.namespace, rev, err) + } + pd, err := utilspoddecoration.GetPodDecorationFromRevision(revision) + if err != nil { + return nil, err + } + p.revisions[rev] = pd + return pd, nil +} + +func (p *podDecorationGetter) getLatest(ctx context.Context) (err error) { + pdList := &appsv1alpha1.PodDecorationList{} + if err = p.List(ctx, pdList, &client.ListOptions{Namespace: p.namespace}); err != nil { + return err + } + for i := range pdList.Items { + pd := &pdList.Items[i] + if pd.Status.UpdatedRevision != "" && pd.Status.ObservedGeneration == pd.Generation { + p.revisions[pd.Status.UpdatedRevision] = pd + } + if pd.Status.IsEffective != nil && *pd.Status.IsEffective && pd.DeletionTimestamp == nil { + p.latestPodDecorations = append(p.latestPodDecorations, pd) + p.latestPodDecorationNames.Insert(pd.Name) + } + } + return } diff --git a/pkg/controllers/collaset/utils/poddecoration_test.go b/pkg/controllers/collaset/utils/poddecoration_test.go new file mode 100644 index 00000000..df505f24 --- /dev/null +++ b/pkg/controllers/collaset/utils/poddecoration_test.go @@ -0,0 +1,80 @@ +/* +Copyright 2023 The KusionStack Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package utils + +import ( + "context" + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + + appsv1alpha1 "kusionstack.io/operating/apis/apps/v1alpha1" +) + +func TestPodDecorationUtils(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "CollaSetController Test Suite") +} + +var _ = Describe("PodDecoration utils", func() { + It("Test PodDecorationGetter", func() { + getter := &podDecorationGetter{ + latestPodDecorationNames: sets.NewString("foo-1", "foo-2"), + revisions: map[string]*appsv1alpha1.PodDecoration{}, + } + getter.latestPodDecorations = []*appsv1alpha1.PodDecoration{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-1", + }, + Status: appsv1alpha1.PodDecorationStatus{ + CurrentRevision: "foo-100", + UpdatedRevision: "foo-101", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-2", + }, + Status: appsv1alpha1.PodDecorationStatus{ + CurrentRevision: "foo-200", + UpdatedRevision: "foo-201", + }, + }, + } + getter.revisions["foo-100"] = getter.latestPodDecorations[0] + getter.revisions["foo-101"] = getter.latestPodDecorations[0] + getter.revisions["foo-200"] = getter.latestPodDecorations[1] + getter.revisions["foo-201"] = getter.latestPodDecorations[1] + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{appsv1alpha1.AnnotationResourceDecorationRevision: "[{\"name\":\"foo-1\",\"revision\":\"foo-100\"},{\"name\":\"foo-2\",\"revision\":\"foo-200\"}]"}}} + pds, err := getter.GetUpdatedDecorationsByOldPod(context.TODO(), pod) + Expect(err).Should(BeNil()) + Expect(len(pds)).Should(Equal(2)) + Expect(pds["foo-101"]).ShouldNot(BeNil()) + Expect(pds["foo-201"]).ShouldNot(BeNil()) + getter.latestPodDecorationNames = sets.NewString() + getter.latestPodDecorations = []*appsv1alpha1.PodDecoration{} + pod.Annotations[appsv1alpha1.AnnotationResourceDecorationRevision] = "[{\"name\":\"foo-1\",\"revision\":\"foo-101\"},{\"name\":\"foo-2\",\"revision\":\"foo-201\"}]" + pds, err = getter.GetUpdatedDecorationsByOldPod(context.TODO(), pod) + Expect(err).Should(BeNil()) + Expect(len(pds)).Should(Equal(0)) + }) +}) diff --git a/pkg/controllers/collaset/utils/resource.go b/pkg/controllers/collaset/utils/resource.go index 54f359ad..55f90ab0 100644 --- a/pkg/controllers/collaset/utils/resource.go +++ b/pkg/controllers/collaset/utils/resource.go @@ -27,9 +27,7 @@ type RelatedResources struct { CurrentRevision *appsv1.ControllerRevision UpdatedRevision *appsv1.ControllerRevision - // collaSet related PodDecoration - PodDecorations []*appsv1alpha1.PodDecoration - OldRevisionDecorations map[string]*appsv1alpha1.PodDecoration + PDGetter PodDecorationGetter NewStatus *appsv1alpha1.CollaSetStatus } diff --git a/pkg/controllers/poddecoration/poddecoration_controller.go b/pkg/controllers/poddecoration/poddecoration_controller.go index 38ee4f30..c0788d27 100644 --- a/pkg/controllers/poddecoration/poddecoration_controller.go +++ b/pkg/controllers/poddecoration/poddecoration_controller.go @@ -23,9 +23,11 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/util/retry" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -154,7 +156,7 @@ func (r *ReconcilePodDecoration) Reconcile(ctx context.Context, request reconcil UpdatedRevision: updatedRevision.Name, CollisionCount: *collisionCount, } - err = r.calculateStatus(ctx, instance, newStatus, affectedPods, affectedCollaSets, instance.Spec.DisablePodDetail) + err = r.calculateStatus(instance, newStatus, affectedPods, affectedCollaSets, instance.Spec.DisablePodDetail) if err != nil { return reconcile.Result{}, err } @@ -162,17 +164,12 @@ func (r *ReconcilePodDecoration) Reconcile(ctx context.Context, request reconcil } func (r *ReconcilePodDecoration) calculateStatus( - ctx context.Context, instance *appsv1alpha1.PodDecoration, status *appsv1alpha1.PodDecorationStatus, affectedPods map[string][]*corev1.Pod, - affectedCollaSets []*appsv1alpha1.CollaSet, + affectedCollaSets sets.String, disablePodDetail bool) error { - heaviest, err := utilspoddecoration.GetHeaviestPDByGroup(ctx, r.Client, instance.Namespace, instance.Spec.InjectStrategy.Group) - if err != nil { - return err - } hasEffectivePods := false status.MatchedPods = 0 status.UpdatedPods = 0 @@ -180,16 +177,15 @@ func (r *ReconcilePodDecoration) calculateStatus( status.UpdatedAvailablePods = 0 status.InjectedPods = 0 var details []appsv1alpha1.PodDecorationWorkloadDetail - for _, collaSet := range affectedCollaSets { - pods := affectedPods[collaSet.Name] + for collaSet := range affectedCollaSets { + pods := affectedPods[collaSet] detail := appsv1alpha1.PodDecorationWorkloadDetail{ AffectedReplicas: int32(len(pods)), - CollaSet: collaSet.Name, + CollaSet: collaSet, } status.MatchedPods += int32(len(pods)) for _, pod := range pods { - currentRevision := utilspoddecoration.GetDecorationGroupRevisionInfo(pod). - GetGroupPDRevision(instance.Spec.InjectStrategy.Group, instance.Name) + currentRevision := utilspoddecoration.GetDecorationRevisionInfo(pod).GetRevision(instance.Name) if currentRevision != nil { hasEffectivePods = true status.InjectedPods++ @@ -216,15 +212,33 @@ func (r *ReconcilePodDecoration) calculateStatus( } details = append(details, detail) } - fullControlByOthPD := heaviest != nil && heaviest.Name != instance.Name && heaviest.Status.CurrentRevision != "" - status.IsEffective = BoolPointer(instance.DeletionTimestamp == nil && (!fullControlByOthPD || hasEffectivePods)) - if status.UpdatedPods == status.MatchedPods { + status.IsEffective = BoolPointer(instance.DeletionTimestamp == nil || hasEffectivePods) + if status.CurrentRevision != status.UpdatedRevision && + status.UpdatedPods == status.MatchedPods && + r.allCollaSetsSatisfyReplicas(affectedCollaSets, instance.Namespace) { status.CurrentRevision = status.UpdatedRevision } status.Details = details return nil } +func (r *ReconcilePodDecoration) allCollaSetsSatisfyReplicas(collaSets sets.String, ns string) bool { + collaSet := &appsv1alpha1.CollaSet{} + for name := range collaSets { + if err := r.Get(context.TODO(), types.NamespacedName{Namespace: ns, Name: name}, collaSet); err != nil { + if errors.IsNotFound(err) { + continue + } + return false + } + // Unreliable in rare cases. + if collaSet.Status.Replicas != *collaSet.Spec.Replicas { + return false + } + } + return true +} + func (r *ReconcilePodDecoration) updateStatus( ctx context.Context, instance *appsv1alpha1.PodDecoration, @@ -255,7 +269,7 @@ func (r *ReconcilePodDecoration) filterOutPodAndCollaSet( ctx context.Context, instance *appsv1alpha1.PodDecoration) ( affectedPods map[string][]*corev1.Pod, - affectedCollaSets []*appsv1alpha1.CollaSet, err error) { + affectedCollaSets sets.String, err error) { var sel labels.Selector podList := &corev1.PodList{} if instance.Spec.Selector != nil { @@ -268,10 +282,12 @@ func (r *ReconcilePodDecoration) filterOutPodAndCollaSet( }); err != nil || len(podList.Items) == 0 { return } + affectedCollaSets = sets.NewString() for i := 0; i < len(podList.Items); i++ { ownerRef := metav1.GetControllerOf(&podList.Items[i]) if ownerRef != nil && ownerRef.Kind == "CollaSet" { affectedPods[ownerRef.Name] = append(affectedPods[ownerRef.Name], &podList.Items[i]) + affectedCollaSets.Insert(ownerRef.Name) } } for key, pods := range affectedPods { @@ -280,18 +296,6 @@ func (r *ReconcilePodDecoration) filterOutPodAndCollaSet( }) affectedPods[key] = pods } - collaSetList := &appsv1alpha1.CollaSetList{} - if err = r.List(ctx, collaSetList, &client.ListOptions{Namespace: instance.Namespace}); err != nil { - return - } - for i := range collaSetList.Items { - if sel == nil || sel.Matches(labels.Set(collaSetList.Items[i].Spec.Template.Labels)) { - affectedCollaSets = append(affectedCollaSets, &collaSetList.Items[i]) - } - } - sort.Slice(affectedCollaSets, func(i, j int) bool { - return affectedCollaSets[i].Name < affectedCollaSets[j].Name - }) return } diff --git a/pkg/controllers/poddecoration/poddecoration_controller_test.go b/pkg/controllers/poddecoration/poddecoration_controller_test.go index 4d4dba60..25fc0920 100644 --- a/pkg/controllers/poddecoration/poddecoration_controller_test.go +++ b/pkg/controllers/poddecoration/poddecoration_controller_test.go @@ -92,6 +92,7 @@ var _ = Describe("PodDecoration controller", func() { }, }, } + // 1, create collaSet Expect(c.Create(ctx, collaSetA)).Should(BeNil()) podList := &corev1.PodList{} Eventually(func() int { @@ -136,7 +137,7 @@ var _ = Describe("PodDecoration controller", func() { }, }, } - // create pd + // 2, create pd Expect(c.Create(ctx, podDecoration)).Should(BeNil()) Eventually(func() error { return c.Get(ctx, types.NamespacedName{Name: podDecoration.Name, Namespace: testcase}, podDecoration) @@ -147,7 +148,11 @@ var _ = Describe("PodDecoration controller", func() { return podDecoration.Status.MatchedPods }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(int32(2))) - // 2 pods during ops + Eventually(func() bool { + Expect(c.Get(ctx, types.NamespacedName{Name: podDecoration.Name, Namespace: testcase}, podDecoration)).Should(BeNil()) + return podDecoration.Status.IsEffective != nil && *podDecoration.Status.IsEffective + }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(true)) + // 3, Expect two pods during ops Eventually(func() int { Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) cnt := 0 @@ -158,7 +163,7 @@ var _ = Describe("PodDecoration controller", func() { } return cnt }, 10*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) - // allow Pod to do update + // 4, Allow Pod to update Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) for i := range podList.Items { pod := &podList.Items[i] @@ -169,13 +174,15 @@ var _ = Describe("PodDecoration controller", func() { return true })).Should(BeNil()) } - - // 2 pods recreated + // 5, Two pods recreated Eventually(func() int32 { Expect(c.Get(ctx, types.NamespacedName{Name: podDecoration.Name, Namespace: testcase}, podDecoration)).Should(BeNil()) return podDecoration.Status.UpdatedPods }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(int32(2))) - //PodInstanceIDLabelKey + Expect(c.List(ctx, podList, client.InNamespace(testcase))).Should(BeNil()) + for _, po := range podList.Items { + Expect(len(po.Spec.Containers)).Should(Equal(2)) + } }) It("test reconcile multi CollaSet with one PodDecoration", func() { @@ -257,7 +264,6 @@ var _ = Describe("PodDecoration controller", func() { }, }, InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-a", Weight: int32Pointer(10), }, UpdateStrategy: appsv1alpha1.PodDecorationUpdateStrategy{ @@ -292,6 +298,8 @@ var _ = Describe("PodDecoration controller", func() { } return false }, 5*time.Second, 1*time.Second).Should(BeTrue()) + Expect(podDecoration.Status.UpdatedRevision).ShouldNot(Equal("")) + Expect(podDecoration.Status.UpdatedRevision).Should(Equal(podDecoration.Status.CurrentRevision)) // create CollaSet after podDecoration, do not need to allow Pod to update Expect(c.Create(ctx, collaSetA)).Should(BeNil()) Expect(c.Create(ctx, collaSetB)).Should(BeNil()) @@ -305,7 +313,58 @@ var _ = Describe("PodDecoration controller", func() { } } return updatedCnt - }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) + }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(4)) + + Eventually(func() error { + err := c.Get(ctx, types.NamespacedName{Name: podDecoration.Name, Namespace: testcase}, podDecoration) + if err != nil { + return err + } + podDecoration.Spec.Template.InitContainers = []*corev1.Container{ + { + Name: "init", + Image: "nginx:v3", + }, + } + return c.Update(ctx, podDecoration) + }, 5*time.Second, 1*time.Second).Should(BeNil()) + + // Expect two pods during ops + Eventually(func() int { + Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) + cnt := 0 + for i := range podList.Items { + if podopslifecycle.IsDuringOps(collasetutils.UpdateOpsLifecycleAdapter, &podList.Items[i]) { + cnt++ + } + } + return cnt + }, 10*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) + // Allow Pod to update + Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) + for i := range podList.Items { + pod := &podList.Items[i] + // allow Pod to do update + if pod.Labels[appsv1alpha1.PodInstanceIDLabelKey] != "0" { + continue + } + Expect(updatePodWithRetry(ctx, c, pod.Namespace, pod.Name, func(pod *corev1.Pod) bool { + labelOperate := fmt.Sprintf("%s/%s", appsv1alpha1.PodOperateLabelPrefix, collasetutils.UpdateOpsLifecycleAdapter.GetID()) + pod.Labels[labelOperate] = fmt.Sprintf("%d", time.Now().UnixNano()) + return true + })).Should(BeNil()) + } + // Two pods inject init container + Eventually(func() int { + Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) + cnt := 0 + for _, po := range podList.Items { + if len(po.Spec.InitContainers) == 1 { + cnt++ + } + } + return cnt + }, 10*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) }) It("test delete PodDecoration", func() { @@ -355,7 +414,6 @@ var _ = Describe("PodDecoration controller", func() { }, }, InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-a", Weight: int32Pointer(10), }, UpdateStrategy: appsv1alpha1.PodDecorationUpdateStrategy{ @@ -381,7 +439,7 @@ var _ = Describe("PodDecoration controller", func() { Expect(c.Create(ctx, podDecoration)).Should(BeNil()) Eventually(func() bool { if c.Get(ctx, types.NamespacedName{Name: podDecoration.Name, Namespace: testcase}, podDecoration) == nil { - return len(podDecoration.Finalizers) != 0 + return len(podDecoration.Finalizers) != 0 && podDecoration.Status.IsEffective != nil && *podDecoration.Status.IsEffective } return false }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(true)) @@ -400,10 +458,23 @@ var _ = Describe("PodDecoration controller", func() { }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) Expect(c.Delete(ctx, podDecoration)).Should(BeNil()) // PodDecoration is disabled + Eventually(func() interface{} { + Expect(c.Get(ctx, types.NamespacedName{Name: podDecoration.Name, Namespace: testcase}, podDecoration)).Should(BeNil()) + return podDecoration.DeletionTimestamp + }, 5*time.Second, 1*time.Second).ShouldNot(BeNil()) + + getter, err := collasetutils.NewPodDecorationGetter(ctx, c, testcase) + Expect(err).Should(BeNil()) + if len(getter.GetLatestDecorations()) != 0 { + bt, _ := json.Marshal(getter.GetLatestDecorations()[0]) + fmt.Printf("test : %s\n", string(bt)) + } + Expect(len(getter.GetLatestDecorations())).Should(Equal(0)) + Eventually(func() bool { Expect(c.Get(ctx, types.NamespacedName{Name: podDecoration.Name, Namespace: testcase}, podDecoration)).Should(BeNil()) - return podDecoration.Status.IsEffective != nil && !*podDecoration.Status.IsEffective - }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(false)) + return podDecoration.Status.IsEffective != nil && *podDecoration.Status.IsEffective + }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(true)) // 2 pods during ops Eventually(func() int { Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) @@ -414,7 +485,7 @@ var _ = Describe("PodDecoration controller", func() { } } return cnt - }, 10*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) + }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) // allow Pod to do update Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) for i := range podList.Items { @@ -439,14 +510,14 @@ var _ = Describe("PodDecoration controller", func() { }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) // annotation cleared for _, po := range podList.Items { - Expect(po.Annotations[appsv1alpha1.AnnotationResourceDecorationRevision]).Should(BeEquivalentTo("{}")) + Expect(po.Annotations[appsv1alpha1.AnnotationResourceDecorationRevision]).Should(BeEquivalentTo("[]")) } Eventually(func() error { return c.Get(ctx, types.NamespacedName{Name: podDecoration.Name, Namespace: testcase}, podDecoration) }, 5*time.Second, 1*time.Second).Should(HaveOccurred()) }) - It("test PodDecoration group weight", func() { + It("test PodDecoration weight", func() { testcase := "test-pd-3" Expect(createNamespace(c, testcase)).Should(BeNil()) collaSetA := &appsv1alpha1.CollaSet{ @@ -508,7 +579,7 @@ var _ = Describe("PodDecoration controller", func() { { InjectPolicy: appsv1alpha1.AfterPrimaryContainer, Container: corev1.Container{ - Name: "sidecar", + Name: "sidecar-1", Image: "nginx:v2", }, }, @@ -544,8 +615,8 @@ var _ = Describe("PodDecoration controller", func() { { InjectPolicy: appsv1alpha1.AfterPrimaryContainer, Container: corev1.Container{ - Name: "sidecar", - Image: "nginx:v2", + Name: "sidecar-2", + Image: "nginx:v3", }, }, }, @@ -610,10 +681,11 @@ var _ = Describe("PodDecoration controller", func() { // 2 pods updated by PodDecoration-B Eventually(func() int { Expect(c.List(ctx, podList, client.InNamespace(testcase))).Should(BeNil()) + Expect(c.Get(ctx, types.NamespacedName{Name: podDecorationB.Name, Namespace: podDecorationB.Namespace}, podDecorationB)).Should(BeNil()) updatedCnt := 0 for _, po := range podList.Items { - currentPD := utilspoddecoration.GetDecorationGroupRevisionInfo(&po).GetCurrentPDNameByGroup("group-a") - if currentPD != nil && *currentPD == "foo-b" { + info := utilspoddecoration.GetDecorationRevisionInfo(&po) + if info.Size() == 1 && info.GetRevision(podDecorationB.Name) != nil && *info.GetRevision(podDecorationB.Name) == podDecorationB.Status.UpdatedRevision { updatedCnt++ } } diff --git a/pkg/controllers/utils/poddecoration/anno.go b/pkg/controllers/utils/poddecoration/anno.go index 98f900a0..1ea0c5cd 100644 --- a/pkg/controllers/utils/poddecoration/anno.go +++ b/pkg/controllers/utils/poddecoration/anno.go @@ -17,53 +17,41 @@ limitations under the License. package poddecoration import ( - "context" "encoding/json" "fmt" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/client-go/kubernetes/scheme" "k8s.io/klog/v2" - "sigs.k8s.io/controller-runtime/pkg/client" appsv1alpha1 "kusionstack.io/operating/apis/apps/v1alpha1" - "kusionstack.io/operating/pkg/utils" ) -type DecorationGroupRevisionInfo map[string]*DecorationInfo +type DecorationRevisionInfo []*DecorationInfo type DecorationInfo struct { Name string `json:"name"` Revision string `json:"revision"` } -func (d DecorationGroupRevisionInfo) GetGroupPDRevision(group, rdName string) *string { - info, ok := d[group] - if ok && info.Name == rdName { - return &info.Revision +func (d DecorationRevisionInfo) GetRevision(name string) *string { + for _, info := range d { + if info.Name == name { + return &info.Revision + } } return nil } -func (d DecorationGroupRevisionInfo) GetCurrentPDNameByGroup(group string) *string { - info, ok := d[group] - if !ok { - return nil - } - return &info.Name -} - -func (d DecorationGroupRevisionInfo) Size() int { +func (d DecorationRevisionInfo) Size() int { return len(d) } -func GetDecorationGroupRevisionInfo(pod *corev1.Pod) (info DecorationGroupRevisionInfo) { - info = DecorationGroupRevisionInfo{} +func GetDecorationRevisionInfo(pod *corev1.Pod) (info DecorationRevisionInfo) { + info = DecorationRevisionInfo{} if pod.Annotations == nil { return } @@ -78,32 +66,30 @@ func GetDecorationGroupRevisionInfo(pod *corev1.Pod) (info DecorationGroupRevisi } func setDecorationInfo(pod *corev1.Pod, podDecorations map[string]*appsv1alpha1.PodDecoration) { - info := DecorationGroupRevisionInfo{} + if pod.Annotations == nil { + pod.Annotations = map[string]string{} + } + pod.Annotations[appsv1alpha1.AnnotationResourceDecorationRevision] = GetDecorationInfoString(podDecorations) +} + +func GetDecorationInfoString(podDecorations map[string]*appsv1alpha1.PodDecoration) string { + info := DecorationRevisionInfo{} for revision, pd := range podDecorations { - info[pd.Spec.InjectStrategy.Group] = &DecorationInfo{ + info = append(info, &DecorationInfo{ Name: pd.Name, Revision: revision, - } + }) } byt, _ := json.Marshal(info) - if pod.Annotations == nil { - pod.Annotations = map[string]string{} - } - pod.Annotations[appsv1alpha1.AnnotationResourceDecorationRevision] = string(byt) + return string(byt) } -func ShouldUpdateDecorationInfo(pod *corev1.Pod, podDecorations map[string]*appsv1alpha1.PodDecoration) bool { - currentInfo := GetDecorationGroupRevisionInfo(pod) - if currentInfo.Size() != len(podDecorations) { - return true - } - for rv, pd := range podDecorations { - revision := currentInfo.GetGroupPDRevision(pd.Spec.InjectStrategy.Group, pd.Name) - if revision == nil || *revision != rv { - return true - } +func UnmarshallFromString(val string) ([]*DecorationInfo, error) { + info := DecorationRevisionInfo{} + if err := json.Unmarshal([]byte(val), &info); err != nil { + return nil, err } - return false + return info, nil } var PodDecorationCodec = scheme.Codecs.LegacyCodec(appsv1alpha1.GroupVersion) @@ -137,43 +123,3 @@ func GetPodDecorationFromRevision(revision *appsv1.ControllerRevision) (*appsv1a } return podDecoration, nil } - -func GetPodDecorationsByPodAnno(ctx context.Context, c client.Client, pod *corev1.Pod) (notFound bool, podDecorations map[string]*appsv1alpha1.PodDecoration, err error) { - rdRevisions := getEffectivePodDecorationRevisionFromPod(pod) - podDecorations = map[string]*appsv1alpha1.PodDecoration{} - var revisions []*appsv1.ControllerRevision - for _, revisionName := range rdRevisions { - if len(revisionName) == 0 { - continue - } - - revision := &appsv1.ControllerRevision{} - if err = c.Get(ctx, types.NamespacedName{Namespace: pod.Namespace, Name: revisionName}, revision); err != nil { - if errors.IsNotFound(err) { - klog.Errorf("fail to get PodDecoration revision %s for pod %s, [not found]: %v", revisionName, utils.ObjectKeyString(pod), err) - notFound = true - return - } - return false, podDecorations, fmt.Errorf("fail to get PodDecoration revision %s for pod %s: %v", revisionName, utils.ObjectKeyString(pod), err) - } - revisions = append(revisions, revision) - } - - for _, revision := range revisions { - pd, err := GetPodDecorationFromRevision(revision) - if err != nil { - return false, podDecorations, fmt.Errorf("fail to get PodDecoration revision %s for pod %s: %v", revision.Name, utils.ObjectKeyString(pod), err) - } - podDecorations[revision.Name] = pd - } - return -} - -func getEffectivePodDecorationRevisionFromPod(pod *corev1.Pod) map[string]string { - info := GetDecorationGroupRevisionInfo(pod) - res := map[string]string{} - for _, pdInfo := range info { - res[pdInfo.Name] = pdInfo.Revision - } - return res -} diff --git a/pkg/controllers/utils/poddecoration/getter.go b/pkg/controllers/utils/poddecoration/getter.go index 894ec1e6..56d97a83 100644 --- a/pkg/controllers/utils/poddecoration/getter.go +++ b/pkg/controllers/utils/poddecoration/getter.go @@ -17,72 +17,93 @@ limitations under the License. package poddecoration import ( - corev1 "k8s.io/api/core/v1" + "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/sets" appsv1alpha1 "kusionstack.io/operating/apis/apps/v1alpha1" ) -func GetPodEffectiveDecorations(pod *corev1.Pod, podDecorations []*appsv1alpha1.PodDecoration, oldRevisions map[string]*appsv1alpha1.PodDecoration) (res map[string]*appsv1alpha1.PodDecoration) { - type RevisionPD struct { - Revision string - PD *appsv1alpha1.PodDecoration - } - - // revision : PD - res = map[string]*appsv1alpha1.PodDecoration{} - // group : PD - currentGroupPD := map[string]*RevisionPD{} - - tryReplace := func(pd *appsv1alpha1.PodDecoration, revision string) { - current, ok := currentGroupPD[pd.Spec.InjectStrategy.Group] - if !ok { - currentGroupPD[pd.Spec.InjectStrategy.Group] = &RevisionPD{ - Revision: revision, - PD: pd, - } - return - } - if lessPD(pd, current.PD) { - currentGroupPD[pd.Spec.InjectStrategy.Group] = &RevisionPD{ - Revision: revision, - PD: pd, - } - } - } - for i, pd := range podDecorations { - if pd.Spec.Selector != nil { - sel, _ := metav1.LabelSelectorAsSelector(pd.Spec.Selector) - if !sel.Matches(labels.Set(pod.Labels)) { - continue - } - } - // no rolling upgrade, upgrade all - if pd.Spec.UpdateStrategy.RollingUpdate == nil { - tryReplace(podDecorations[i], pd.Status.UpdatedRevision) +func GetEffectiveRevisionsFormLatestDecorations(latestPodDecorations []*appsv1alpha1.PodDecoration, lb map[string]string) (updatedRevisions, stableRevisions sets.String) { + groupedDecorations := map[string]*appsv1alpha1.PodDecoration{} + groupIsUpdatedRevision := map[string]bool{} + groupRevision := map[string]string{} + updatedRevisions = sets.NewString() + stableRevisions = sets.NewString() + for i, pd := range latestPodDecorations { + revision, isUpdatedRevision := getEffectiveRevision(pd, lb) + if revision == "" { continue } - // by selector - if pd.Spec.UpdateStrategy.RollingUpdate.Selector != nil { - sel, _ := metav1.LabelSelectorAsSelector(pd.Spec.UpdateStrategy.RollingUpdate.Selector) - if sel.Matches(labels.Set(pod.Labels)) { - tryReplace(podDecorations[i], pd.Status.UpdatedRevision) - } else if pd.Status.CurrentRevision != "" { - // use CurrentRevision - oldPD, ok := oldRevisions[pd.Status.CurrentRevision] - if ok { - tryReplace(oldPD, pd.Status.CurrentRevision) - } + // no group PD is effective default + if pd.Spec.InjectStrategy.Group == "" { + if isUpdatedRevision { + updatedRevisions.Insert(revision) + } else { + stableRevisions.Insert(revision) } continue } - // TODO: by partition - //if pd.Spec.UpdateStrategy.RollingUpdate.Partition != nil { - //} + + // update by heaviest one + stable, ok := groupedDecorations[pd.Spec.InjectStrategy.Group] + if !ok || isHeaviest(latestPodDecorations[i], stable) { + groupedDecorations[pd.Spec.InjectStrategy.Group] = latestPodDecorations[i] + groupRevision[pd.Spec.InjectStrategy.Group] = revision + groupIsUpdatedRevision[pd.Spec.InjectStrategy.Group] = isUpdatedRevision + } } - for _, revisionPD := range currentGroupPD { - res[revisionPD.Revision] = revisionPD.PD + for group, revision := range groupRevision { + if groupIsUpdatedRevision[group] { + updatedRevisions.Insert(revision) + } else { + stableRevisions.Insert(revision) + } } return } + +func getEffectiveRevision(pd *appsv1alpha1.PodDecoration, lb map[string]string) (string, bool) { + sel, _ := metav1.LabelSelectorAsSelector(pd.Spec.Selector) + if !sel.Matches(labels.Set(lb)) && pd.Spec.Selector != nil { + return "", false + } + if inUpdateStrategy(pd, lb) { + return pd.Status.UpdatedRevision, true + } + return pd.Status.CurrentRevision, false +} + +// if current is heaviest, return true +func isHeaviest(current, t *appsv1alpha1.PodDecoration) bool { + if *current.Spec.InjectStrategy.Weight == *t.Spec.InjectStrategy.Weight { + return current.CreationTimestamp.Time.After(t.CreationTimestamp.Time) + } + return *current.Spec.InjectStrategy.Weight > *t.Spec.InjectStrategy.Weight +} + +func inUpdateStrategy(pd *appsv1alpha1.PodDecoration, lb map[string]string) bool { + if pd.Spec.UpdateStrategy.RollingUpdate == nil { + return true + } + if pd.Spec.UpdateStrategy.RollingUpdate.Selector != nil { + sel, _ := metav1.LabelSelectorAsSelector(pd.Spec.UpdateStrategy.RollingUpdate.Selector) + if sel.Matches(labels.Set(lb)) { + return true + } + } + return false +} + +func BuildInfo(revisionMap map[string]*appsv1alpha1.PodDecoration) (info string) { + for k, v := range revisionMap { + if info == "" { + info = fmt.Sprintf("{%s: %s}", v.Name, k) + } else { + info = info + fmt.Sprintf(", {%s: %s}", v.Name, k) + } + } + return fmt.Sprintf("PodDecorations=[%s]", info) +} diff --git a/pkg/controllers/utils/poddecoration/patch.go b/pkg/controllers/utils/poddecoration/patch.go index 63223381..6c7382cc 100644 --- a/pkg/controllers/utils/poddecoration/patch.go +++ b/pkg/controllers/utils/poddecoration/patch.go @@ -17,6 +17,8 @@ limitations under the License. package poddecoration import ( + "sort" + corev1 "k8s.io/api/core/v1" "kusionstack.io/operating/pkg/utils" @@ -42,7 +44,7 @@ func PatchPodDecoration(pod *corev1.Pod, template *appsv1alpha1.PodDecorationPod } if len(template.Volumes) > 0 { - pod.Spec.Volumes = patch.MergeVolumes(pod.Spec.Volumes, template.Volumes) + pod.Spec.Volumes = patch.MergeWithOverwriteVolumes(pod.Spec.Volumes, template.Volumes) } if template.Affinity != nil { @@ -50,14 +52,19 @@ func PatchPodDecoration(pod *corev1.Pod, template *appsv1alpha1.PodDecorationPod } if template.Tolerations != nil { - pod.Spec.Tolerations = patch.MergeTolerations(pod.Spec.Tolerations, template.Tolerations) + pod.Spec.Tolerations = patch.MergeWithOverwriteTolerations(pod.Spec.Tolerations, template.Tolerations) } return } func PatchListOfDecorations(pod *corev1.Pod, podDecorations map[string]*appsv1alpha1.PodDecoration) (err error) { + var pds []*appsv1alpha1.PodDecoration for _, pd := range podDecorations { - if patchErr := PatchPodDecoration(pod, &pd.Spec.Template); patchErr != nil { + pds = append(pds, pd) + } + sort.Sort(PodDecorations(pds)) + for i := range pds { + if patchErr := PatchPodDecoration(pod, &pds[i].Spec.Template); patchErr != nil { err = utils.Join(err, patchErr) } } diff --git a/pkg/controllers/utils/poddecoration/patch/affinity.go b/pkg/controllers/utils/poddecoration/patch/affinity.go index 8a3d7f36..23e5063b 100644 --- a/pkg/controllers/utils/poddecoration/patch/affinity.go +++ b/pkg/controllers/utils/poddecoration/patch/affinity.go @@ -57,3 +57,21 @@ func MergeTolerations(original []corev1.Toleration, additional []corev1.Tolerati } return original } + +func MergeWithOverwriteTolerations(original []corev1.Toleration, additional []corev1.Toleration) []corev1.Toleration { + additionalMap := map[string]*corev1.Toleration{} + for i, toleration := range additional { + additionalMap[toleration.Key] = &additional[i] + } + for i, toleration := range original { + rep, ok := additionalMap[toleration.Key] + if ok { + original[i] = *rep + delete(additionalMap, toleration.Key) + } + } + for _, add := range additionalMap { + original = append(original, *add) + } + return original +} diff --git a/pkg/controllers/utils/poddecoration/patch/container.go b/pkg/controllers/utils/poddecoration/patch/container.go index 9dadc40e..3b2cbec4 100644 --- a/pkg/controllers/utils/poddecoration/patch/container.go +++ b/pkg/controllers/utils/poddecoration/patch/container.go @@ -45,6 +45,7 @@ func PrimaryContainerPatch(pod *corev1.Pod, patchs []*appsv1alpha1.PrimaryContai for idx := range pod.Spec.Containers { if patch.Name != nil && pod.Spec.Containers[idx].Name == *patch.Name { patchContainer(&pod.Spec.Containers[idx], &patchs[i].PodDecorationPrimaryContainer) + break } } case appsv1alpha1.InjectAllContainers: diff --git a/pkg/controllers/utils/poddecoration/patch_test.go b/pkg/controllers/utils/poddecoration/patch_test.go index e5271a36..2ebdb574 100644 --- a/pkg/controllers/utils/poddecoration/patch_test.go +++ b/pkg/controllers/utils/poddecoration/patch_test.go @@ -344,9 +344,7 @@ var _ = Describe("PodDecoration controller", func() { It("test anno utils", func() { pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - appsv1alpha1.AnnotationResourceDecorationRevision: "", - }, + Annotations: map[string]string{}, Labels: map[string]string{ "app": "foo", }, @@ -354,6 +352,7 @@ var _ = Describe("PodDecoration controller", func() { } i0Int32 := int32(0) i1Int32 := int32(1) + i2Int32 := int32(3) pdA := &appsv1alpha1.PodDecoration{ ObjectMeta: metav1.ObjectMeta{ Name: "pd-a", @@ -408,12 +407,12 @@ var _ = Describe("PodDecoration controller", func() { } pdC := &appsv1alpha1.PodDecoration{ ObjectMeta: metav1.ObjectMeta{ - Name: "pd-b", + Name: "pd-c", }, Spec: appsv1alpha1.PodDecorationSpec{ InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ Group: "group-b", - Weight: &i1Int32, + Weight: &i2Int32, }, Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ @@ -432,21 +431,53 @@ var _ = Describe("PodDecoration controller", func() { UpdatedRevision: "102", }, } + pdD := &appsv1alpha1.PodDecoration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pd-d", + }, + Spec: appsv1alpha1.PodDecorationSpec{ + InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ + Weight: &i2Int32, + }, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "foo", + }, + }, + UpdateStrategy: appsv1alpha1.PodDecorationUpdateStrategy{ + RollingUpdate: &appsv1alpha1.PodDecorationRollingUpdate{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "id": "1", + }, + }, + }, + }, + Template: appsv1alpha1.PodDecorationPodTemplate{ + Metadata: []*appsv1alpha1.PodDecorationPodTemplateMeta{ + { + Labels: map[string]string{"inj": "group-b-new-1"}, + }, + }, + }, + }, + Status: appsv1alpha1.PodDecorationStatus{ + CurrentRevision: "201", + UpdatedRevision: "202", + }, + } pds := map[string]*appsv1alpha1.PodDecoration{ "100": pdA, "101": pdB, } - Expect(ShouldUpdateDecorationInfo(pod, pds)).Should(BeTrue()) Expect(PatchListOfDecorations(pod, pds)).Should(BeNil()) - Expect(len(GetPodEffectiveDecorations(pod, []*appsv1alpha1.PodDecoration{pdA, pdC}, pds))).Should(Equal(2)) - Expect(GetDecorationGroupRevisionInfo(pod).Size()).Should(Equal(2)) + Expect(GetDecorationRevisionInfo(pod).Size()).Should(Equal(2)) appsv1alpha1.SchemeBuilder.AddToScheme(scheme.Scheme) - _, _, err := GetPodDecorationsByPodAnno(context.TODO(), &mockClient{}, pod) - Expect(err).ShouldNot(HaveOccurred()) - - hav, err := GetHeaviestPDByGroup(context.TODO(), &mockClient{}, "", "") - Expect(err).ShouldNot(HaveOccurred()) - Expect(hav.Name).Should(Equal("pd-d")) + updatedRevisions, stableRevisions := GetEffectiveRevisionsFormLatestDecorations([]*appsv1alpha1.PodDecoration{pdA, pdB, pdC, pdD}, map[string]string{ + "app": "foo", + }) + Expect(updatedRevisions.Len()).Should(Equal(2)) + Expect(stableRevisions.Len()).Should(Equal(1)) }) }) diff --git a/pkg/controllers/utils/poddecoration/sort.go b/pkg/controllers/utils/poddecoration/sort.go index 39849b56..014f4ea0 100644 --- a/pkg/controllers/utils/poddecoration/sort.go +++ b/pkg/controllers/utils/poddecoration/sort.go @@ -17,14 +17,7 @@ limitations under the License. package poddecoration import ( - "context" - "sort" - - "k8s.io/apimachinery/pkg/fields" - "sigs.k8s.io/controller-runtime/pkg/client" - appsv1alpha1 "kusionstack.io/operating/apis/apps/v1alpha1" - "kusionstack.io/operating/pkg/utils/inject" ) type PodDecorations []*appsv1alpha1.PodDecoration @@ -34,45 +27,13 @@ func (br PodDecorations) Len() int { } func (br PodDecorations) Less(i, j int) bool { - if br[i].Spec.InjectStrategy.Group == br[j].Spec.InjectStrategy.Group { - if *br[i].Spec.InjectStrategy.Weight == *br[j].Spec.InjectStrategy.Weight { - return br[i].CreationTimestamp.After(br[j].CreationTimestamp.Time) - } - return *br[i].Spec.InjectStrategy.Weight > *br[j].Spec.InjectStrategy.Weight - } - return br[i].Spec.InjectStrategy.Group < br[j].Spec.InjectStrategy.Group + return lessPD(br[i], br[j]) } func (br PodDecorations) Swap(i, j int) { br[i], br[j] = br[j], br[i] } -func BuildSortedPodDecorationPointList(list *appsv1alpha1.PodDecorationList) []*appsv1alpha1.PodDecoration { - res := PodDecorations{} - for i := range list.Items { - res = append(res, &list.Items[i]) - } - sort.Sort(res) - return res -} - -func GetHeaviestPDByGroup(ctx context.Context, c client.Client, namespace, group string) (heaviest *appsv1alpha1.PodDecoration, err error) { - pdList := &appsv1alpha1.PodDecorationList{} - if err = c.List(ctx, pdList, - &client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector( - inject.FieldIndexPodDecorationGroup, group), - Namespace: namespace, - }); err != nil { - return - } - podDecorations := BuildSortedPodDecorationPointList(pdList) - if len(podDecorations) > 0 { - return podDecorations[0], nil - } - return -} - func lessPD(a, b *appsv1alpha1.PodDecoration) bool { if a.Spec.InjectStrategy.Group == b.Spec.InjectStrategy.Group { if *a.Spec.InjectStrategy.Weight == *b.Spec.InjectStrategy.Weight { diff --git a/pkg/utils/inject/inject.go b/pkg/utils/inject/inject.go index 0ed6368c..de12d885 100644 --- a/pkg/utils/inject/inject.go +++ b/pkg/utils/inject/inject.go @@ -33,7 +33,6 @@ import ( const ( FieldIndexOwnerRefUID = "ownerRefUID" FieldIndexPodTransitionRule = "podTransitionRuleIndex" - FieldIndexPodDecorationGroup = "podDecorationGroup" FieldIndexPodDecorationCollaSets = "podDecorationCollaSets" ) @@ -82,14 +81,6 @@ func NewCacheWithFieldIndex(config *rest.Config, opts cache.Options) (cache.Cach return obj.(*appsv1alpha1.PodTransitionRule).Status.Targets })) - runtime.Must(c.IndexField( - context.TODO(), - &appsv1alpha1.PodDecoration{}, - FieldIndexPodDecorationGroup, - func(obj client.Object) []string { - return []string{obj.(*appsv1alpha1.PodDecoration).Spec.InjectStrategy.Group} - })) - runtime.Must(c.IndexField( context.TODO(), &appsv1alpha1.PodDecoration{}, diff --git a/pkg/webhook/server/generic/poddecoration/poddecoration_mutating_handler.go b/pkg/webhook/server/generic/poddecoration/poddecoration_mutating_handler.go index b1d0e01c..92d10dfb 100644 --- a/pkg/webhook/server/generic/poddecoration/poddecoration_mutating_handler.go +++ b/pkg/webhook/server/generic/poddecoration/poddecoration_mutating_handler.go @@ -65,9 +65,6 @@ func SetDefaultPodDecoration(pd *appsv1alpha1.PodDecoration) { var int32Zero int32 pd.Spec.InjectStrategy.Weight = &int32Zero } - if pd.Spec.InjectStrategy.Group == "" { - pd.Spec.InjectStrategy.Group = "default" - } for i := range pd.Spec.Template.Metadata { if pd.Spec.Template.Metadata[i].PatchPolicy == "" { pd.Spec.Template.Metadata[i].PatchPolicy = appsv1alpha1.RetainMetadata diff --git a/pkg/webhook/server/generic/poddecoration/poddecoration_validating_handler.go b/pkg/webhook/server/generic/poddecoration/poddecoration_validating_handler.go index 569023d8..4c2802a2 100644 --- a/pkg/webhook/server/generic/poddecoration/poddecoration_validating_handler.go +++ b/pkg/webhook/server/generic/poddecoration/poddecoration_validating_handler.go @@ -18,11 +18,18 @@ package poddecoration import ( "context" - "fmt" "net/http" + "path" + "path/filepath" + "strings" admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/apis/core" + k8scorev1 "k8s.io/kubernetes/pkg/apis/core/v1" + corevalidation "k8s.io/kubernetes/pkg/apis/core/validation" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -58,11 +65,157 @@ func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) ( return admission.Allowed("") } +var ( + defaultValidationOptions = corevalidation.PodValidationOptions{ + AllowDownwardAPIHugePages: true, + AllowInvalidPodDeletionCost: true, + AllowIndivisibleHugePagesValues: true, + AllowWindowsHostProcessField: true, + AllowExpandedDNSConfig: true, + } +) + func ValidatePodDecoration(pd *appsv1alpha1.PodDecoration) error { - for _, container := range pd.Spec.Template.PrimaryContainers { - if container.TargetPolicy == appsv1alpha1.InjectByName && container.Name == nil { - return fmt.Errorf("invalid primaryContainers.ByName, target name cannot be nil") + allErrs := field.ErrorList{} + specPath := field.NewPath("spec") + allErrs = append(allErrs, ValidateTemplate(&pd.Spec.Template, specPath.Child("template"))...) + return allErrs.ToAggregate() +} + +func ValidateTemplate(template *appsv1alpha1.PodDecorationPodTemplate, fldPath *field.Path) (allErrs field.ErrorList) { + allErrs = append(allErrs, ValidatePrimaryContainers(template.PrimaryContainers, fldPath.Child("primaryContainers"))...) + allErrs = append(allErrs, ValidatePodDecorationPodTemplateMeta(template.Metadata, fldPath.Child("metadata"))...) + allErrs = append(allErrs, ValidateContainers(template.InitContainers, fldPath.Child("initContainers"))...) + allErrs = append(allErrs, ValidateVolumes(template.Volumes, fldPath.Child("volumes"))...) + allErrs = append(allErrs, ValidateTolerations(template.Tolerations, fldPath.Child("tolerations"))...) + return +} + +func ValidateTolerations(tolerations []corev1.Toleration, fldPath *field.Path) (allErrs field.ErrorList) { + var coreTolerations []core.Toleration + for i := range tolerations { + idxPath := fldPath.Index(i) + coreToleration := &core.Toleration{} + if err := k8scorev1.Convert_v1_Toleration_To_core_Toleration(&tolerations[i], coreToleration, nil); err != nil { + allErrs = append(allErrs, field.InternalError(idxPath, err)) + } + coreTolerations = append(coreTolerations, *coreToleration) + } + + allErrs = append(allErrs, corevalidation.ValidateTolerations(coreTolerations, fldPath)...) + return +} + +func ValidateVolumes(volumes []corev1.Volume, fldPath *field.Path) (allErrs field.ErrorList) { + var coreVolumes []core.Volume + for i := range volumes { + idxPath := fldPath.Index(i) + coreVolume := &core.Volume{} + if err := k8scorev1.Convert_v1_Volume_To_core_Volume(&volumes[i], coreVolume, nil); err != nil { + allErrs = append(allErrs, field.InternalError(idxPath, err)) + } + coreVolumes = append(coreVolumes, *coreVolume) + } + _, errs := corevalidation.ValidateVolumes(coreVolumes, nil, fldPath, defaultValidationOptions) + allErrs = append(allErrs, errs...) + return +} + +func ValidateContainers(containers []*corev1.Container, fldPath *field.Path) (allErrs field.ErrorList) { + for i, c := range containers { + coreContainer := &core.Container{} + idxPath := fldPath.Index(i) + if err := k8scorev1.Convert_v1_Container_To_core_Container(c, coreContainer, nil); err != nil { + allErrs = append(allErrs, field.InternalError(idxPath, err)) + } + // TODO: validate containers + } + return +} + +func ValidatePodDecorationPodTemplateMeta(meta []*appsv1alpha1.PodDecorationPodTemplateMeta, fldPath *field.Path) (allErrs field.ErrorList) { + for i, m := range meta { + idxPath := fldPath.Index(i) + if m.PatchPolicy == appsv1alpha1.MergePatchJsonMetadata && m.Labels != nil { + allErrs = append(allErrs, field.Invalid(idxPath.Child("labels"), m.Labels, "patchPolicy MergePatchJson is only effective for annotations")) + } + allErrs = append(allErrs, corevalidation.ValidateAnnotations(m.Annotations, idxPath.Child("annotations"))...) + } + return +} + +func ValidatePrimaryContainers(containers []*appsv1alpha1.PrimaryContainerPatch, fldPath *field.Path) (allErrs field.ErrorList) { + for idx, container := range containers { + allErrs = append(allErrs, ValidatePrimaryContainer(container, fldPath.Index(idx))...) + } + return +} + +func ValidatePrimaryContainer(container *appsv1alpha1.PrimaryContainerPatch, fldPath *field.Path) (allErrs field.ErrorList) { + if container.TargetPolicy == appsv1alpha1.InjectByName && container.Name == nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), nil, "target name cannot be empty if targetPolicy=ByName")) + } + vmPatch := fldPath.Child("volumeMounts") + for i, vm := range container.VolumeMounts { + idxPath := vmPatch.Index(i) + if len(vm.Name) == 0 { + allErrs = append(allErrs, field.Required(idxPath.Child("name"), "")) + } + if len(vm.MountPath) == 0 { + allErrs = append(allErrs, field.Required(idxPath.Child("mountPath"), "")) + } + if len(vm.SubPath) > 0 { + allErrs = append(allErrs, validateLocalDescendingPath(vm.SubPath, idxPath.Child("subPath"))...) + } + if len(vm.SubPathExpr) > 0 { + if len(vm.SubPath) > 0 { + allErrs = append(allErrs, field.Invalid(idxPath.Child("subPathExpr"), vm.SubPathExpr, "subPathExpr and subPath are mutually exclusive")) + } + allErrs = append(allErrs, validateLocalDescendingPath(vm.SubPathExpr, idxPath.Child("subPathExpr"))...) + } + } + //type []"k8s.io/api/core/v1".EnvVar) as the type []"k8s.io/kubernetes/pkg/apis/core + if len(container.Env) > 0 { + var coreEnvs []core.EnvVar + for i, env := range container.Env { + coreEnv := &core.EnvVar{} + if err := k8scorev1.Convert_v1_EnvVar_To_core_EnvVar(env.DeepCopy(), coreEnv, nil); err != nil { + allErrs = append(allErrs, field.InternalError(fldPath.Child("env").Index(i), err)) + } + coreEnvs = append(coreEnvs, *coreEnv) + } + allErrs = append(allErrs, + corevalidation.ValidateEnv(coreEnvs, fldPath.Child("env"), defaultValidationOptions)...) + } + return +} + +// This validate will make sure targetPath: +// 1. is not abs path +// 2. does not have any element which is ".." +func validateLocalDescendingPath(targetPath string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if path.IsAbs(targetPath) { + allErrs = append(allErrs, field.Invalid(fldPath, targetPath, "must be a relative path")) + } + + allErrs = append(allErrs, validatePathNoBacksteps(targetPath, fldPath)...) + + return allErrs +} + +// validatePathNoBacksteps makes sure the targetPath does not have any `..` path elements when split +// +// This assumes the OS of the apiserver and the nodes are the same. The same check should be done +// on the node to ensure there are no backsteps. +func validatePathNoBacksteps(targetPath string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + parts := strings.Split(filepath.ToSlash(targetPath), "/") + for _, item := range parts { + if item == ".." { + allErrs = append(allErrs, field.Invalid(fldPath, targetPath, "must not contain '..'")) + break // even for `../../..`, one error is sufficient to make the point } } - return nil + return allErrs } diff --git a/pkg/webhook/server/generic/poddecoration/poddecoration_webhook_test.go b/pkg/webhook/server/generic/poddecoration/poddecoration_webhook_test.go index f8bff630..4cee9046 100644 --- a/pkg/webhook/server/generic/poddecoration/poddecoration_webhook_test.go +++ b/pkg/webhook/server/generic/poddecoration/poddecoration_webhook_test.go @@ -41,6 +41,50 @@ var _ = Describe("PodDecoration webhook", func() { }, } Expect(ValidatePodDecoration(pd)).Should(HaveOccurred()) + pd = &appsv1alpha1.PodDecoration{ + Spec: appsv1alpha1.PodDecorationSpec{ + Template: appsv1alpha1.PodDecorationPodTemplate{ + Volumes: []corev1.Volume{ + { + Name: "", + VolumeSource: corev1.VolumeSource{}, + }, + { + Name: "aaa", + VolumeSource: corev1.VolumeSource{}, + }, + }, + }, + }, + } + Expect(ValidatePodDecoration(pd)).Should(HaveOccurred()) + pd = &appsv1alpha1.PodDecoration{ + Spec: appsv1alpha1.PodDecorationSpec{ + Template: appsv1alpha1.PodDecorationPodTemplate{ + InitContainers: []*corev1.Container{ + { + Name: "foo", + Image: "nginx:v1", + }, + }, + }, + }, + } + Expect(ValidatePodDecoration(pd)).ShouldNot(HaveOccurred()) + pd = &appsv1alpha1.PodDecoration{ + Spec: appsv1alpha1.PodDecorationSpec{ + Template: appsv1alpha1.PodDecorationPodTemplate{ + Tolerations: []corev1.Toleration{ + { + Key: "", + Operator: corev1.TolerationOpExists, + Value: "foo", + }, + }, + }, + }, + } + Expect(ValidatePodDecoration(pd)).Should(HaveOccurred()) }) It("test mutating", func() { pd := &appsv1alpha1.PodDecoration{