Skip to content

Use reflector for all repeated read operations & set kube timeouts less than test timeouts, CI stress edition - #330

Draft
Jakob-Naucke wants to merge 5 commits into
trusted-execution-clusters:mainfrom
Jakob-Naucke:get-cached-ci
Draft

Use reflector for all repeated read operations & set kube timeouts less than test timeouts, CI stress edition#330
Jakob-Naucke wants to merge 5 commits into
trusted-execution-clusters:mainfrom
Jakob-Naucke:get-cached-ci

Conversation

@Jakob-Naucke

@Jakob-Naucke Jakob-Naucke commented Aug 10, 2026

Copy link
Copy Markdown
Member

Summary by Sourcery

Unify controller context around a shared OperatorContext using reflector-backed caches, switch Kubernetes write operations to server-side apply with a bounded write timeout, and adjust CI integration tests for stress-style repeated runs and updated images.

New Features:

  • Introduce a shared OperatorContext struct that exposes cached access to core Kubernetes resources across all controllers.
  • Add helper logic to read image PCRs from the ConfigMap cache and apply PCR updates via server-side apply patches.
  • Extend image and job controllers to track related Job resources and use cached ApprovedImage and ConfigMap state.

Bug Fixes:

  • Avoid failing secret mount/unmount and attestation key updates when the trustee deployment or TEC resources are absent by treating missing cached objects as benign where appropriate.
  • Ensure image disallow/allow flows and reference value recomputation operate correctly even when PCR ConfigMaps are missing or partially populated.

Enhancements:

  • Refactor attestation key, registration server, trustee, and reference value controllers to take OperatorContext instead of raw clients and to rely on reflector stores for read paths.
  • Change deployment and ConfigMap mutations to use Patch::Apply with force-enabled PatchParams instead of full object replacement.
  • Tighten Kubernetes client write_timeout to a fixed duration shorter than test timeouts to avoid hanging writes under stress.
  • Improve test utilities with timestamped log output and adapt unit tests to the new cached, patch-based update behavior.
  • Update default test and approved images in the Makefile and enable human-readable timestamps in env_logger.

CI:

  • Revise integration-tests workflow to use a fixed external registry/tag, drop image build-and-push, remove the global timeout, loop integration tests for stress, and always collect and upload must-gather artifacts.

@openshift-ci

openshift-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

