fix: camel-k ignores changes to traits configured using annotations #3480
Conversation
lburgazzoli
changed the title
lburgazzoli
force-pushed
the
github-3479
branch
6 times, most recently
from
017897c
to
fd1666a
Compare
|
@squakez @tadayosi @christophd can you ahve a look ? btw I'm not sure if that's the proper way to solve the issue but |
| @@ -75,6 +77,109 @@ func newReconciler(mgr manager.Manager, c client.Client) reconcile.Reconciler { | |||
| ) | |||
| } | |||
|
|
|||
| func integrationUpdateFunc(old *v1.Integration, it *v1.Integration) bool { | |||
There was a problem hiding this comment.
+1 for encapsulating these functions. However, I could not understand if there is any logic change in them or it was just the encapsulation which changed. Can you clarify?
There was a problem hiding this comment.
the linter was complaining about complexity of the method so I created dedicated functions to make the method more readable.
| } | ||
| } | ||
|
|
||
| return integrationMatches(integration, kit) |
There was a problem hiding this comment.
I've just fixed another possibly related issue here #3487. In that case it was failing because the match always returned false when an IntegrationKit was on error. Not sure if there is any check in the integrationMatches that is influencing the failure here, and if it makes sense to revisit it. I'd expect the integrationMatches to take care of those new added conditions as well.
There was a problem hiding this comment.
For what I understood, in the case the status of a kit should not influence the trigger as in fact every change to a kit should result in a reconcile being triggered. Then the integration controller should decide what to do so I guess your changes are improving also this case.
There was a problem hiding this comment.
Apart from my comments, there's one silently failing e2e test in common-it. I'd like to check if it's really not related to this pull req or not.
2022-07-25T15:08:07.0934307Z === RUN TestKameletBindingScale
...
2022-07-25T15:10:44.4605241Z scale_binding_test.go:58:
2022-07-25T15:10:44.4605677Z Timed out after 60.003s.
2022-07-25T15:10:44.4606001Z Expected
2022-07-25T15:10:44.4606444Z <v1.ConditionStatus>: False
2022-07-25T15:10:44.4606760Z to equal
2022-07-25T15:10:44.4607175Z <v1.ConditionStatus>: True
See #3465 for the issue of e2e silently failing. It's already fixed in the main branch.
lburgazzoli
force-pushed
the
github-3479
branch
2 times, most recently
from
f83de21
to
fdf1818
Compare
lburgazzoli
force-pushed
the
github-3479
branch
2 times, most recently
from
8e3aea7
to
8c58d16
Compare
|
@tadayosi @christophd I see knative test failing but I'm unsure if they are related to this change or not. I may need some help to troubleshoot the failure |
|
@lburgazzoli We've done a lot of efforts to stabilise the e2e tests and now they are reilable enough to keep all green before merging a pull req. The fact that I'm not knowledgeable about the yaks test either but I'd start from replicating the automated test manually and see what's causing this error:
|
Suggestions have been implemented, I guess the overall logging stuff may need to be improved |
|
@christophd I may need some help trying to understand what the failure is about |
lburgazzoli
force-pushed
the
github-3479
branch
from
15c2823
to
c107c32
Compare
lburgazzoli
force-pushed
the
github-3479
branch
from
c107c32
to
2d3c48f
Compare
… on the traits configured on the KameletBinding only apache#3479
lburgazzoli
force-pushed
the
github-3479
branch
from
2d3c48f
to
03c72c7
Compare
Release Note
Fixes #3479