You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "squakez (via GitHub)" <gi...@apache.org> on 2024/03/05 15:38:51 UTC
[PR] chore(traits): store executed traits [camel-k]
squakez opened a new pull request, #5216:
URL: https://github.com/apache/camel-k/pull/5216
<!-- Description -->
This PR addresses a wrong assumption I made in the development of #5153 where we were naively storing the spec after their execution. However, this is not necessarily true, as the trait execution may change certain parameters originally introduced by the user. With this PR we store correctly the executed traits and we make sure to use the `.status.trait` where it would be required for Integration comparisons.
This PR also proves another wrong assumption I made during the development of #5180 where we introduced a check for a possible drift in the `.status.trait`. However, this should never be checked for a change as it's an internal status controlled by the operator and it would wrongly trigger a new rebuild when the operator stores the `.status.trait` value. The operator should normally monitor for changes in the `.spec` only in this case in order to trigger a rebuild if it is required.
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
chore(traits): store executed traits
```
--
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: [PR] chore(traits): store executed traits [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #5216:
URL: https://github.com/apache/camel-k/pull/5216#discussion_r1513957971
##########
pkg/trait/quarkus.go:
##########
@@ -308,7 +308,7 @@ func propagateKitTraits(e *Environment) v1.IntegrationKitTraits {
propagate(fmt.Sprintf("integration profile %q", e.IntegrationProfile.Name), e.IntegrationProfile.Status.Traits, &kitTraits, e)
}
- propagate(fmt.Sprintf("integration %q", e.Integration.Name), e.Integration.Spec.Traits, &kitTraits, e)
+ propagate(fmt.Sprintf("integration %q", e.Integration.Name), e.Integration.Status.Traits, &kitTraits, e)
Review Comment:
Good point. Probably we need to use the "executed trait" that have been used in this reconciliation loop instead of either the `spec` (which represent the user specification) or the `status` (which represent the previous reconciliation loop execution).
--
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: [PR] chore(traits): store executed traits [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez closed pull request #5216: chore(traits): store executed traits
URL: https://github.com/apache/camel-k/pull/5216
--
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: [PR] chore(traits): store executed traits [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5216:
URL: https://github.com/apache/camel-k/pull/5216#issuecomment-1983735186
I realized there are more inconsistencies that need to be tackled before this one. Closing for now and I'll open it again once the rest of problems are fixed.
--
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: [PR] chore(traits): store executed traits [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5216:
URL: https://github.com/apache/camel-k/pull/5216#discussion_r1513299222
##########
pkg/trait/quarkus.go:
##########
@@ -308,7 +308,7 @@ func propagateKitTraits(e *Environment) v1.IntegrationKitTraits {
propagate(fmt.Sprintf("integration profile %q", e.IntegrationProfile.Name), e.IntegrationProfile.Status.Traits, &kitTraits, e)
}
- propagate(fmt.Sprintf("integration %q", e.Integration.Name), e.Integration.Spec.Traits, &kitTraits, e)
+ propagate(fmt.Sprintf("integration %q", e.Integration.Name), e.Integration.Status.Traits, &kitTraits, e)
Review Comment:
Pardon if this may be a totally wrong question but just looking at the code and from some old memories, since the traits are persisted to the status after they have been applied, what the `quarkus` trait sees in the status are the traits for from the previous reconciliation, isn't 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
Re: [PR] chore(traits): store executed traits [camel-k]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5216:
URL: https://github.com/apache/camel-k/pull/5216#issuecomment-1979078990
:heavy_check_mark: Unit test coverage report - coverage increased from 36.8% to 37.2% (**+0.4%**)
--
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