You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/09/28 13:42:25 UTC
[GitHub] [camel-k] astefanutti opened a new pull request #2662: feat: Consistent Integration scale status
astefanutti opened a new pull request #2662:
URL: https://github.com/apache/camel-k/pull/2662
This PR provides a consistent Integration scaling status, by directly reconciling Integration Pods, rather than relying on owned resources, like Deployment, KnativeService and CronJob, that do not provide a minimal common denominator.
It fixes several issues, for which the reported number of replicas was incorrect, when the wrong ReplicaSet was selected for example.
It also restricts the caching of Pods by Integration label selector, and also projects the cached resources to `PartialObjectMetadata`, in order to limit memory requirement for the operator when scaling the number of Integrations.
Last but not least, It prepares better error handling reporting, for which proper replicas reporting is a pre-requisite.
**Release Note**
```release-note
feat: Consistent Integration scale status
```
--
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
[GitHub] [camel-k] lburgazzoli commented on a change in pull request #2662: feat: Consistent Integration scale status
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #2662:
URL: https://github.com/apache/camel-k/pull/2662#discussion_r717595519
##########
File path: pkg/apis/camel/v1/integration_types.go
##########
@@ -184,12 +184,10 @@ const (
IntegrationConditionProbesAvailableReason string = "ProbesAvailable"
// IntegrationConditionErrorReason --
IntegrationConditionErrorReason string = "Error"
- // IntegrationConditionCronJobCreatedReason --
- IntegrationConditionCronJobCreatedReason string = "CronJobCreated"
- // IntegrationConditionReplicaSetReadyReason --
- IntegrationConditionReplicaSetReadyReason string = "ReplicaSetReady"
- // IntegrationConditionReplicaSetNotReadyReason --
- IntegrationConditionReplicaSetNotReadyReason string = "ReplicaSetNotReady"
+ // IntegrationConditionPodsReadyReason --
+ IntegrationConditionPodsReadyReason string = "PodsReady"
+ // IntegrationConditionPodsNotReadyReason --
+ IntegrationConditionPodsNotReadyReason string = "PodsNotReady"
Review comment:
Should we take this as a potential minor breaking change ? i.e. for tools that are monitoring an integration, changing a condition could break them.
I don't have any objection but maybe we need to document it
--
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
[GitHub] [camel-k] astefanutti merged pull request #2662: feat: Consistent Integration scale status
Posted by GitBox <gi...@apache.org>.
astefanutti merged pull request #2662:
URL: https://github.com/apache/camel-k/pull/2662
--
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
[GitHub] [camel-k] astefanutti commented on a change in pull request #2662: feat: Consistent Integration scale status
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2662:
URL: https://github.com/apache/camel-k/pull/2662#discussion_r717601356
##########
File path: pkg/apis/camel/v1/integration_types.go
##########
@@ -184,12 +184,10 @@ const (
IntegrationConditionProbesAvailableReason string = "ProbesAvailable"
// IntegrationConditionErrorReason --
IntegrationConditionErrorReason string = "Error"
- // IntegrationConditionCronJobCreatedReason --
- IntegrationConditionCronJobCreatedReason string = "CronJobCreated"
- // IntegrationConditionReplicaSetReadyReason --
- IntegrationConditionReplicaSetReadyReason string = "ReplicaSetReady"
- // IntegrationConditionReplicaSetNotReadyReason --
- IntegrationConditionReplicaSetNotReadyReason string = "ReplicaSetNotReady"
+ // IntegrationConditionPodsReadyReason --
+ IntegrationConditionPodsReadyReason string = "PodsReady"
+ // IntegrationConditionPodsNotReadyReason --
+ IntegrationConditionPodsNotReadyReason string = "PodsNotReady"
Review comment:
I agree, ideally we should have a "Breaking Changes" section in the release notes, that would document these kind of changes.
--
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
[GitHub] [camel-k] astefanutti commented on a change in pull request #2662: feat: Consistent Integration scale status
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2662:
URL: https://github.com/apache/camel-k/pull/2662#discussion_r717601356
##########
File path: pkg/apis/camel/v1/integration_types.go
##########
@@ -184,12 +184,10 @@ const (
IntegrationConditionProbesAvailableReason string = "ProbesAvailable"
// IntegrationConditionErrorReason --
IntegrationConditionErrorReason string = "Error"
- // IntegrationConditionCronJobCreatedReason --
- IntegrationConditionCronJobCreatedReason string = "CronJobCreated"
- // IntegrationConditionReplicaSetReadyReason --
- IntegrationConditionReplicaSetReadyReason string = "ReplicaSetReady"
- // IntegrationConditionReplicaSetNotReadyReason --
- IntegrationConditionReplicaSetNotReadyReason string = "ReplicaSetNotReady"
+ // IntegrationConditionPodsReadyReason --
+ IntegrationConditionPodsReadyReason string = "PodsReady"
+ // IntegrationConditionPodsNotReadyReason --
+ IntegrationConditionPodsNotReadyReason string = "PodsNotReady"
Review comment:
I agree, ideally we should have a "Breaking Changes" section in the release notes, that would document these kind of changes.
--
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
[GitHub] [camel-k] astefanutti commented on pull request #2662: feat: Consistent Integration scale status
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #2662:
URL: https://github.com/apache/camel-k/pull/2662#issuecomment-932146025
I think this is ready.
So besides fixing the scale status issues, it also removes the watches for Pods, that could lead to receiving all the updates for all the Pods on the entire cluster, except but for the Integration Pods, which should improve scalability by multiple orders of magnitude. This is strictly enforced by the label selector sets on the operator manager cache, so care must be taken to use a non-cached client when getting non Integration Pods, otherwise `nil` or an empty list is returned.
--
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
[GitHub] [camel-k] lburgazzoli commented on a change in pull request #2662: feat: Consistent Integration scale status
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #2662:
URL: https://github.com/apache/camel-k/pull/2662#discussion_r717595519
##########
File path: pkg/apis/camel/v1/integration_types.go
##########
@@ -184,12 +184,10 @@ const (
IntegrationConditionProbesAvailableReason string = "ProbesAvailable"
// IntegrationConditionErrorReason --
IntegrationConditionErrorReason string = "Error"
- // IntegrationConditionCronJobCreatedReason --
- IntegrationConditionCronJobCreatedReason string = "CronJobCreated"
- // IntegrationConditionReplicaSetReadyReason --
- IntegrationConditionReplicaSetReadyReason string = "ReplicaSetReady"
- // IntegrationConditionReplicaSetNotReadyReason --
- IntegrationConditionReplicaSetNotReadyReason string = "ReplicaSetNotReady"
+ // IntegrationConditionPodsReadyReason --
+ IntegrationConditionPodsReadyReason string = "PodsReady"
+ // IntegrationConditionPodsNotReadyReason --
+ IntegrationConditionPodsNotReadyReason string = "PodsNotReady"
Review comment:
Should we take this as a potential minor breaking change ? i.e. for tools that are monitoring an integration, changing a condition could break them.
I don't have any objection but maybe we need to document it
--
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