Bug 1378587 - maxSurge and maxUnavailable are not respected when creating, editing, or patching a dc
Summary: maxSurge and maxUnavailable are not respected when creating, editing, or patc...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: OKD
Classification: Red Hat
Component: Deployments
Version: 3.x
Hardware: Unspecified
OS: Unspecified
unspecified
high
Target Milestone: ---
: ---
Assignee: Michail Kargakis
QA Contact: zhou ying
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-09-22 19:55 UTC by Fabiano Franz
Modified: 2016-09-23 14:20 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-09-23 08:44:56 UTC
Target Upstream Version:


Attachments (Terms of Use)

Description Fabiano Franz 2016-09-22 19:55:12 UTC
Description of problem:

Trying to create a deployment config through 'oc create -v <filename>' and the following rolling params:

```
rollingParams:
  intervalSeconds: 1
  maxSurge: 50%
  maxUnavailable: 50%
  timeoutSeconds: 600
  updatePercent: -25
  updatePeriodSeconds: 1
```

But the dc always gets created with

```
rollingParams:
  intervalSeconds: 1
  maxSurge: 0
  maxUnavailable: 25%
  timeoutSeconds: 600
  updatePercent: -25
  updatePeriodSeconds: 1
```

It I try to subsequently use 'oc edit' to adjust maxSurge and maxUnavailable to the desired values, edit returns "deploymentconfig <name> edited" but the values are not changed. 

Same issue (values don't change) if I try to set it with patch:

oc patch dc router -p '{"spec":{"strategy":{"rollingParams":{"maxSurge":"50%","maxUnavailable":"50%"}}}}'

Version-Release number of selected component (if applicable): 
Origin master


How reproducible:
Always 


Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:


Additional info:

Comment 1 Fabiano Franz 2016-09-22 19:56:10 UTC
s/oc create -v/oc create -f

Comment 2 zhou ying 2016-09-23 07:37:20 UTC
The "updatePercent: -25"  will update the 'maxSurge' and 'maxUnavailable'. If you want to set the 'maxSurge' and 'maxUnavailable', you could delete 'updatePercent'.

Comment 3 Michail Kargakis 2016-09-23 08:20:20 UTC
Right, UpdatePercent is deprecated in favor of MaxSurge/MaxUnavailable:
https://github.com/openshift/origin/blob/8134786b8f1deb28c821a6a41dc97888f7122fc4/pkg/deploy/api/types.go#L174

Comment 4 Michail Kargakis 2016-09-23 08:44:56 UTC
You can inspect swagger for the field:

$ oc explain dc.spec.strategy.rollingParams.updatePercent
FIELD: updatePercent <integer>

DESCRIPTION:
     UpdatePercent is the percentage of replicas to scale up or down each
     interval. If nil, one replica will be scaled up and down each interval. If
     negative, the scale order will be down/up instead of up/down. DEPRECATED:
     Use MaxUnavailable/MaxSurge instead.

Comment 5 Kenny Woodson 2016-09-23 13:37:50 UTC
Thanks for the info on this.

The operations team is currently testing upgrades from 3.2 to 3.3 and we ran into this.  

If updatePercent is deprecated in favor of MaxUnavailable/MaxSurge, would it be reasonable to remove updatePercent and take the newer values when requested instead of failing silently?

Perhaps a deprecation warning message would be helpful in this case and notify the user that updatePercent, even though deprecated, takes precedence over the newer, recommended MaxUnavailable/MaxSurge parameters?

Wouldn't it be better to have MaxUnavailable/MaxSurge take precedence when present and either ignore updatePercent or remove it altogether?

Comment 6 Michail Kargakis 2016-09-23 14:20:24 UTC
This has been deprecated since v1.0.6 [0] and there is a note in UPGRADE.md about it [1]. I guess we can remove the field altogether now.

[0] https://github.com/openshift/origin/commit/086d72f1164493fccf46c975d227bf5151f30e6b
[1] See point 5 in https://github.com/openshift/origin/blob/master/UPGRADE.md#origin-10x--ose-30x


Note You need to log in before you can comment on or make changes to this bug.