Skip to content

Commit 32c810c

Browse files
mikkeloscarmikkeloscar
committed
Improve forward feature
Signed-off-by: Mikkel Oscar Lyderik Larsen <[email protected]>
1 parent 0948802 commit 32c810c

File tree

4 files changed

+66
-14
lines changed

4 files changed

+66
-14
lines changed

‎controller/stack_resources.go‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,23 @@ func isOwned(ownerReferences []metav1.OwnerReference) (bool, types.UID) {
4444
func (c *StackSetController) ReconcileStackDeployment(ctx context.Context, stack *zv1.Stack, existing *apps.Deployment, generateUpdated func() *apps.Deployment) error {
4545
deployment := generateUpdated()
4646

47+
// no deployment
48+
if deployment == nil {
49+
if existing != nil {
50+
err := c.client.AppsV1().Deployments(existing.Namespace).Delete(ctx, existing.Name, metav1.DeleteOptions{})
51+
if err != nil {
52+
return err
53+
}
54+
c.recorder.Eventf(
55+
stack,
56+
apiv1.EventTypeNormal,
57+
"DeletedDeployment",
58+
"Deleted Deployment %s",
59+
existing.Name)
60+
}
61+
return nil
62+
}
63+
4764
// Create new deployment
4865
if existing == nil {
4966
_, err := c.client.AppsV1().Deployments(deployment.Namespace).Create(ctx, deployment, metav1.CreateOptions{})
@@ -88,7 +105,7 @@ func (c *StackSetController) ReconcileStackHPA(ctx context.Context, stack *zv1.S
88105
return err
89106
}
90107

91-
// HPA removed
108+
// no HPA
92109
if hpa == nil {
93110
if existing != nil {
94111
err := c.client.AutoscalingV2().HorizontalPodAutoscalers(existing.Namespace).Delete(ctx, existing.Name, metav1.DeleteOptions{})
@@ -100,7 +117,7 @@ func (c *StackSetController) ReconcileStackHPA(ctx context.Context, stack *zv1.S
100117
apiv1.EventTypeNormal,
101118
"DeletedHPA",
102119
"Deleted HPA %s",
103-
existing.Namespace)
120+
existing.Name)
104121
}
105122
return nil
106123
}

‎pkg/core/stack_resources.go‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,10 @@ func (sc *StackContainer) selector() map[string]string {
196196
// "zalando.org/forward-backend", the deployment will be set to
197197
// replicas 1.
198198
func (sc *StackContainer) GenerateDeployment() *appsv1.Deployment {
199+
if sc.TrafficForward() {
200+
// during traffic forwarding we do not need a deployment
201+
return nil
202+
}
199203

200204
stack := sc.Stack
201205

@@ -234,12 +238,6 @@ func (sc *StackContainer) GenerateDeployment() *appsv1.Deployment {
234238
Labels: embeddedCopy.Labels,
235239
}
236240

237-
if _, clusterMigration := sc.Stack.Annotations[forwardBackendAnnotation]; clusterMigration && *updatedReplicas != 0 {
238-
updatedReplicas = wrapReplicas(1)
239-
sc.deploymentReplicas = 1
240-
sc.stackReplicas = 1
241-
}
242-
243241
deployment := &appsv1.Deployment{
244242
ObjectMeta: sc.resourceMeta(),
245243
Spec: appsv1.DeploymentSpec{
@@ -270,7 +268,7 @@ func (sc *StackContainer) GenerateHPA() (
270268
*autoscaling.HorizontalPodAutoscaler,
271269
error,
272270
) {
273-
if _, clusterMigration := sc.Stack.Annotations[forwardBackendAnnotation]; clusterMigration {
271+
if sc.TrafficForward() {
274272
return nil, nil
275273
}
276274

@@ -477,7 +475,7 @@ func (sc *StackContainer) generateIngress(segment bool) (
477475
Rules: rules,
478476
},
479477
}
480-
if _, clusterMigration := sc.Stack.Annotations[forwardBackendAnnotation]; clusterMigration {
478+
if sc.TrafficForward() {
481479
// see https://opensource.zalando.com/skipper/kubernetes/ingress-usage/#skipper-ingress-annotations
482480
result.Annotations["zalando.org/skipper-backend"] = "forward"
483481
}

‎pkg/core/stack_resources_test.go‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,6 +1226,7 @@ func TestStackGenerateDeployment(t *testing.T) {
12261226
deploymentReplicas int32
12271227
noTrafficSince time.Time
12281228
expectedReplicas int32
1229+
expectedDeployment bool
12291230
maxUnavailable int
12301231
maxSurge int
12311232
stackAnnotations map[string]string
@@ -1235,64 +1236,74 @@ func TestStackGenerateDeployment(t *testing.T) {
12351236
stackReplicas: 0,
12361237
deploymentReplicas: 3,
12371238
expectedReplicas: 0,
1239+
expectedDeployment: true,
12381240
},
12391241
{
12401242
name: "stack scaled down to zero, deployment already scaled down",
12411243
stackReplicas: 0,
12421244
deploymentReplicas: 0,
12431245
expectedReplicas: 0,
1246+
expectedDeployment: true,
12441247
},
12451248
{
12461249
name: "stack scaled down because it doesn't have traffic, deployment still running",
12471250
stackReplicas: 3,
12481251
deploymentReplicas: 3,
12491252
noTrafficSince: time.Now().Add(-time.Hour),
12501253
expectedReplicas: 0,
1254+
expectedDeployment: true,
12511255
},
12521256
{
12531257
name: "stack scaled down because it doesn't have traffic, deployment already scaled down",
12541258
stackReplicas: 3,
12551259
deploymentReplicas: 0,
12561260
noTrafficSince: time.Now().Add(-time.Hour),
12571261
expectedReplicas: 0,
1262+
expectedDeployment: true,
12581263
},
12591264
{
12601265
name: "stack scaled down to zero, deployment already scaled down",
12611266
stackReplicas: 0,
12621267
deploymentReplicas: 0,
12631268
expectedReplicas: 0,
1269+
expectedDeployment: true,
12641270
},
12651271
{
12661272
name: "stack running, deployment has zero replicas",
12671273
stackReplicas: 3,
12681274
deploymentReplicas: 0,
12691275
expectedReplicas: 3,
1276+
expectedDeployment: true,
12701277
},
12711278
{
12721279
name: "stack running, deployment has zero replicas, hpa enabled",
12731280
hpaEnabled: true,
12741281
stackReplicas: 3,
12751282
deploymentReplicas: 0,
12761283
expectedReplicas: 3,
1284+
expectedDeployment: true,
12771285
},
12781286
{
12791287
name: "stack running, deployment has the same amount replicas",
12801288
stackReplicas: 3,
12811289
deploymentReplicas: 3,
12821290
expectedReplicas: 3,
1291+
expectedDeployment: true,
12831292
},
12841293
{
12851294
name: "stack running, deployment has a different amount of replicas",
12861295
stackReplicas: 3,
12871296
deploymentReplicas: 5,
12881297
expectedReplicas: 3,
1298+
expectedDeployment: true,
12891299
},
12901300
{
12911301
name: "stack running, deployment has a different amount of replicas, hpa enabled",
12921302
hpaEnabled: true,
12931303
stackReplicas: 3,
12941304
deploymentReplicas: 5,
12951305
expectedReplicas: 5,
1306+
expectedDeployment: true,
12961307
},
12971308
{
12981309
name: "stack running, deployment has zero replicas, prescaling enabled",
@@ -1301,6 +1312,7 @@ func TestStackGenerateDeployment(t *testing.T) {
13011312
prescalingReplicas: 7,
13021313
deploymentReplicas: 0,
13031314
expectedReplicas: 7,
1315+
expectedDeployment: true,
13041316
},
13051317
{
13061318
name: "stack running, deployment has zero replicas, hpa enabled, prescaling enabled",
@@ -1310,6 +1322,7 @@ func TestStackGenerateDeployment(t *testing.T) {
13101322
stackReplicas: 3,
13111323
deploymentReplicas: 0,
13121324
expectedReplicas: 7,
1325+
expectedDeployment: true,
13131326
},
13141327
{
13151328
name: "stack running, deployment has the same amount replicas, prescaling enabled",
@@ -1318,6 +1331,7 @@ func TestStackGenerateDeployment(t *testing.T) {
13181331
prescalingReplicas: 7,
13191332
deploymentReplicas: 7,
13201333
expectedReplicas: 7,
1334+
expectedDeployment: true,
13211335
},
13221336
{
13231337
name: "stack running, deployment has a different amount of replicas, prescaling enabled",
@@ -1326,6 +1340,7 @@ func TestStackGenerateDeployment(t *testing.T) {
13261340
prescalingReplicas: 7,
13271341
deploymentReplicas: 5,
13281342
expectedReplicas: 7,
1343+
expectedDeployment: true,
13291344
},
13301345
{
13311346
name: "stack running, deployment has a different amount of replicas, hpa enabled, prescaling enabled",
@@ -1335,20 +1350,23 @@ func TestStackGenerateDeployment(t *testing.T) {
13351350
stackReplicas: 3,
13361351
deploymentReplicas: 5,
13371352
expectedReplicas: 5,
1353+
expectedDeployment: true,
13381354
},
13391355
{
13401356
name: "max surge is specified",
13411357
stackReplicas: 3,
13421358
deploymentReplicas: 3,
13431359
expectedReplicas: 3,
13441360
maxSurge: 10,
1361+
expectedDeployment: true,
13451362
},
13461363
{
13471364
name: "max unavailable is specified",
13481365
stackReplicas: 3,
13491366
deploymentReplicas: 3,
13501367
expectedReplicas: 3,
13511368
maxUnavailable: 10,
1369+
expectedDeployment: true,
13521370
},
13531371
{
13541372
name: "max surge and max unavailable are specified",
@@ -1357,10 +1375,12 @@ func TestStackGenerateDeployment(t *testing.T) {
13571375
expectedReplicas: 3,
13581376
maxSurge: 1,
13591377
maxUnavailable: 10,
1378+
expectedDeployment: true,
13601379
},
13611380
{
1362-
name: "minReadySeconds should be set",
1363-
minReadySeconds: 5,
1381+
name: "minReadySeconds should be set",
1382+
minReadySeconds: 5,
1383+
expectedDeployment: true,
13641384
},
13651385
{
13661386
name: "cluster migration should scale down deployment to 1",
@@ -1369,7 +1389,7 @@ func TestStackGenerateDeployment(t *testing.T) {
13691389
stackAnnotations: map[string]string{
13701390
forwardBackendAnnotation: "fwd-deployment",
13711391
},
1372-
expectedReplicas: 1,
1392+
expectedDeployment: false,
13731393
},
13741394
} {
13751395
t.Run(tc.name, func(t *testing.T) {
@@ -1431,7 +1451,13 @@ func TestStackGenerateDeployment(t *testing.T) {
14311451
}
14321452
}
14331453
deployment := c.GenerateDeployment()
1434-
expected := &apps.Deployment{
1454+
var expected *apps.Deployment
1455+
if !tc.expectedDeployment {
1456+
require.Nil(t, deployment)
1457+
return
1458+
}
1459+
1460+
expected = &apps.Deployment{
14351461
ObjectMeta: testResourceMeta,
14361462
Spec: apps.DeploymentSpec{
14371463
Replicas: wrapReplicas(tc.expectedReplicas),

‎pkg/core/types.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,12 @@ func (sc *StackContainer) HasTraffic() bool {
159159
}
160160

161161
func (sc *StackContainer) IsReady() bool {
162+
if sc.TrafficForward() {
163+
// if stack is configured to forward traffic, consider it
164+
// ready.
165+
return true
166+
}
167+
162168
// Calculate minimum required replicas for the Deployment to be considered ready
163169
minRequiredReplicas := int32(math.Ceil(float64(sc.deploymentReplicas) * sc.minReadyPercent))
164170

@@ -187,6 +193,11 @@ func (sc *StackContainer) ScaledDown() bool {
187193
return !sc.noTrafficSince.IsZero() && time.Since(sc.noTrafficSince) > sc.scaledownTTL
188194
}
189195

196+
func (sc *StackContainer) TrafficForward() bool {
197+
_, clusterMigration := sc.Stack.Annotations[forwardBackendAnnotation]
198+
return clusterMigration
199+
}
200+
190201
func (sc *StackContainer) Name() string {
191202
return sc.Stack.Name
192203
}

0 commit comments

Comments
 (0)