[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

@sourcery-ai

sourcery-ai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Reviewer's Guide

Refactors the operator to use a unified OperatorContext backed by reflector caches for repeated reads, switches several config/deployment mutations to server-side apply patches, adjusts controllers and tests accordingly, and tightens CI and runtime timeouts for more reliable stress testing.

Sequence diagram for keygen_reconcile using reflector-backed OperatorContext

sequenceDiagram
    actor Operator
    participant Machine
    participant KeygenController as keygen_reconcile
    participant OperatorContext
    participant Trustee as trustee
    participant TecStore as tec_store

    Operator->>Machine: create/update Machine
    Machine-->>KeygenController: event

    KeygenController->>OperatorContext: access client
    KeygenController->>Machine: inspect spec.id
    alt Event::Apply
        KeygenController->>Machine: generate_owner_reference(machine)
        KeygenController->>trustee: generate_secret(client, id, owner_reference)
        KeygenController->>trustee: mount_secret(OperatorContext, id)
        KeygenController-->>Operator: Action::await_change
    else Event::Cleanup
        KeygenController->>Machine: read owner_references
        KeygenController->>TecStore: get(ObjectRef::new(tec_name).within(ns))
        alt TEC deleting or missing
            KeygenController-->>Operator: Action::await_change
        else TEC active
            KeygenController->>trustee: unmount_secret(OperatorContext, id)
            KeygenController-->>Operator: Action::await_change
        end
    end
Loading

File-Level Changes

Change Details Files
Introduce a unified OperatorContext with reflector-backed stores and use it across controllers and trustee helpers instead of ad‑hoc contexts or direct GETs.
  • Define OperatorContext with cached stores for TrustedExecutionCluster, ConfigMap, Deployment, Machine, AttestationKey, Secret, and ApprovedImage
  • Add helper get_opt_tec on OperatorContext to return the single TrustedExecutionCluster or error on multiple
  • Replace AkContextData and ClusterContext usages with OperatorContext in attestation_key_register, reference_values, main, register_server, and trustee call sites
operator/src/lib.rs
operator/src/main.rs
operator/src/attestation_key_register.rs
operator/src/reference_values.rs
operator/src/register_server.rs
operator/src/trustee.rs
Switch ConfigMap and Deployment mutations to use Patch::Apply with PatchParams::apply and cached objects, reducing repeated reads and improving idempotency.
  • Change update_reference_values to accept ImagePcrs and patch TRUSTEE_DATA_MAP via server-side apply instead of GET+replace
  • Change do_mount_secret to operate on cached Deployment from deployment_store and patch volumes/volumeMounts via server-side apply
  • Update update_attestation_keys to use Patch::Apply with PatchParams and cached Deployment
  • Introduce apply_image_pcrs to patch PCR_CONFIG_MAP via server-side apply and call update_reference_values, removing the update_image_pcrs macro
  • Adjust compute-pcrs main to inline the previous macro logic for PCR_CONFIG_MAP update
operator/src/trustee.rs
operator/src/reference_values.rs
compute-pcrs/src/main.rs
lib/src/reference_values.rs
Refactor reference value and image controllers to consume cached data from OperatorContext stores, reduce API calls, and wire ownership between ApprovedImage and Job controllers.
  • Add cached_image_pcrs helper to read ImagePcrs from cm_store
  • Refactor job_reconcile to take OperatorContext, delete completed Jobs, read PCRs from cache, and call update_reference_values with ImagePcrs
  • Refactor launch_rv_job_controller and launch_rv_image_controller to accept OperatorContext, and have the image controller own Jobs via watcher labels
  • Change adopt_approved_images, image_reconcile, image_add_reconcile, image_remove_reconcile, handle_new_image, and disallow_image to use OperatorContext and cached stores instead of listing or GETing resources
operator/src/reference_values.rs
Adjust keygen and attestation-key controllers to use OperatorContext caches for Machines, Secrets, Deployments, and TrustedExecutionCluster state.
  • Update ak_reconcile, machine_reconcile, secret_reconcile, and approve_ak to accept OperatorContext and use secret_store/deployment_store
  • Update keygen_reconcile to accept OperatorContext, use tec_store for TEC deletion checks, and call trustee mount/unmount helpers with OperatorContext
  • Update launch_*_controller functions to pass OperatorContext instead of raw Client
operator/src/attestation_key_register.rs
operator/src/register_server.rs
Rework main initialization to create all reflectors up front, share them via OperatorContext, and configure a bounded kube write timeout.
  • Infer kube::Config, set write_timeout to 30 seconds, and construct Client from it
  • Create reflector stores/writers for all relevant types (TrustedExecutionCluster, ConfigMap, Deployment, Machine, AttestationKey, Secret, ApprovedImage) and spawn_reflector for each
  • Populate OperatorContext with these stores, perform best-effort sync_cache calls for each store via a macro, and pass Arc into all controller launch functions
  • Start register_server, attestation_key_register, and reference_values controllers using the shared context
operator/src/main.rs
operator/src/lib.rs
Update unit tests to reflect cache-based access and patch-based mutations, including new helper contexts and expectations on HTTP methods.
  • Add helpers to build OperatorContext with pre-populated reflector stores for Deployments, ConfigMaps, ApprovedImages, and TrustedExecutionClusters
  • Change tests in trustee, reference_values, register_server, main, and attestation_key_register to use OperatorContext, expect PATCH instead of GET/PUT, and adjust interaction counts
  • Ensure tests that previously relied on direct API GETs now either use cached state or assert that no API calls occur
operator/src/trustee.rs
operator/src/reference_values.rs
operator/src/register_server.rs
operator/src/main.rs
Tighten CI integration tests for stress runs, add must-gather collection, and adjust logging/timeouts for better observability.
  • Change REGISTRY and TAG env defaults and update TEST_IMAGE tag in Makefile
  • Remove workflow timeout-minutes, stop building/pushing images in CI, and run integration-tests in a loop until failure, then gather and upload must-gather artifacts
  • Enhance test_utils logging macros to prefix messages with UTC timestamps and enable env_logger humantime feature
.github/workflows/integration-tests.yml
Makefile
test_utils/src/lib.rs
Cargo.toml

Possibly linked issues

  • #0: The PR introduces OperatorContext and reflector stores to cache resources, replacing direct get/list calls as requested.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Extend the use of a reflector cache from attestation keys to the
entire operator. Replace AkContextData with OperatorContext to pass
caches. Use server-side apply patches instead of get+replace.

- Updating image-pcrs without the cache is only used in compute-pcrs
  now, thus inline the macro
- trustee::update_reference_values receives PCRs from argument, it is
  sometimes available from the call site anyhow

Fixes: trusted-execution-clusters#251

Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
Assisted-by: AI
so that status is updated in a timely manner

Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
Read defaults to None, write to 295s. Set 30s which is less than test
timeouts, so each test case has a chance for a retry.

Note that since kube-rs v4, the None read_timeout is no longer
required for watchers (which we do use), although still for exec &
attach (which we do not).

Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
@Jakob-Naucke Jakob-Naucke changed the title Use reflector for all repeated read operations & set write timeout less than test timeouts, CI stress edition Use reflector for all repeated read operations & set kube timeouts less than test timeouts, CI stress edition Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant