Repository navigation
Conversation
I add an informational condition and kubectl column to show whether an Integration has an associated CamelMonitor. I verify direct ownership or the bounded Deployment/CronJob chain by namespace, name and UID. I reuse unstructured lookup and optional watches to keep the discovery condition current without changing runtime readiness. I add focused regression tests, minimal read permissions and regenerated CRD/Helm assets. Fixes apache#6898
squakez
left a comment
squakez
left a comment
There was a problem hiding this comment.
Thanks for the PR. I think the logic can be simplified.
We should "watch" the CamelMonitor resources (when installed) and trigger a drift if a cmon is either created or removed. Then, in the monitoring state, the existing monitor action should just add or remove the condition accordingly.
| // +kubebuilder:storageversion | ||
| // +kubebuilder:subresource:scale:specpath=.spec.replicas,statuspath=.status.replicas,selectorpath=.status.selector | ||
| // +kubebuilder:printcolumn:name="Phase",type=string,JSONPath=`.status.phase`,description="The integration phase" | ||
| // +kubebuilder:printcolumn:name="Camel Monitor",type=string,JSONPath=`.status.conditions[?(@.type=="CamelMonitorAvailable")].status`,description="Whether an associated CamelMonitor resource is available" |
There was a problem hiding this comment.
Better move as last column
I let CamelMonitor events use normal reconciliation and update the availability condition from the existing monitor action. I remove the separate status patch and fixed retry from the reconciler. I clear the condition when the association disappears and keep lookup errors from interrupting normal runtime monitoring. I move the kubectl column last and update the regression tests and documentation. Fixes apache#6898
|
I've pushed the requested update: the column is last, and the condition uses the existing monitor action. The focused tests pass; the known baseline failure is documented in the description. Could you approve the pending workflows and take another look once CI has run? I write it, AI helps with my English (@manduinca). |
| // updateCamelMonitorCondition reports resource discovery, independently of runtime readiness. | ||
| // Lookup failures use the existing action error/retry path. | ||
| func updateCamelMonitorCondition(ctx context.Context, c ctrl.Reader, integration *v1.Integration) error { | ||
| monitor, err := kubernetes.GetUnstructured(ctx, c, camelMonitorGVK, integration.Name, integration.Namespace) |
There was a problem hiding this comment.
We should avoid to use the "unstructured" type and use instead the api coming from https://github.com/camel-tooling/camel-monitor-operator project.
Sorry, it does not seem to go in the right direction. I have the feeling this work require a bit more knowledge on the foundation for the project. I'm removing the "good first issue" as it does not seem to be that easy as we originally thought. |
I add the
CamelMonitorAvailablecondition and a finalCamel Monitorkubectl column to show whether an Integration has an associated monitor. I keep resource discovery independent of runtime readiness.I let optional CamelMonitor watches enqueue the Integration and update the condition through the existing monitor action and status update path. I remove the condition when the resource is absent, unassociated, or its API is not installed. Lookup failures report
Unknownand use normal controller retries; they do not prevent the runtime monitoring checks from running.I verify the monitor's name, namespace and owner UID, either directly through the Integration or through its Deployment/CronJob. I reuse the unstructured lookup helper and handle creation, deletion and ownership changes, including non-controller owners. The lookup follows the same-name contract; it does not discover revision-suffixed monitors.
I register optional watches after checking discovery and permissions for each effective namespace. I add only
get/list/watchpermissions for CamelMonitor and regenerate the CRD and Helm artifacts. I document the restart needed when the CRD or watch permissions are added after operator startup.Validation:
make generate,make update-docs,make fmt,make goimportandmake lintpass; lint reports 0 issues. The source and generated CRD/Helm column order match.make testrun reports 1,421 passes, one skip and one failure:TestGitCloneCommitin the unchanged builder package. The sameworktree contains unstaged changesfailure was reproduced in a clean worktree atmain(a27343186).Fixes #6898
For the code, AI directed by me (@manduinca). I write it, AI helps with my English.