Skip to content

Quit RV controllers upon TEC recreation#237

Closed
Jakob-Naucke wants to merge 1 commit intotrusted-execution-clusters:mainfrom
Jakob-Naucke:rv-controller-stop
Closed

Quit RV controllers upon TEC recreation#237
Jakob-Naucke wants to merge 1 commit intotrusted-execution-clusters:mainfrom
Jakob-Naucke:rv-controller-stop

Conversation

@Jakob-Naucke
Copy link
Copy Markdown
Contributor

to prevent racing of controllers on server side apply

Fixes: #216

@alicefr quirks discovered, lmk if they should be bugs:

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Apr 9, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Jakob-Naucke

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@alicefr
Copy link
Copy Markdown
Contributor

alicefr commented Apr 16, 2026

to prevent racing of controllers on server side apply

Fixes: #216

@alicefr quirks discovered, lmk if they should be bugs:

  • creating ApprovedImage without a TEC present will result in the image silently not being created

Approved images are a bit independent from TEC, so I think they could be created head

We can debate a bit on this. But I think ApprovedImages can be defined by the cluster admin manually or by another operator so they shouldn't be cleaned up by our operator. Although, in the test probably they remain

@alicefr
Copy link
Copy Markdown
Contributor

alicefr commented Apr 17, 2026

I think we need to review if the approved image should be owned by TEC. I personally don't think so anymore. Then, I don't think this solution is required

@Jakob-Naucke
Copy link
Copy Markdown
Contributor Author

Jakob-Naucke commented Apr 21, 2026

  • creating ApprovedImage without a TEC present will result in the image silently not being created

is a quirk of this fix, not an existing one because the watcher should quit on TEC deletion not on recreation. converting to draft.

cannot reproduce correction: when deleting TEC before operator was running, the ApprovedImage will not have been adopted yet. this is probably expected.

@Jakob-Naucke Jakob-Naucke marked this pull request as draft April 21, 2026 13:18
to prevent racing of controllers on server side apply. Skip
computation on creation if no new TEC exists yet (cannot quit
ApprovedImage controller on TEC deletion as its finalizer must still
run).

Add a test case (includes small refactoring for constants and imports).

Fixes: trusted-execution-clusters#216

Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
Assisted-by: Claude
@Jakob-Naucke
Copy link
Copy Markdown
Contributor Author

Updated, back out of draft. Watcher should not quit on TEC deletion because it won't be able to remove the finalizer, instead, do not attempt computation on addition if no TEC present. @alicefr PTAL or delay that look until after CI revival if you prefer

@Jakob-Naucke Jakob-Naucke marked this pull request as ready for review April 22, 2026 08:51
@alicefr
Copy link
Copy Markdown
Contributor

alicefr commented Apr 22, 2026

Updated, back out of draft. Watcher should not quit on TEC deletion because it won't be able to remove the finalizer, instead, do not attempt computation on addition if no TEC present. @alicefr PTAL or delay that look until after CI revival if you prefer

@Jakob-Naucke why do we need TEC for the computation? Is because the image of compute PCR is there?

@Jakob-Naucke
Copy link
Copy Markdown
Contributor Author

Indeed, the image-pcrs configmap is owned by the TEC object, so the computed PCRs have nowhere to go without a TEC present

@alicefr
Copy link
Copy Markdown
Contributor

alicefr commented Apr 22, 2026

Then maybe we could prioritize this #215 , and make approved images independent from the TEC CR

UPDATE: what if we just create the configmap independently of TEC and then let adopted by TEC CR when created

@alicefr
Copy link
Copy Markdown
Contributor

alicefr commented Apr 22, 2026

In general, I think it is easier to react when objects are created rather the retroactively. Otherwise, we need a way to understand when the approved images existed but they weren't calculated yet

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Apr 22, 2026

@Jakob-Naucke: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/operator-lifecycle-verify 36414af link true /test operator-lifecycle-verify

Full PR test history. Your PR dashboard.

Details

Instructions 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.

@Jakob-Naucke
Copy link
Copy Markdown
Contributor Author

Closing in favour of #246

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] ApprovedImage Creation Fails with HTTP 422 Due to blockOwnerDeletion + Finalizer Conflict

2 participants