Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Abort rollout removes all canary pods for setCanaryScale. Fixes #1337 #1352

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions test/e2e/istio/istio-rollout-abort-delete-all-canary-pods.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
apiVersion: v1
kind: Service
metadata:
name: istio-host-split-canary
spec:
ports:
- port: 80
targetPort: http
protocol: TCP
name: http
selector:
app: istio-host-split

---
apiVersion: v1
kind: Service
metadata:
name: istio-host-split-stable
spec:
ports:
- port: 80
targetPort: http
protocol: TCP
name: http
selector:
app: istio-host-split

---
apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
name: istio-host-split-vsvc
spec:
hosts:
- istio-host-split
http:
- name: primary
route:
- destination:
host: istio-host-split-stable
weight: 100
- destination:
host: istio-host-split-canary
weight: 0

---
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
name: istio-host-split
spec:
replicas: 5
strategy:
canary:
canaryService: istio-host-split-canary
stableService: istio-host-split-stable
trafficRouting:
istio:
virtualService:
name: istio-host-split-vsvc
routes:
- primary
steps:
- setCanaryScale:
replicas: 2
- setWeight: 20
- pause: {}
- setCanaryScale:
replicas: 4
- setWeight: 40
- pause: {}
selector:
matchLabels:
app: istio-host-split
template:
metadata:
labels:
app: istio-host-split
spec:
containers:
- name: istio-host-split
image: nginx:1.19-alpine
ports:
- name: http
containerPort: 80
protocol: TCP
resources:
requests:
memory: 16Mi
cpu: 5m
29 changes: 29 additions & 0 deletions test/e2e/istio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,3 +224,32 @@ func (s *IstioSuite) TestIstioAbortUpdate() {
Then().
ExpectRevisionPodCount("2", 1)
}

func (s *IstioSuite) TestIstioAbortUpdateDeleteAllCanaryPods() {
s.Given().
RolloutObjects("@istio/istio-rollout-abort-delete-all-canary-pods.yaml").
When().
ApplyManifests().
WaitForRolloutStatus("Healthy").
Then().
When().
UpdateSpec().
WaitForRolloutStatus("Paused").
Then().
ExpectRevisionPodCount("2", 2).
When().
PromoteRollout().
WaitForRolloutStatus("Paused").
Then().
When().
PromoteRollout().
WaitForRolloutStatus("Paused").
Then().
ExpectRevisionPodCount("2", 4).
When().
AbortRollout().
WaitForRolloutStatus("Degraded").
Then().
ExpectRevisionPodCount("2", 0).
ExpectRevisionPodCount("1", 5)
}
4 changes: 4 additions & 0 deletions utils/replicaset/canary.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,10 @@ func GetCurrentSetWeight(rollout *v1alpha1.Rollout) int32 {
// TrafficRouting is required to be set for SetCanaryScale to be applicable.
// If MatchTrafficWeight is set after a previous SetCanaryScale step, it will likewise be ignored.
func UseSetCanaryScale(rollout *v1alpha1.Rollout) *v1alpha1.SetCanaryScale {
// Return nil when rollout is aborted
if rollout.Status.Abort {
return nil
}
currentStep, currentStepIndex := GetCurrentCanaryStep(rollout)
if currentStep == nil {
return nil
Expand Down