diff --git a/pkg/controllers/collaset/collaset_controller_test.go b/pkg/controllers/collaset/collaset_controller_test.go index ece1e37c..b4564ea4 100644 --- a/pkg/controllers/collaset/collaset_controller_test.go +++ b/pkg/controllers/collaset/collaset_controller_test.go @@ -3237,7 +3237,7 @@ var _ = Describe("collaset controller", func() { Expect(err).Should(BeNil()) } return podDecoration.Status.UpdatedPods - }, 300*time.Second, 3*time.Second).Should(BeEquivalentTo(2)) + }, 30*time.Second, 3*time.Second).Should(BeEquivalentTo(2)) Expect(expectedStatusReplicas(c, cs, 0, 0, 0, 2, 2, 0, 0, 0)).Should(BeNil()) Expect(c.List(context.TODO(), podList, client.InNamespace(cs.Namespace))).Should(BeNil()) @@ -3276,6 +3276,115 @@ var _ = Describe("collaset controller", func() { return errors.IsNotFound(err) }, 5*time.Second, 1*time.Second).Should(BeTrue()) }) + + It("pod create failed with PodDecoration", func() { + testcase := "test-pod-create-failed-with-poddecoration" + Expect(createNamespace(c, testcase)).Should(BeNil()) + + // create a bad podDecoration + podDecoration := &appsv1alpha1.PodDecoration{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testcase, + Name: "pd-foo", + }, + Spec: appsv1alpha1.PodDecorationSpec{ + HistoryLimit: 5, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "foo", + }, + }, + Weight: int32Pointer(1), + UpdateStrategy: appsv1alpha1.PodDecorationUpdateStrategy{ + RollingUpdate: &appsv1alpha1.PodDecorationRollingUpdate{ + Partition: int32Pointer(0), + }, + }, + Template: appsv1alpha1.PodDecorationPodTemplate{ + Containers: []*appsv1alpha1.ContainerPatch{ + { + InjectPolicy: appsv1alpha1.AfterPrimaryContainer, + Container: corev1.Container{ + Name: "sidecar", + Image: imageutils.GetE2EImage(imageutils.Nginx), + Command: []string{ + "sleep", + "2h", + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "non-exist", + MountPath: "/non/exist", + }, + }, + }, + }, + }, + }, + }, + } + Expect(c.Create(context.TODO(), podDecoration)).Should(BeNil()) + + cs := &appsv1alpha1.CollaSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testcase, + Name: "foo", + }, + Spec: appsv1alpha1.CollaSetSpec{ + Replicas: int32Pointer(2), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "foo", + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "foo", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "foo", + Image: "nginx:v1", + }, + }, + }, + }, + }, + } + Expect(c.Create(context.TODO(), cs)).Should(BeNil()) + + // create pod failed + Eventually(func() bool { + err := c.Get(context.TODO(), types.NamespacedName{Namespace: cs.Namespace, Name: cs.Name}, cs) + if errors.IsNotFound(err) { + return false + } + Expect(err).Should(BeNil()) + + for _, cond := range cs.Status.Conditions { + if cond.Reason == "ScaleOutFailed" { + return true + } + } + return false + }, 30*time.Second, 3*time.Second).Should(BeTrue()) + + // update podDecoration with good manner + Expect(c.Get(context.TODO(), types.NamespacedName{Namespace: podDecoration.Namespace, Name: podDecoration.Name}, podDecoration)).Should(BeNil()) + podDecoration.Spec.Template.Containers[0].VolumeMounts = []corev1.VolumeMount{} + Expect(c.Update(ctx, podDecoration)).Should(BeNil()) + + // check pod create succeeded + podList := &corev1.PodList{} + Eventually(func() bool { + Expect(c.List(context.TODO(), podList, client.InNamespace(cs.Namespace))).Should(BeNil()) + return len(podList.Items) == 2 + }, 5*time.Second, 1*time.Second).Should(BeTrue()) + Expect(c.Get(context.TODO(), types.NamespacedName{Namespace: cs.Namespace, Name: cs.Name}, cs)).Should(BeNil()) + }) }) func expectedStatusReplicas(c client.Client, cls *appsv1alpha1.CollaSet, scheduledReplicas, readyReplicas, availableReplicas, replicas, updatedReplicas, operatingReplicas, diff --git a/pkg/controllers/collaset/synccontrol/sync_control.go b/pkg/controllers/collaset/synccontrol/sync_control.go index 8de959df..58b67454 100644 --- a/pkg/controllers/collaset/synccontrol/sync_control.go +++ b/pkg/controllers/collaset/synccontrol/sync_control.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "strconv" + "sync/atomic" "time" "github.com/go-logr/logr" @@ -316,12 +317,13 @@ func (r *RealSyncControl) Scale( podInstanceIDSet := collasetutils.CollectPodInstanceID(activePods) // find IDs and their contexts which have not been used by owned Pods availableContext := extractAvailableContexts(diff, ownedIDs, podInstanceIDSet) - needUpdateContext := false + needUpdateContext := atomic.Bool{} succCount, err := controllerutils.SlowStartBatch(diff, controllerutils.SlowStartInitialBatchSize, false, func(idx int, _ error) (err error) { availableIDContext := availableContext[idx] - shouldInitPDContext := false defer func() { - needUpdateContext = decideContextRevision(availableIDContext, resources.UpdatedRevision, err == nil) || shouldInitPDContext + if decideContextRevision(availableIDContext, resources.UpdatedRevision, err == nil) { + needUpdateContext.Store(true) + } }() // use revision recorded in Context revision := resources.UpdatedRevision @@ -349,7 +351,7 @@ func (r *RealSyncControl) Scale( if localErr != nil { return localErr } - shouldInitPDContext = true + needUpdateContext.Store(true) availableIDContext.Put(podcontext.PodDecorationRevisionKey, anno.GetDecorationInfoString(pds)) } else { // upgrade by recreate pod case @@ -385,7 +387,7 @@ func (r *RealSyncControl) Scale( // add an expectation for this pod creation, before next reconciling return collasetutils.ActiveExpectations.ExpectCreate(cls, expectations.Pod, pod.Name) }) - if needUpdateContext { + if needUpdateContext.Load() { logger.V(1).Info("try to update ResourceContext for CollaSet after scaling out") if updateContextErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { return podcontext.UpdateToPodContext(r.client, cls, ownedIDs) @@ -599,9 +601,11 @@ func decideContextRevision(contextDetail *appsv1alpha1.ContextDetail, updatedRev if contextDetail.Contains(podcontext.JustCreateContextDataKey, "true") { // TODO choose just create pods' revision according to scaleStrategy contextDetail.Put(podcontext.RevisionContextDataKey, updatedRevision.Name) + delete(contextDetail.Data, podcontext.PodDecorationRevisionKey) needUpdateContext = true } else if contextDetail.Contains(podcontext.RecreateUpdateContextDataKey, "true") { contextDetail.Put(podcontext.RevisionContextDataKey, updatedRevision.Name) + delete(contextDetail.Data, podcontext.PodDecorationRevisionKey) needUpdateContext = true } // if pod is delete and recreate, never change revisionKey