Skip to content

Commit 7943d8d

Browse files
zachallerzachaller
committed
fix: fallback to patch on scale conflict
Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> fix: switch to retry logic Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> lint Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> retry experiments Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> remove TODO Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> remove accidental add Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> remove accidental add Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> add retry to setting revision Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> chore(deps): bump slsa-framework/slsa-github-generator from 1.10.0 to 2.0.0 (argoproj#3537) chore(deps): bump slsa-framework/slsa-github-generator Bumps [slsa-framework/slsa-github-generator](https://github.com/slsa-framework/slsa-github-generator) from 1.10.0 to 2.0.0. - [Release notes](https://github.com/slsa-framework/slsa-github-generator/releases) - [Changelog](https://github.com/slsa-framework/slsa-github-generator/blob/main/CHANGELOG.md) - [Commits](slsa-framework/slsa-github-generator@v1.10.0...v2.0.0) --- updated-dependencies: - dependency-name: slsa-framework/slsa-github-generator dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> chore(deps): bump sigstore/cosign-installer from 3.4.0 to 3.5.0 (argoproj#3522) Bumps [sigstore/cosign-installer](https://github.com/sigstore/cosign-installer) from 3.4.0 to 3.5.0. - [Release notes](https://github.com/sigstore/cosign-installer/releases) - [Commits](sigstore/cosign-installer@e1523de...59acb62) --- updated-dependencies: - dependency-name: sigstore/cosign-installer dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> chore(deps): bump golangci/golangci-lint-action from 4 to 5 (argoproj#3540) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 4 to 5. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](golangci/golangci-lint-action@v4...v5) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> docs: provide recommendation for strategies (argoproj#3531) * docs: provide recommendation for strategies Signed-off-by: Kostis (Codefresh) <39800303+kostis-codefresh@users.noreply.github.com> * docs: traffic manager clarifications Signed-off-by: Kostis (Codefresh) <39800303+kostis-codefresh@users.noreply.github.com> * docs: explain canary with/out traffic manager Signed-off-by: Kostis (Codefresh) <39800303+kostis-codefresh@users.noreply.github.com> * docs: add 3 columns on the comparison table Signed-off-by: Kostis (Codefresh) <39800303+kostis-codefresh@users.noreply.github.com> --------- Signed-off-by: Kostis (Codefresh) <39800303+kostis-codefresh@users.noreply.github.com> feat(dashboard): change the color of the current rollout step (argoproj#3526) I feel that having the current (running) step in a orange color is misleading, as orange usually means warning. This commit changes the color to the `$argo-running-color`. Signed-off-by: Alejandro López Sánchez <alejandro.lopez@factorial.co> chore(deps): bump github.com/aws/aws-sdk-go-v2/service/cloudwatch from 1.37.0 to 1.38.0 (argoproj#3525) chore(deps): bump github.com/aws/aws-sdk-go-v2/service/cloudwatch Bumps [github.com/aws/aws-sdk-go-v2/service/cloudwatch](https://github.com/aws/aws-sdk-go-v2) from 1.37.0 to 1.38.0. - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Changelog](https://github.com/aws/aws-sdk-go-v2/blob/service/s3/v1.38.0/CHANGELOG.md) - [Commits](aws/aws-sdk-go-v2@service/s3/v1.37.0...service/s3/v1.38.0) --- updated-dependencies: - dependency-name: github.com/aws/aws-sdk-go-v2/service/cloudwatch dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> perform all of set revision actions on retry Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> fix variable Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> add retry counts to log Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> add retry counts to logs Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> clean logs, always dump controller e2e logs Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> lower timeout Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> bump timeout on e2e Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> retry on rollout conflict Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> don't reque on rs changes Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> reque rs Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> bump qps for e2e Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> fix gen-crd Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> switch to patch Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> switch to patch Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> add log Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> move log lines Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> Trigger Build Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> fix one e2e test Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> lint Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> add test Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> chore(deps): bump actions/setup-go from 5.0.0 to 5.0.1 (argoproj#3552) Bumps [actions/setup-go](https://github.com/actions/setup-go) from 5.0.0 to 5.0.1. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v5.0.0...v5.0.1) --- updated-dependencies: - dependency-name: actions/setup-go dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> chore(deps): bump codecov/codecov-action from 4.3.0 to 4.3.1 (argoproj#3550) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 4.3.0 to 4.3.1. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v4.3.0...v4.3.1) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> chore(deps): bump google.golang.org/protobuf from 1.33.0 to 1.34.0 (argoproj#3548) Bumps google.golang.org/protobuf from 1.33.0 to 1.34.0. --- updated-dependencies: - dependency-name: google.golang.org/protobuf dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> refactor Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> add test for updating rs revision Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> add retry for ephemeral metadata Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> clear some fields Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> add logs Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> refactor into function Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> change log Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> switch rollout update to patch fallback Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> siwtch ephemeral metadata sync to shared function Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> siwtch merge type Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> lint Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> don't update status Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> switch rollout update to not use patch Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> change log Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> switch to small patch Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> some cleanup Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> remove not found rollout removal Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> working setup Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> lint Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> fix test Signed-off-by: Zach Aller <zachaller@users.noreply.github.com> small cleanup Signed-off-by: Zach Aller <zachaller@users.noreply.github.com>
1 parent 125fc3d commit 7943d8d

13 files changed

Lines changed: 237 additions & 43 deletions

File tree

‎.github/workflows/e2e.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,4 @@ jobs:
9595
with:
9696
name: e2e-controller-k8s-${{ matrix.kubernetes-minor-version }}.log
9797
path: /tmp/e2e-controller.log
98-
if: ${{ failure() }}
98+
if: ${{ always() }}

‎Makefile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ DEV_IMAGE ?= false
2222
E2E_INSTANCE_ID ?= argo-rollouts-e2e
2323
E2E_TEST_OPTIONS ?=
2424
E2E_PARALLEL ?= 1
25-
E2E_WAIT_TIMEOUT ?= 120
25+
E2E_WAIT_TIMEOUT ?= 90
2626
GOPATH ?= $(shell go env GOPATH)
2727

2828
# Global toolchain configuration
@@ -239,7 +239,7 @@ start-e2e: ## start e2e test environment
239239

240240
.PHONY: test-e2e
241241
test-e2e: install-devtools-local
242-
${DIST_DIR}/gotestsum --rerun-fails-report=rerunreport.txt --junitfile=junit.xml --format=testname --packages="./test/e2e" --rerun-fails=5 -- -timeout 60m -count 1 --tags e2e -p ${E2E_PARALLEL} -parallel ${E2E_PARALLEL} -v --short ./test/e2e ${E2E_TEST_OPTIONS}
242+
${DIST_DIR}/gotestsum --rerun-fails-report=rerunreport.txt --junitfile=junit.xml --format=testname --packages="./test/e2e" --rerun-fails=5 -- -timeout 90m -count 1 --tags e2e -p ${E2E_PARALLEL} -parallel ${E2E_PARALLEL} -v --short ./test/e2e ${E2E_TEST_OPTIONS}
243243

244244
.PHONY: test-unit
245245
test-unit: install-devtools-local ## run unit tests

‎experiments/replicaset.go‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"fmt"
77
"time"
88

9+
"github.com/argoproj/argo-rollouts/utils/diff"
10+
911
log "github.com/sirupsen/logrus"
1012
appsv1 "k8s.io/api/apps/v1"
1113
"k8s.io/apimachinery/pkg/api/errors"
@@ -287,16 +289,39 @@ func (ec *experimentContext) scaleReplicaSet(rs *appsv1.ReplicaSet, newScale int
287289
sizeNeedsUpdate := oldScale != newScale
288290
scaled := false
289291
var err error
292+
var updatedRS *appsv1.ReplicaSet
290293
if sizeNeedsUpdate {
291294
rsCopy := rs.DeepCopy()
292295
*(rsCopy.Spec.Replicas) = newScale
293-
rs, err = ec.kubeclientset.AppsV1().ReplicaSets(rsCopy.Namespace).Update(ctx, rsCopy, metav1.UpdateOptions{})
296+
297+
updatedRS, err = ec.kubeclientset.AppsV1().ReplicaSets(rsCopy.Namespace).Update(ctx, rsCopy, metav1.UpdateOptions{})
298+
if err != nil {
299+
if errors.IsConflict(err) {
300+
ec.log.Infof("Conflict when updating replicaset %s, falling back to patch", rs.Name)
301+
302+
patchRS := appsv1.ReplicaSet{}
303+
patchRS.Spec.Replicas = rsCopy.Spec.Replicas
304+
305+
patch, changed, err := diff.CreateTwoWayMergePatch(appsv1.ReplicaSet{}, patchRS, appsv1.ReplicaSet{})
306+
if err != nil {
307+
return scaled, nil, err
308+
}
309+
310+
if changed {
311+
ec.log.Infof("Patching expirment replicaset with patch: %s", string(patch))
312+
updatedRS, err = ec.kubeclientset.AppsV1().ReplicaSets(rs.Namespace).Patch(ctx, rs.Name, patchtypes.StrategicMergePatchType, patch, metav1.PatchOptions{})
313+
if err != nil {
314+
return scaled, nil, err
315+
}
316+
}
317+
}
318+
}
294319
if err == nil && sizeNeedsUpdate {
295320
scaled = true
296321
ec.recorder.Eventf(ec.ex, record.EventOptions{EventReason: conditions.ScalingReplicaSetReason}, "Scaled %s ReplicaSet %s from %d to %d", scalingOperation, rs.Name, oldScale, newScale)
297322
}
298323
}
299-
return scaled, rs, err
324+
return scaled, updatedRS, err
300325
}
301326

302327
func newReplicaSetAnnotations(experimentName, templateName string) map[string]string {

‎hack/gen-crd-spec/main.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,12 @@ func NewCustomResourceDefinition() []*extensionsobj.CustomResourceDefinition {
9696
// clean up stuff left by controller-gen
9797
deleteFile("config/webhook/manifests.yaml")
9898
deleteFile("config/webhook")
99-
deleteFile("config/argoproj.io_analysisruns.yaml")
100-
deleteFile("config/argoproj.io_analysistemplates.yaml")
101-
deleteFile("config/argoproj.io_clusteranalysistemplates.yaml")
102-
deleteFile("config/argoproj.io_experiments.yaml")
103-
deleteFile("config/argoproj.io_rollouts.yaml")
99+
deleteFile("config/crd/argoproj.io_analysisruns.yaml")
100+
deleteFile("config/crd/argoproj.io_analysistemplates.yaml")
101+
deleteFile("config/crd/argoproj.io_clusteranalysistemplates.yaml")
102+
deleteFile("config/crd/argoproj.io_experiments.yaml")
103+
deleteFile("config/crd/argoproj.io_rollouts.yaml")
104+
deleteFile("config/crd")
104105
deleteFile("config")
105106

106107
crds := []*extensionsobj.CustomResourceDefinition{}

‎rollout/canary.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func (c *rolloutContext) reconcileCanaryStableReplicaSet() (bool, error) {
112112
}
113113
scaled, _, err := c.scaleReplicaSetAndRecordEvent(c.stableRS, desiredStableRSReplicaCount)
114114
if err != nil {
115-
return scaled, fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in reconcileCanaryStableReplicaSet:L %w", err)
115+
return scaled, fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in reconcileCanaryStableReplicaSet: %w", err)
116116
}
117117
return scaled, err
118118
}

‎rollout/canary_test.go‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ import (
88
"testing"
99
"time"
1010

11+
"k8s.io/apimachinery/pkg/api/errors"
12+
"k8s.io/apimachinery/pkg/runtime"
13+
"k8s.io/apimachinery/pkg/runtime/schema"
14+
k8stesting "k8s.io/client-go/testing"
15+
1116
"github.com/stretchr/testify/assert"
1217
appsv1 "k8s.io/api/apps/v1"
1318
v1 "k8s.io/api/apps/v1"
@@ -2141,3 +2146,90 @@ func TestCanaryReplicaAndSpecChangedTogether(t *testing.T) {
21412146
// check the canary one is updated
21422147
assert.NotEqual(t, originReplicas, int(*updated.Spec.Replicas))
21432148
}
2149+
2150+
func TestSyncRolloutWithConflictInScaleReplicaSet(t *testing.T) {
2151+
f := newFixture(t)
2152+
defer f.Close()
2153+
2154+
steps := []v1alpha1.CanaryStep{
2155+
{
2156+
SetWeight: int32Ptr(10),
2157+
}, {
2158+
Pause: &v1alpha1.RolloutPause{
2159+
Duration: v1alpha1.DurationFromInt(10),
2160+
},
2161+
},
2162+
}
2163+
r1 := newCanaryRollout("foo", 10, nil, steps, int32Ptr(1), intstr.FromInt(1), intstr.FromInt(0))
2164+
r2 := bumpVersion(r1)
2165+
2166+
rs1 := newReplicaSetWithStatus(r1, 9, 9)
2167+
rs2 := newReplicaSetWithStatus(r2, 1, 1)
2168+
f.kubeobjects = append(f.kubeobjects, rs1, rs2)
2169+
f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)
2170+
2171+
f.rolloutLister = append(f.rolloutLister, r2)
2172+
f.objects = append(f.objects, r2)
2173+
2174+
f.expectPatchRolloutAction(r2)
2175+
f.expectUpdateReplicaSetAction(rs2) // attempt to scale replicaset but conflict
2176+
f.expectPatchReplicaSetAction(rs2) // instead of update patch replicaset
2177+
2178+
key := fmt.Sprintf("%s/%s", r2.Namespace, r2.Name)
2179+
c, i, k8sI := f.newController(func() time.Duration { return 30 * time.Minute })
2180+
2181+
f.kubeclient.PrependReactor("update", "replicasets", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
2182+
return true, &appsv1.ReplicaSet{}, errors.NewConflict(schema.GroupResource{
2183+
Group: "Apps",
2184+
Resource: "ReplicaSet",
2185+
}, "", fmt.Errorf("test error"))
2186+
})
2187+
2188+
f.runController(key, true, false, c, i, k8sI)
2189+
}
2190+
2191+
func TestSyncRolloutWithConflictInSyncReplicaSetRevision(t *testing.T) {
2192+
f := newFixture(t)
2193+
defer f.Close()
2194+
2195+
steps := []v1alpha1.CanaryStep{
2196+
{
2197+
SetWeight: int32Ptr(10),
2198+
}, {
2199+
Pause: &v1alpha1.RolloutPause{
2200+
Duration: v1alpha1.DurationFromInt(10),
2201+
},
2202+
},
2203+
}
2204+
r1 := newCanaryRollout("foo", 3, nil, steps, int32Ptr(1), intstr.FromInt(1), intstr.FromInt(0))
2205+
r2 := bumpVersion(r1)
2206+
2207+
rs1 := newReplicaSetWithStatus(r1, 3, 3)
2208+
rs2 := newReplicaSetWithStatus(r2, 3, 3)
2209+
rs2.Annotations["rollout.argoproj.io/revision"] = "1"
2210+
2211+
f.kubeobjects = append(f.kubeobjects, rs1, rs2)
2212+
f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)
2213+
2214+
f.rolloutLister = append(f.rolloutLister, r2)
2215+
f.objects = append(f.objects, r2)
2216+
2217+
key := fmt.Sprintf("%s/%s", r1.Namespace, r1.Name)
2218+
c, i, k8sI := f.newController(func() time.Duration { return 30 * time.Minute })
2219+
2220+
f.kubeclient.PrependReactor("update", "replicasets", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
2221+
return true, &appsv1.ReplicaSet{}, errors.NewConflict(schema.GroupResource{
2222+
Group: "Apps",
2223+
Resource: "ReplicaSet",
2224+
}, "", fmt.Errorf("test error"))
2225+
})
2226+
2227+
f.expectPatchRolloutAction(r2)
2228+
f.expectUpdateReplicaSetAction(rs1) // attempt to update replicaset revision but conflict
2229+
f.expectPatchReplicaSetAction(rs1) // instead of update patch replicaset
2230+
2231+
f.expectUpdateReplicaSetAction(rs2) // attempt to scale replicaset but conflict
2232+
f.expectPatchReplicaSetAction(rs2) // instead of update patch replicaset
2233+
2234+
f.runController(key, true, false, c, i, k8sI)
2235+
}

