Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing integration does not report error condition #3412

Closed
squakez opened this issue Jul 4, 2022 · 5 comments · Fixed by #3487
Closed

Failing integration does not report error condition #3412

squakez opened this issue Jul 4, 2022 · 5 comments · Fixed by #3487
Assignees
Labels
area/core Core features of the integration platform kind/bug Something isn't working

Comments

@squakez
Copy link
Contributor

squakez commented Jul 4, 2022

Any failing integration, ie kamel run -d mvn:com.example:nonexistent:1.0 Hello.java will correctly set the error on IntegrationKit, but won't do on Integration, resulting the Integration in a Building Kit state even if it failed:

squake:~ $ kamel get
NAME	PHASE		KIT
hello	Building Kit	default/kit-cb19gvc268e9p6peapt0

squake:~ $ k get integrationkit
NAME                       PHASE   TYPE       IMAGE
kit-cb19gvc268e9p6peapt0   Error   platform 
@squakez squakez added kind/bug Something isn't working area/programming-model labels Jul 4, 2022
@squakez squakez assigned squakez and unassigned squakez Jul 11, 2022
@squakez
Copy link
Contributor Author

squakez commented Jul 13, 2022

Hey @astefanutti, I had a first look at this, but I am not able to figure it out where it is failing. Do you have some suggestion to places I should look for a fix? I had a look at the code and I think that error conditions should be captured by

if kit.Status.Phase == v1.IntegrationKitPhaseError {
, although it's not. Thanks in advance for the support.

@tadayosi
Copy link
Member

tadayosi commented Jul 13, 2022

Is this somewhere like this that is relevant to the issue?
https://github.com/apache/camel-k/blob/main/pkg/controller/integration/monitor.go#L218

@squakez
Copy link
Contributor Author

squakez commented Jul 13, 2022

Is this somewhere like this that is relevant to the issue? https://github.com/apache/camel-k/blob/main/pkg/controller/integration/monitor.go#L218

Thanks @tadayosi. I am not sure what is the expected behavior. I mean, I'm assuming that a failing IntegrationKit build will take care to turn the Integration which owns it as errored as well but I don't know where it used to happen. There were several refactoring in the past, so, likely we missed something. Also we need to introduce an E2E test to take care of this situation and avoid any future regression.

@astefanutti
Copy link
Member

@squakez as you pointed it out, the integration phase is set to error when the kit fails in:

if kit.Status.Phase == v1.IntegrationKitPhaseError {

I suspect this works, but the integration phase gets overwritten later with:

integration.Status.Phase = v1.IntegrationPhaseRunning

For some reasons, the phase is not set to error while introspecting the underlying deployment / pods. That being said, it may be more robust to force the phase to error in that case, which would likely provide a better error message.

@astefanutti astefanutti added area/core Core features of the integration platform and removed area/programming-model labels Jul 13, 2022
@squakez squakez self-assigned this Jul 21, 2022
@squakez
Copy link
Contributor Author

squakez commented Jul 26, 2022

I've gone deep in this and found the responsible is this check here:

func statusMatches(integration *v1.Integration, kit *v1.IntegrationKit, ilog *log.Logger) bool {
if kit.Status.Phase == v1.IntegrationKitPhaseError {
ilog.Debug("Integration kit has a phase of Error", "integration-kit", kit.Name, "namespace", integration.Namespace)
return false
}
Basically we never found a match when the integrationkit has failed, resulting in the issue described. Unfortunately the method is used in other places, so I will need to find a way to solve it.

squakez added a commit to squakez/camel-k that referenced this issue Jul 26, 2022
squakez added a commit that referenced this issue Jul 26, 2022
squakez added a commit to squakez/camel-k that referenced this issue Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core Core features of the integration platform kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants