Expand ownership check for profile bundle controller#1098
Expand ownership check for profile bundle controller#1098
Conversation
The controller previously only watched ProfileBundle objects. When the profileparser Deployment's pods changed state, the controller was never notified. Adding Owns means any change to the owned Deployment triggers a reconciliation of the parent ProfileBundle, so the controller is responsive to pod lifecycle events. Also, once the controller found an existing pod with no startup error, it exited the controller reconcilation loop without requeue — regardless of whether the ProfileBundle was still in PENDING state. If the profileparser hadn't finished (or never ran due to a rollout delay), the controller would never check again. This commit also updates the profile bundle controller to requeues every 10 seconds while the status is still DataStreamPending, ensuring the controller keeps monitoring until the profileparser either succeeds (sets VALID) or fails (sets INVALID / pod startup error detected). This should improve the resilience of profile bundle parsing, especially in testing, where we delete deployments after modifying the profile bundle image to simulate operator updates. Assisted-By: Opus 4.6
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhmdnd The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Note for reviewers - we should recheck the serial tests several times to ensure they come back green consistently before we merge this. |
|
🤖 To deploy this PR, run the following command: |
|
@rhmdnd: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Serial tests passed on the first run, so that's a good sign. Parallel tests are failing and it looks like that failure is related to github.com//pull/1093 /test e2e-aws-serial-arm |
|
I tested with a special scenario, when setting profilebundle to a non-exist image. I can see the requeue working as expected. The only concern is the unconditional requeueing every 10 seconds while PENDING, for example this CrashLoopBackOff scenario when the profileparser container crashes repeatedly. |
|
I opened this using the wrong branch. Closing and submitting as a PR from my fork. |
|
Picking up the review as #1100 instead (I opened this using the wrong branch initially), otherwise 1100 is the exact same patch. |
2 participants
The controller previously only watched ProfileBundle objects. When the
profileparser Deployment's pods changed state, the controller was never
notified.
Adding Owns means any change to the owned Deployment triggers a
reconciliation of the parent ProfileBundle, so the controller is
responsive to pod lifecycle events.
Also, once the controller found an existing pod with no startup error,
it exited the controller reconcilation loop without requeue — regardless
of whether the ProfileBundle was still in PENDING state. If the
profileparser hadn't finished (or never ran due to a rollout delay), the
controller would never check again.
This commit also updates the profile bundle controller to requeues every
10 seconds while the status is still DataStreamPending, ensuring the
controller keeps monitoring until the profileparser either succeeds
(sets VALID) or fails (sets INVALID / pod startup error detected).
This should improve the resilience of profile bundle parsing, especially
in testing, where we delete deployments after modifying the profile
bundle image to simulate operator updates.
Assisted-By: Opus 4.6