‎rollout/controller.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,21 @@ import (
99
"sync"
1010
"time"
1111

12+
"github.com/argoproj/argo-rollouts/utils/diff"
1213
"k8s.io/apimachinery/pkg/runtime/schema"
1314

1415
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts"
1516
smiclientset "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/split/clientset/versioned"
1617
log "github.com/sirupsen/logrus"
1718
appsv1 "k8s.io/api/apps/v1"
1819
corev1 "k8s.io/api/core/v1"
20+
"k8s.io/apimachinery/pkg/api/errors"
1921
k8serrors "k8s.io/apimachinery/pkg/api/errors"
2022
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2123
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2224
"k8s.io/apimachinery/pkg/runtime"
2325
"k8s.io/apimachinery/pkg/types"
26+
patchtypes "k8s.io/apimachinery/pkg/types"
2427
"k8s.io/apimachinery/pkg/util/validation/field"
2528
"k8s.io/apimachinery/pkg/util/wait"
2629
"k8s.io/client-go/dynamic"
@@ -937,3 +940,78 @@ func remarshalRollout(r *v1alpha1.Rollout) *v1alpha1.Rollout {
937940
}
938941
return &remarshalled
939942
}
943+
944+
// updateReplicaSetWithPatch updates the replicaset using patch and on
945+
func (c *rolloutContext) updateReplicaSetFallbackToPatch(ctx context.Context, rs *appsv1.ReplicaSet) (*appsv1.ReplicaSet, error) {
946+
rsCopy := rs.DeepCopy()
947+
updatedRS, err := c.kubeclientset.AppsV1().ReplicaSets(rs.Namespace).Update(ctx, rs, metav1.UpdateOptions{})
948+
if err != nil {
949+
if errors.IsConflict(err) {
950+
c.log.Infof("Conflict when updating replicaset %s, falling back to patch", rs.Name)
951+
952+
patchRS := appsv1.ReplicaSet{}
953+
patchRS.Spec.Replicas = rsCopy.Spec.Replicas
954+
patchRS.Annotations = rsCopy.Annotations
955+
patchRS.Labels = rsCopy.Labels
956+
patchRS.Spec.Template.Labels = rsCopy.Spec.Template.Labels
957+
patchRS.Spec.Template.Annotations = rsCopy.Spec.Template.Annotations
958+
patchRS.Spec.Selector = rsCopy.Spec.Selector
959+
960+
patch, changed, err := diff.CreateTwoWayMergePatch(appsv1.ReplicaSet{}, patchRS, appsv1.ReplicaSet{})
961+
if err != nil {
962+
return nil, err
963+
}
964+
965+
if changed {
966+
c.log.Infof("Patching replicaset with patch: %s", string(patch))
967+
updatedRS, err = c.kubeclientset.AppsV1().ReplicaSets(rs.Namespace).Patch(ctx, rs.Name, patchtypes.StrategicMergePatchType, patch, metav1.PatchOptions{})
968+
if err != nil {
969+
return nil, err
970+
}
971+
}
972+
973+
err = c.replicaSetInformer.GetIndexer().Update(updatedRS)
974+
if err != nil {
975+
err = fmt.Errorf("error updating replicaset informer in scaleReplicaSet: %w", err)
976+
return nil, err
977+
}
978+
979+
return updatedRS, err
980+
}
981+
}
982+
983+
return updatedRS, err
984+
}
985+
986+
// updateRolloutWithRetry updates the rollout with a retry if there is a conflict from an update operation, it runs the modifyRollout function to update a fresh rollout from the cluster.
987+
//func (c *rolloutContext) updateRolloutWithRetry(ctx context.Context, ro *v1alpha1.Rollout, modifyRollout func(ro *v1alpha1.Rollout) *v1alpha1.Rollout) (*v1alpha1.Rollout, error) {
988+
// updatedRollout, err := c.argoprojclientset.ArgoprojV1alpha1().Rollouts(c.rollout.Namespace).Update(context.TODO(), c.rollout, metav1.UpdateOptions{})
989+
// if err != nil {
990+
// if errors.IsConflict(err) {
991+
// retryCount := 0
992+
// errRetry := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
993+
// retryCount++
994+
// c.log.Infof("conflict when updating rollout %s, retrying the update operation with new rollout from cluster, attempt: %d", c.rollout.Name, retryCount)
995+
// roGet, err := c.argoprojclientset.ArgoprojV1alpha1().Rollouts(c.rollout.Namespace).Get(context.TODO(), c.rollout.Name, metav1.GetOptions{})
996+
// if err != nil {
997+
// return fmt.Errorf("error getting rollout %s: %w", c.rollout.Name, err)
998+
// }
999+
//
1000+
// roCopy := modifyRollout(roGet)
1001+
// updatedRollout, err = c.argoprojclientset.ArgoprojV1alpha1().Rollouts(c.rollout.Namespace).Update(context.TODO(), roCopy, metav1.UpdateOptions{})
1002+
// if err != nil {
1003+
// return err
1004+
// }
1005+
//
1006+
// return nil
1007+
// })
1008+
// if errRetry != nil {
1009+
// return nil, errRetry
1010+
// }
1011+
// } else {
1012+
// c.log.WithError(err).Error("Error: updating rollout revision")
1013+
// return nil, err
1014+
// }
1015+
// }
1016+
// return updatedRollout, nil
1017+
//}

