You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "lsergio (via GitHub)" <gi...@apache.org> on 2024/03/06 14:00:08 UTC
[I] Deployment Trait maxUnavailable and maxSurge default values differ from documentation [camel-k]
lsergio opened a new issue, #5223:
URL: https://github.com/apache/camel-k/issues/5223
### What happened?
Create an Integration and configure the Deployment trait as below:
```
traits:
deployment:
strategy: RollingUpdate
```
Per the docs, the default maxSurge and maxUnavailable should be 25%, but checking the Deployment we get 25, not 25%:
```
strategy:
rollingUpdate:
maxSurge: 25
maxUnavailable: 25
type: RollingUpdate
```
It works, though, if we do not declare the deployment trait at all. In this case the deploment will be as below:
```
strategy:
rollingUpdate:
maxSurge: 25%
maxUnavailable: 25%
type: RollingUpdate
```
### Steps to reproduce
_No response_
### Relevant log output
_No response_
### Camel K version
2.2.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deployment Trait maxUnavailable and maxSurge default values differ from documentation and do not support % values [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez closed issue #5223: Deployment Trait maxUnavailable and maxSurge default values differ from documentation and do not support % values
URL: https://github.com/apache/camel-k/issues/5223
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deployment Trait maxUnavailable and maxSurge default values differ from documentation [camel-k]
Posted by "lsergio (via GitHub)" <gi...@apache.org>.
lsergio commented on issue #5223:
URL: https://github.com/apache/camel-k/issues/5223#issuecomment-1980952929
Investigating a little further, I see that I cannot even set a percentage value explicitly:
```
deployment:
strategy: RollingUpdate
rollingUpdateMaxSurge: 10%
rollingUpdateMaxUnavailable: 10%
```
This issues the error:
```
* spec.traits.deployment.rollingUpdateMaxSurge: Invalid value: "string": spec.traits.deployment.rollingUpdateMaxSurge in body must be of type integer: "string"
* spec.traits.deployment.rollingUpdateMaxUnavailable: Invalid value: "string": spec.traits.deployment.rollingUpdateMaxUnavailable in body must be of type integer: "string"
*
```
So we would also have to change the type of these variables from int to string:
```
// The maximum number of pods that can be unavailable during the update.
// Value can be an absolute number (ex: 5) or a percentage of desired pods (ex: 10%).
// Absolute number is calculated from percentage by rounding down.
// This can not be 0 if MaxSurge is 0.
// Defaults to `25%`.
RollingUpdateMaxUnavailable *int `property:"rolling-update-max-unavailable" json:"rollingUpdateMaxUnavailable,omitempty"`
// The maximum number of pods that can be scheduled above the desired number of
// pods.
// Value can be an absolute number (ex: 5) or a percentage of desired pods (ex: 10%).
// This can not be 0 if MaxUnavailable is 0.
// Absolute number is calculated from percentage by rounding up.
// Defaults to `25%`.
RollingUpdateMaxSurge *int `property:"rolling-update-max-surge" json:"rollingUpdateMaxSurge,omitempty"`
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deployment Trait maxUnavailable and maxSurge values differ from documentation and do not support % values [camel-k]
Posted by "lsergio (via GitHub)" <gi...@apache.org>.
lsergio commented on issue #5223:
URL: https://github.com/apache/camel-k/issues/5223#issuecomment-1981524817
After the change I was able to apply Integrations with all the following deployment trait configurations with the snapshot operator:
```
traits
deployment:
strategy: RollingUpdate
rollingUpdateMaxUnavailable: 30%
rollingUpdateMaxSurge: 20%
traits
deployment:
strategy: RollingUpdate
rollingUpdateMaxUnavailable: 30
rollingUpdateMaxSurge: 20
traits
deployment:
strategy: RollingUpdate
traits
// no deployment trait configured
```
in all cases the generated deployment respected the values I provided or used the 25% default
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deployment Trait maxUnavailable and maxSurge default values differ from documentation [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on issue #5223:
URL: https://github.com/apache/camel-k/issues/5223#issuecomment-1981071600
Okey, it seems the codegen is already taking care of that. Better, then. I'll have a look at the PR later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deployment Trait maxUnavailable and maxSurge default values differ from documentation [camel-k]
Posted by "lsergio (via GitHub)" <gi...@apache.org>.
lsergio commented on issue #5223:
URL: https://github.com/apache/camel-k/issues/5223#issuecomment-1981330726
I see that in v2.2.0 the 25 default value is set at the crd (https://github.com/apache/camel-k/blob/v2.2.0/helm/camel-k/crds/crd-integration.yaml#L6755). It was supposed to mean 25% but it really means 25.
However, the main branch does not have such a value (https://github.com/apache/camel-k/blob/main/helm/camel-k/crds/crd-integration.yaml#L6755) an thus delegate the default values to Kubernetes.
So I think the wrong default value of 25 instead of 25% is no longer an issue and the PR would fix only the datatype in order to allow the users to inform percentage values on the trait.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deployment Trait maxUnavailable and maxSurge default values differ from documentation [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on issue #5223:
URL: https://github.com/apache/camel-k/issues/5223#issuecomment-1981000453
More than changing the variable (which would require deprecating the API in favour of a new parameter), I wonder if we can just make a trasformation into the operator to cast to the value expected by Kubernetes instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Deployment Trait maxUnavailable and maxSurge default values differ from documentation [camel-k]
Posted by "lsergio (via GitHub)" <gi...@apache.org>.
lsergio commented on issue #5223:
URL: https://github.com/apache/camel-k/issues/5223#issuecomment-1981019746
@squakez if we change the type from int to intorstring, the api would still work with numeric values, so we might not need new parameters. Take a look at this: https://github.com/apache/camel-k/pull/5224
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org