diff --git a/pkg/controller/batchrelease/context/context.go b/pkg/controller/batchrelease/context/context.go index 6ad325b0..f81b7784 100644 --- a/pkg/controller/batchrelease/context/context.go +++ b/pkg/controller/batchrelease/context/context.go @@ -32,6 +32,8 @@ type BatchContext struct { CurrentBatch int32 `json:"currentBatchIndex"` // workload update revision UpdateRevision string `json:"updateRevision,omitempty"` + // stable revision + StableRevision string `json:"stableRevision,omitempty"` // workload replicas Replicas int32 `json:"replicas"` diff --git a/pkg/controller/batchrelease/control/canarystyle/control_plane.go b/pkg/controller/batchrelease/control/canarystyle/control_plane.go index dcef1061..45da6582 100644 --- a/pkg/controller/batchrelease/control/canarystyle/control_plane.go +++ b/pkg/controller/batchrelease/control/canarystyle/control_plane.go @@ -103,11 +103,19 @@ func (rc *realCanaryController) UpgradeBatch() error { return fmt.Errorf("wait canary workload %v reconcile", canary.GetCanaryInfo().LogKey) } - batchContext := rc.CalculateBatchContext(rc.release) + batchContext, err := rc.CalculateBatchContext(rc.release) + if err != nil { + return err + } klog.Infof("BatchRelease %v calculated context when upgrade batch: %s", klog.KObj(rc.release), batchContext.Log()) - return canary.UpgradeBatch(batchContext) + err = canary.UpgradeBatch(batchContext) + if err != nil { + return err + } + + return rc.patcher.PatchPodBatchLabel(batchContext) } func (rc *realCanaryController) CheckBatchReady() error { @@ -129,7 +137,10 @@ func (rc *realCanaryController) CheckBatchReady() error { return fmt.Errorf("wait canary workload %v reconcile", canary.GetCanaryInfo().LogKey) } - batchContext := rc.CalculateBatchContext(rc.release) + batchContext, err := rc.CalculateBatchContext(rc.release) + if err != nil { + return err + } klog.Infof("BatchRelease %v calculated context when check batch ready: %s", klog.KObj(rc.release), batchContext.Log()) diff --git a/pkg/controller/batchrelease/control/canarystyle/deployment/canary.go b/pkg/controller/batchrelease/control/canarystyle/deployment/canary.go index 88987af2..80f15c08 100644 --- a/pkg/controller/batchrelease/control/canarystyle/deployment/canary.go +++ b/pkg/controller/batchrelease/control/canarystyle/deployment/canary.go @@ -43,6 +43,7 @@ type realCanaryController struct { canaryObject *apps.Deployment canaryClient client.Client objectKey types.NamespacedName + canaryPods []*corev1.Pod } func newCanary(cli client.Client, key types.NamespacedName) realCanaryController { diff --git a/pkg/controller/batchrelease/control/canarystyle/deployment/control.go b/pkg/controller/batchrelease/control/canarystyle/deployment/control.go index c0f61670..a806201e 100644 --- a/pkg/controller/batchrelease/control/canarystyle/deployment/control.go +++ b/pkg/controller/batchrelease/control/canarystyle/deployment/control.go @@ -27,6 +27,7 @@ import ( "github.com/openkruise/rollouts/pkg/util" utilclient "github.com/openkruise/rollouts/pkg/util/client" apps "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -82,19 +83,40 @@ func (rc *realController) BuildCanaryController(release *v1beta1.BatchRelease) ( return rc, nil } -func (rc *realController) CalculateBatchContext(release *v1beta1.BatchRelease) *batchcontext.BatchContext { +func (rc *realController) CalculateBatchContext(release *v1beta1.BatchRelease) (*batchcontext.BatchContext, error) { + rolloutID := release.Spec.ReleasePlan.RolloutID + if rolloutID != "" { + // if rollout-id is set, the pod will be patched batch label, + // so we have to list pod here. + if _, err := rc.ListOwnedPods(); err != nil { + return nil, err + } + } replicas := *rc.stableObject.Spec.Replicas currentBatch := release.Status.CanaryStatus.CurrentBatch desiredUpdate := int32(control.CalculateBatchReplicas(release, int(replicas), int(currentBatch))) return &batchcontext.BatchContext{ + Pods: rc.canaryPods, + RolloutID: rolloutID, Replicas: replicas, + UpdateRevision: release.Status.UpdateRevision, + StableRevision: release.Status.StableRevision, CurrentBatch: currentBatch, DesiredUpdatedReplicas: desiredUpdate, FailureThreshold: release.Spec.ReleasePlan.FailureThreshold, UpdatedReplicas: rc.canaryObject.Status.Replicas, UpdatedReadyReplicas: rc.canaryObject.Status.AvailableReplicas, - } + FilterFunc: func(pods []*v1.Pod, ctx *batchcontext.BatchContext) []*v1.Pod { + filteredPods := make([]*v1.Pod, 0, len(pods)) + for i := range pods { + if !util.IsConsistentWithRevision(pods[i], ctx.StableRevision) { + filteredPods = append(filteredPods, pods[i]) + } + } + return filteredPods + }, + }, nil } func (rc *realController) getLatestTemplate() (*v1.PodTemplateSpec, error) { @@ -104,3 +126,12 @@ func (rc *realController) getLatestTemplate() (*v1.PodTemplateSpec, error) { } return &rc.stableObject.Spec.Template, nil } + +func (rc *realController) ListOwnedPods() ([]*corev1.Pod, error) { + if rc.canaryPods != nil { + return rc.canaryPods, nil + } + var err error + rc.canaryPods, err = util.ListOwnedPods(rc.canaryClient, rc.canaryObject) + return rc.canaryPods, err +} diff --git a/pkg/controller/batchrelease/control/canarystyle/deployment/control_test.go b/pkg/controller/batchrelease/control/canarystyle/deployment/control_test.go index 08e2c76c..a3cacb2b 100644 --- a/pkg/controller/batchrelease/control/canarystyle/deployment/control_test.go +++ b/pkg/controller/batchrelease/control/canarystyle/deployment/control_test.go @@ -226,7 +226,9 @@ func TestCalculateBatchContext(t *testing.T) { canaryObject: canary, }, } - got := control.CalculateBatchContext(cs.release()) + got, err := control.CalculateBatchContext(cs.release()) + got.FilterFunc = nil + Expect(err).NotTo(HaveOccurred()) Expect(reflect.DeepEqual(got, cs.result)).Should(BeTrue()) }) } @@ -290,7 +292,8 @@ func TestRealCanaryController(t *testing.T) { Expect(util.EqualIgnoreHash(&c.canaryObject.Spec.Template, &deployment.Spec.Template)).Should(BeTrue()) // check rolling - batchContext := c.CalculateBatchContext(release) + batchContext, err := c.CalculateBatchContext(release) + Expect(err).NotTo(HaveOccurred()) err = controller.UpgradeBatch(batchContext) Expect(err).NotTo(HaveOccurred()) canary := getCanaryDeployment(release, deployment, c) diff --git a/pkg/controller/batchrelease/control/canarystyle/interface.go b/pkg/controller/batchrelease/control/canarystyle/interface.go index f2585446..e7155ead 100644 --- a/pkg/controller/batchrelease/control/canarystyle/interface.go +++ b/pkg/controller/batchrelease/control/canarystyle/interface.go @@ -33,7 +33,7 @@ type Interface interface { BuildCanaryController(release *v1beta1.BatchRelease) (CanaryInterface, error) // CalculateBatchContext calculate the current batch context according to // our release plan and the statues of stable workload and canary workload. - CalculateBatchContext(release *v1beta1.BatchRelease) *batchcontext.BatchContext + CalculateBatchContext(release *v1beta1.BatchRelease) (*batchcontext.BatchContext, error) } // CanaryInterface contains the methods about canary workload diff --git a/pkg/controller/batchrelease/control/partitionstyle/cloneset/control.go b/pkg/controller/batchrelease/control/partitionstyle/cloneset/control.go index b136b104..772ed1b8 100644 --- a/pkg/controller/batchrelease/control/partitionstyle/cloneset/control.go +++ b/pkg/controller/batchrelease/control/partitionstyle/cloneset/control.go @@ -182,6 +182,15 @@ func (rc *realController) CalculateBatchContext(release *v1beta1.BatchRelease) ( NoNeedUpdatedReplicas: noNeedUpdate, PlannedUpdatedReplicas: plannedUpdate, DesiredUpdatedReplicas: desiredUpdate, + FilterFunc: func(pods []*corev1.Pod, ctx *batchcontext.BatchContext) []*corev1.Pod { + filteredPods := make([]*corev1.Pod, 0, len(pods)) + for i := range pods { + if util.IsConsistentWithRevision(pods[i], ctx.UpdateRevision) { + filteredPods = append(filteredPods, pods[i]) + } + } + return filteredPods + }, } if noNeedUpdate != nil { diff --git a/pkg/controller/batchrelease/control/partitionstyle/daemonset/control.go b/pkg/controller/batchrelease/control/partitionstyle/daemonset/control.go index d1f03885..bd049441 100644 --- a/pkg/controller/batchrelease/control/partitionstyle/daemonset/control.go +++ b/pkg/controller/batchrelease/control/partitionstyle/daemonset/control.go @@ -53,7 +53,7 @@ func (rc *realController) BuildController() (partitionstyle.Interface, error) { } rc.object = object - //update this function + // update this function rc.WorkloadInfo = util.ParseWorkload(object) // for Advanced DaemonSet which has no updatedReadyReplicas field, we should @@ -189,6 +189,15 @@ func (rc *realController) CalculateBatchContext(release *v1beta1.BatchRelease) ( NoNeedUpdatedReplicas: noNeedUpdate, PlannedUpdatedReplicas: plannedUpdate, DesiredUpdatedReplicas: desiredUpdate, + FilterFunc: func(pods []*corev1.Pod, ctx *batchcontext.BatchContext) []*corev1.Pod { + filteredPods := make([]*corev1.Pod, 0, len(pods)) + for i := range pods { + if util.IsConsistentWithRevision(pods[i], ctx.UpdateRevision) { + filteredPods = append(filteredPods, pods[i]) + } + } + return filteredPods + }, } if noNeedUpdate != nil { diff --git a/pkg/controller/batchrelease/control/partitionstyle/deployment/control.go b/pkg/controller/batchrelease/control/partitionstyle/deployment/control.go index 94e8542d..8f17b470 100644 --- a/pkg/controller/batchrelease/control/partitionstyle/deployment/control.go +++ b/pkg/controller/batchrelease/control/partitionstyle/deployment/control.go @@ -175,6 +175,7 @@ func (rc *realController) CalculateBatchContext(release *v1beta1.BatchRelease) ( RolloutID: rolloutID, CurrentBatch: currentBatch, UpdateRevision: release.Status.UpdateRevision, + StableRevision: release.Status.StableRevision, DesiredPartition: desiredPartition, FailureThreshold: release.Spec.ReleasePlan.FailureThreshold, @@ -183,6 +184,15 @@ func (rc *realController) CalculateBatchContext(release *v1beta1.BatchRelease) ( UpdatedReadyReplicas: rc.Status.UpdatedReadyReplicas, PlannedUpdatedReplicas: PlannedUpdatedReplicas, DesiredUpdatedReplicas: PlannedUpdatedReplicas, + FilterFunc: func(pods []*corev1.Pod, ctx *batchcontext.BatchContext) []*corev1.Pod { + filteredPods := make([]*corev1.Pod, 0, len(pods)) + for i := range pods { + if util.IsConsistentWithRevision(pods[i], ctx.UpdateRevision) { + filteredPods = append(filteredPods, pods[i]) + } + } + return filteredPods + }, }, nil } diff --git a/pkg/controller/batchrelease/control/partitionstyle/statefulset/control.go b/pkg/controller/batchrelease/control/partitionstyle/statefulset/control.go index 8d83a559..67a776b4 100644 --- a/pkg/controller/batchrelease/control/partitionstyle/statefulset/control.go +++ b/pkg/controller/batchrelease/control/partitionstyle/statefulset/control.go @@ -196,6 +196,15 @@ func (rc *realController) CalculateBatchContext(release *v1beta1.BatchRelease) ( NoNeedUpdatedReplicas: noNeedUpdate, PlannedUpdatedReplicas: plannedUpdate, DesiredUpdatedReplicas: desiredUpdate, + FilterFunc: func(pods []*corev1.Pod, ctx *batchcontext.BatchContext) []*corev1.Pod { + filteredPods := make([]*corev1.Pod, 0, len(pods)) + for i := range pods { + if util.IsConsistentWithRevision(pods[i], ctx.UpdateRevision) { + filteredPods = append(filteredPods, pods[i]) + } + } + return filteredPods + }, } if noNeedUpdate != nil { diff --git a/pkg/controller/batchrelease/labelpatch/patcher.go b/pkg/controller/batchrelease/labelpatch/patcher.go index 129320ee..4f52642a 100644 --- a/pkg/controller/batchrelease/labelpatch/patcher.go +++ b/pkg/controller/batchrelease/labelpatch/patcher.go @@ -66,11 +66,6 @@ func (r *realPatcher) patchPodBatchLabel(pods []*corev1.Pod, ctx *batchcontext.B klog.InfoS("Pod is being deleted, skip patching", "pod", klog.KObj(pod), "rollout", r.logKey) continue } - // we don't patch label for the active old revision pod - if !util.IsConsistentWithRevision(pod, ctx.UpdateRevision) { - klog.InfoS("Pod is not consistent with revision, skip patching", "pod", klog.KObj(pod), "rollout", r.logKey) - continue - } if pod.Labels[v1beta1.RolloutIDLabel] != ctx.RolloutID { // for example: new/recreated pods updatedButUnpatchedPods = append(updatedButUnpatchedPods, pod)