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 2022/04/17 18:46:48 UTC
[GitHub] [camel-k] SubhasmitaSw opened a new pull request, #3218: added .status.observedGeneration to camel K CRDs
SubhasmitaSw opened a new pull request, #3218:
URL: https://github.com/apache/camel-k/pull/3218
fixes: issue #3182
cc @astefanutti
--
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 diff in pull request #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
astefanutti commented on code in PR #3218:
URL: https://github.com/apache/camel-k/pull/3218#discussion_r853822760
##########
pkg/controller/kameletbinding/kamelet_binding_controller.go:
##########
@@ -214,6 +214,8 @@ func (r *ReconcileKameletBinding) Reconcile(ctx context.Context, request reconci
break
}
}
+ // Update the metadata.Generation to avoid conflicts with other clients updating the resource at the same time we are reconciling it.
+ target.Status.ObservedGeneration = instance.Generation
Review Comment:
This must be added just before line 195.
##########
pkg/controller/kamelet/kamelet_controller.go:
##########
@@ -200,6 +200,9 @@ func (r *reconcileKamelet) Reconcile(ctx context.Context, request reconcile.Requ
break
}
+ // Update the metadata.generation to avoid conflicts with other reconcile operations
+ target.Status.ObservedGeneration = instance.GetGeneration()
Review Comment:
This must be added just before:
```
if err := r.client.Status().Patch(ctx, target, ctrl.MergeFrom(&instance)); err != nil {
```
Line 181.
##########
pkg/controller/integrationkit/integrationkit_controller.go:
##########
@@ -303,6 +303,7 @@ func (r *reconcileIntegrationKit) Reconcile(ctx context.Context, request reconci
func (r *reconcileIntegrationKit) update(ctx context.Context, base *v1.IntegrationKit, target *v1.IntegrationKit) (reconcile.Result, error) {
dgst, err := digest.ComputeForIntegrationKit(target)
+ target.Status.ObservedGeneration = base.Generation
Review Comment:
This is better moved after line 311.
##########
pkg/apis/camel/v1/integrationkit_types.go:
##########
@@ -67,6 +67,8 @@ type IntegrationKitSpec struct {
// IntegrationKitStatus defines the observed state of IntegrationKit
type IntegrationKitStatus struct {
+ // ObservedGeneration is the last generation change we observed. This is used to determine if we need to refresh the status of the integration kit.
Review Comment:
```suggestion
// ObservedGeneration is the most recent generation observed for this IntegrationKit.
```
##########
pkg/controller/integrationplatform/integrationplatform_controller.go:
##########
@@ -173,6 +173,9 @@ func (r *reconcileIntegrationPlatform) Reconcile(ctx context.Context, request re
target := instance.DeepCopy()
targetLog := rlog.ForIntegrationPlatform(target)
+ // Update the metadata.generation to avoid conflicts with other reconcile operations
+ target.Status.ObservedGeneration = instance.Generation
Review Comment:
I'd recommend to add it just before line 195.
##########
pkg/apis/camel/v1alpha1/kamelet_binding_types.go:
##########
@@ -94,6 +94,8 @@ type EndpointProperties struct {
// KameletBindingStatus specify the status of a binding
type KameletBindingStatus struct {
+ // ObservedGeneration is the generation observed by the KameletBinding controller.
Review Comment:
```suggestion
// ObservedGeneration is the most recent generation observed for this KameletBinding.
```
##########
pkg/apis/camel/v1/integration_types.go:
##########
@@ -85,6 +85,9 @@ type IntegrationSpec struct {
// IntegrationStatus defines the observed state of Integration
type IntegrationStatus struct {
+ // ObservedGeneration is the most recent generation observed for this Integration.
Review Comment:
```suggestion
// ObservedGeneration is the most recent generation observed for this Integration.
```
##########
pkg/controller/integration/integration_controller.go:
##########
@@ -338,6 +338,7 @@ func (r *reconcileIntegration) Reconcile(ctx context.Context, request reconcile.
}
func (r *reconcileIntegration) update(ctx context.Context, base *v1.Integration, target *v1.Integration) (reconcile.Result, error) {
+ target.Status.ObservedGeneration = base.Generation
Review Comment:
I'd suggest to move it after line 347.
##########
pkg/apis/camel/v1/build_types.go:
##########
@@ -146,6 +146,8 @@ type S2iTask struct {
// BuildStatus defines the observed state of Build
type BuildStatus struct {
+ //ObservedGeneration is the most recent generation observed for this Build.
Review Comment:
```suggestion
// ObservedGeneration is the most recent generation observed for this Build.
```
##########
pkg/apis/camel/v1/integrationplatform_types.go:
##########
@@ -52,7 +52,8 @@ type IntegrationPlatformResourcesSpec struct {
// IntegrationPlatformStatus defines the observed state of IntegrationPlatform
type IntegrationPlatformStatus struct {
IntegrationPlatformSpec `json:",inline"`
-
+ // ObservedGeneration is the most recent generation observed by the controller. It corresponds to the controller's `metadata.generation` and is updated on mutation by the API Server.
Review Comment:
```suggestion
// ObservedGeneration is the most recent generation observed for this IntegrationPlatform.
```
--
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 diff in pull request #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
astefanutti commented on code in PR #3218:
URL: https://github.com/apache/camel-k/pull/3218#discussion_r853894806
##########
pkg/controller/kamelet/kamelet_controller.go:
##########
@@ -178,6 +178,9 @@ func (r *reconcileKamelet) Reconcile(ctx context.Context, request reconcile.Requ
}
if target != nil {
+ // Update the metadata.generation to avoid conflicts with other reconcile operations.
Review Comment:
That comment can be removed.
##########
pkg/controller/kameletbinding/kamelet_binding_controller.go:
##########
@@ -192,6 +192,9 @@ func (r *ReconcileKameletBinding) Reconcile(ctx context.Context, request reconci
}
if target != nil {
+ // Update the metadata.Generation to avoid conflicts with other clients updating the resource at the same time we are reconciling it.
Review Comment:
That comment can be removed.
##########
pkg/controller/integrationplatform/integrationplatform_controller.go:
##########
@@ -189,6 +189,9 @@ func (r *reconcileIntegrationPlatform) Reconcile(ctx context.Context, request re
}
if target != nil {
+ // Update the metadata.generation to avoid conflicts with other reconcile operations
Review Comment:
That comment can be removed.
--
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 diff in pull request #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
astefanutti commented on code in PR #3218:
URL: https://github.com/apache/camel-k/pull/3218#discussion_r853962309
##########
pkg/apis/camel/v1/integration_types.go:
##########
@@ -85,6 +85,9 @@ type IntegrationSpec struct {
// IntegrationStatus defines the observed state of Integration
type IntegrationStatus struct {
+ // ObservedGeneration is the most recent generation observed for this Integration.
+ // It corresponds to the Integration's generation, which is updated on mutation by the API Server.
Review Comment:
That line can be removed.
--
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] squakez commented on pull request #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3218:
URL: https://github.com/apache/camel-k/pull/3218#issuecomment-1101293192
I am not very familiar with this part, but I think you can have a look at the update function, which should be in charge to set that value: https://github.com/apache/camel-k/blob/main/pkg/controller/build/build_controller.go#L74
--
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] christophd commented on pull request #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
christophd commented on PR #3218:
URL: https://github.com/apache/camel-k/pull/3218#issuecomment-1104945446
YAKS has a pinned prerelease available [0.11.0-202204210858](https://github.com/citrusframework/yaks/releases/tag/0.11.0-202204210858) that should fix the issue.
@SubhasmitaSw to use the fixed YAKS version please update the version in the GitHub actions workflow to `0.11.0-202204210858`:
https://github.com/apache/camel-k/blob/main/.github/actions/e2e-knative-yaks/action.yml#L81
--
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 #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
astefanutti merged PR #3218:
URL: https://github.com/apache/camel-k/pull/3218
--
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] squakez commented on pull request #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3218:
URL: https://github.com/apache/camel-k/pull/3218#issuecomment-1101247354
We should also provide some basic E2E test if possible.
--
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 #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
astefanutti commented on PR #3218:
URL: https://github.com/apache/camel-k/pull/3218#issuecomment-1104811406
There are some YAKS tests failing, that requires a change in YAKS itself. I've created citrusframework/yaks#403. Once it's fixed, that PR can be merged.
--
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 #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
astefanutti commented on PR #3218:
URL: https://github.com/apache/camel-k/pull/3218#issuecomment-1146004603
Let's merge this. The `TestKitTimerToLogFullNativeBuild` test failure is already present in main branch.
--
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 #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
astefanutti commented on PR #3218:
URL: https://github.com/apache/camel-k/pull/3218#issuecomment-1146004861
Thanks!
--
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] SubhasmitaSw commented on pull request #3218: added .status.observedGeneration to camel K CRDs
Posted by GitBox <gi...@apache.org>.
SubhasmitaSw commented on PR #3218:
URL: https://github.com/apache/camel-k/pull/3218#issuecomment-1101282003
> Nice, thanks. However I think we're still missing the update part. We should check in the reconciliation cycle and update accordingly.
For this part, @astefanutti was about to share some background references to me regarding `metadata.generation` field, however he seems to be occupied so I'll be glad if you could provide some information on proceeding with this.
--
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