‎rollout/controller_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,12 @@ func (f *fixture) expectPatchServiceAction(s *corev1.Service, newLabel string) i
792792
return len
793793
}
794794

795+
func (f *fixture) expectGetReplicaSetAction(r *appsv1.ReplicaSet) int { //nolint:unused
796+
len := len(f.kubeactions)
797+
f.kubeactions = append(f.kubeactions, core.NewGetAction(schema.GroupVersionResource{Resource: "replicasets"}, r.Namespace, r.Name))
798+
return len
799+
}
800+
795801
func (f *fixture) expectCreateReplicaSetAction(r *appsv1.ReplicaSet) int {
796802
len := len(f.kubeactions)
797803
f.kubeactions = append(f.kubeactions, core.NewCreateAction(schema.GroupVersionResource{Resource: "replicasets"}, r.Namespace, r))

‎rollout/ephemeralmetadata.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package rollout
22

33
import (
44
"context"
5-
"fmt"
65

76
appsv1 "k8s.io/api/apps/v1"
87
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -82,14 +81,12 @@ func (c *rolloutContext) syncEphemeralMetadata(ctx context.Context, rs *appsv1.R
8281
}
8382

8483
// 2. Update ReplicaSet so that any new pods it creates will have the metadata
85-
rs, err = c.kubeclientset.AppsV1().ReplicaSets(modifiedRS.Namespace).Update(ctx, modifiedRS, metav1.UpdateOptions{})
84+
rs, err = c.updateReplicaSetFallbackToPatch(ctx, modifiedRS)
8685
if err != nil {
87-
return fmt.Errorf("error updating replicaset in syncEphemeralMetadata: %w", err)
88-
}
89-
err = c.replicaSetInformer.GetIndexer().Update(rs)
90-
if err != nil {
91-
return fmt.Errorf("error updating replicaset informer in syncEphemeralMetadata: %w", err)
86+
c.log.Infof("failed to sync ephemeral metadata %v to ReplicaSet %s: %v", podMetadata, rs.Name, err)
87+
return err
9288
}
89+
9390
c.log.Infof("synced ephemeral metadata %v to ReplicaSet %s", podMetadata, rs.Name)
9491
return nil
9592
}

0 commit comments

Comments
 (0)