astefanutti commented on a change in pull request #2292:
URL: https://github.com/apache/camel-k/pull/2292#discussion_r651606528
##########
File path: pkg/controller/integration/error.go
##########
@@ -56,6 +57,27 @@ func (action *errorAction) Handle(ctx context.Context,
integration *v1.Integrati
return integration, nil
}
+ // the integration in error state may have recovered and the running
pod may be ok
+ // at this point we need to check, if the pod is ok set the integration
to running.
+ podList, _ :=
action.client.CoreV1().Pods(integration.Namespace).List(ctx, metav1.ListOptions{
+ LabelSelector: v1.IntegrationLabel + "=" + integration.Name,
+ FieldSelector: "status.phase=Running",
+ })
+
+ if len(podList.Items) > 0 {
+ // when pod respin, the podlist may contain two pods (the
terminating pod and the new starting)
+ // we want the last one as it is the newest
+ pod := podList.Items[len(podList.Items)-1]
+
+ // we look at the container status, because the pod may be in
running phase
+ // but the container may be in error or running state
+ if running := pod.Status.ContainerStatuses[0].State.Running;
running != nil {
Review comment:
The Integration Pod(s) can have multiple containers, e.g., with Knative
sidecars, or even when using the Pod trait. Relying on the first container
seems wrong in that case.
##########
File path: pkg/controller/integration/error.go
##########
@@ -56,6 +57,27 @@ func (action *errorAction) Handle(ctx context.Context,
integration *v1.Integrati
return integration, nil
}
+ // the integration in error state may have recovered and the running
pod may be ok
+ // at this point we need to check, if the pod is ok set the integration
to running.
+ podList, _ :=
action.client.CoreV1().Pods(integration.Namespace).List(ctx, metav1.ListOptions{
+ LabelSelector: v1.IntegrationLabel + "=" + integration.Name,
+ FieldSelector: "status.phase=Running",
+ })
+
+ if len(podList.Items) > 0 {
+ // when pod respin, the podlist may contain two pods (the
terminating pod and the new starting)
+ // we want the last one as it is the newest
+ pod := podList.Items[len(podList.Items)-1]
Review comment:
The Integration can have multiple replicas, when it's scaled manually,
or automatically with Knative or HPA. Relying on the last Pod from the list
seems wrong in that case.
##########
File path: pkg/controller/integration/error.go
##########
@@ -56,6 +57,27 @@ func (action *errorAction) Handle(ctx context.Context,
integration *v1.Integrati
return integration, nil
}
+ // the integration in error state may have recovered and the running
pod may be ok
+ // at this point we need to check, if the pod is ok set the integration
to running.
+ podList, _ :=
action.client.CoreV1().Pods(integration.Namespace).List(ctx, metav1.ListOptions{
Review comment:
The reconcile loop won't always be called, as Pods are not being watched
by the Integration reconciler.
##########
File path: pkg/controller/integration/error.go
##########
@@ -56,6 +57,27 @@ func (action *errorAction) Handle(ctx context.Context,
integration *v1.Integrati
return integration, nil
}
+ // the integration in error state may have recovered and the running
pod may be ok
+ // at this point we need to check, if the pod is ok set the integration
to running.
+ podList, _ :=
action.client.CoreV1().Pods(integration.Namespace).List(ctx, metav1.ListOptions{
Review comment:
Generally, it's preferable to use the client from controller-runtime, as
it relies on cached informers.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
