Uh oh!
There was an error while loading. Please reload this page.
test: sample for virtual threads - #3554
Closed
csviri wants to merge 21 commits into
Closed
Conversation
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
…us (operator-framework#3480) Add integration tests that showcases handling explicit state in status. Both for simple managed/secondary resources and using a dependent resource. Signed-off-by: Attila Mészáros <a_meszaros@apple.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* feat: pool informers so controllers and event sources can share them Every InformerEventSource used to create its own SharedIndexInformer, so an operator whose controllers all watch the same secondary type - ConfigMap and Secret being the usual suspects - opened one watch connection and kept one cache per controller for the very same resources. Informers are now handed out by an InformerPool obtained from the ConfigurationService, keyed by an InformerClassifier. Event sources whose classifiers are equal are backed by one informer; the pool reference counts its users and stops the informer once the last one releases it. The classifier is made up of everything that decides what an informer watches and how: the KubernetesClient instance (compared by identity, since two clients for the same API server may still differ in credentials, impersonation or TLS material), the resource class or the group/version/kind for generic resources, the namespace, the label, field and shard selectors, and the item store. Two components are deliberately not part of that identity. The informerListLimit is excluded, so event sources that disagree only on it still share an informer, keeping the limit of whichever one created it and logging a warning. Indexers are excluded because they can be added to a running informer: they are registered under a name qualified with the controller and event source that added them, so index names stay private to an event source while callers keep using their own names, and they are removed again when that event source releases the informer. Two strategies ship: DefaultInformerPool shares as described and is the default, NonSharingInformerPool creates a dedicated informer per event source for anyone wanting to opt out. Either is selected with ConfigurationServiceOverrider#withInformerPool, and a custom strategy extends AbstractInformerPool, which already creates the informers from a classifier, starts them and waits for their caches to sync, leaving the subclass only the question of whether and when an informer is shared. Consequently informer creation and startup moved out of InformerWrapper and InformerManager into the pool, InformerManager acquires and releases informers instead of owning them, and it removes its own event handler and indexers from an informer that keeps running for others. An event source registered dynamically against an already running shared informer needs no special handling: the client replays the cache contents to a newly added handler. Also in support of the above: ConfigurationService#informerPool, an InformerEventSource constructor that no longer needs an EventSourceContext (the one taking it is deprecated), the resource group/version/kind on InformerConfiguration, equality and toString on FieldSelector, and equality of GroupVersionKindPlural made consistent with its hashCode so that an unspecified plural no longer splits informers. The pooling itself is production ready; the configuration API around it is marked experimental and may still change. Covered by unit tests for the pools, the classifier, the wrapper and the manager, and by integration tests for sharing, dynamic registration and de-registration that each run against both strategies. Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
…-framework#3511) Bumps [org.apache.maven.plugins:maven-jar-plugin](https://github.com/apache/maven-jar-plugin) from 3.5.0 to 3.5.1. - [Release notes](https://github.com/apache/maven-jar-plugin/releases) - [Commits](apache/maven-jar-plugin@maven-jar-plugin-3.5.0...maven-jar-plugin-3.5.1) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-jar-plugin dependency-version: 3.5.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…-framework#3510) Bumps `openrewrite.version` from 8.87.5 to 8.87.7. Updates `org.openrewrite:rewrite-java` from 8.87.5 to 8.87.7 - [Release notes](https://github.com/openrewrite/rewrite/releases) - [Commits](openrewrite/rewrite@v8.87.5...v8.87.7) Updates `org.openrewrite:rewrite-maven` from 8.87.5 to 8.87.7 - [Release notes](https://github.com/openrewrite/rewrite/releases) - [Commits](openrewrite/rewrite@v8.87.5...v8.87.7) Updates `org.openrewrite:rewrite-test` from 8.87.5 to 8.87.7 - [Release notes](https://github.com/openrewrite/rewrite/releases) - [Commits](openrewrite/rewrite@v8.87.5...v8.87.7) Updates `org.openrewrite:rewrite-java-17` from 8.87.5 to 8.87.7 - [Release notes](https://github.com/openrewrite/rewrite/releases) - [Commits](openrewrite/rewrite@v8.87.5...v8.87.7) --- updated-dependencies: - dependency-name: org.openrewrite:rewrite-java dependency-version: 8.87.7 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.openrewrite:rewrite-maven dependency-version: 8.87.7 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.openrewrite:rewrite-test dependency-version: 8.87.7 dependency-type: direct:development update-type: version-update:semver-patch - dependency-name: org.openrewrite:rewrite-java-17 dependency-version: 8.87.7 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…8.0 to 3.9.0 (operator-framework#3515) Signed-off-by: dependabot[bot] <support@github.com>
…erator-framework#3514) Fabric8's no-context patch() fetches the resource from the server and computes an RFC 6902 JSON Patch diff, which removes the fields omitted from a partial resource (including the whole spec). Pass an explicit JSON_MERGE PatchContext so the non-status merge patch operations really send application/merge-patch+json.
…ator E2E test (operator-framework#3513) Adds a Maven-compiled Kotlin test source set to operator-framework-core and a smoke test verifying that a checked (non-RuntimeException) Exception thrown from a Kotlin DependentResource is properly caught and reported by the workflow executor, so that retries are triggered as expected. Kotlin does not have checked exceptions, so Kotlin code can throw a checked Exception from an overridden method without declaring it, even though the Java DependentResource#reconcile signature does not declare `throws Exception`. Before operator-framework#2965 this exception would not have been caught by NodeExecutor, since it only handled RuntimeException, silently swallowing the error and preventing retries. Closesoperator-framework#2967 * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: close class brace in Kotlin test; add Kotlin sample operator E2E Fixes a missing closing brace left in KotlinCheckedExceptionDependentResourceTest.kt. Adds a new sample-operators/kotlin-operator module: a minimalist Kotlin operator (ConfigMapCopyReconciler) that copies a value from a custom resource's spec into a ConfigMap, with a dual-mode (local/remote) E2E test mirroring the other samples. This verifies deserialization via the fabric8 client and the full reconciliation runtime when both the CR and reconciler are implemented in Kotlin, addressing the maintainer's request for broader coverage than the existing unit-level smoke test. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…-framework#3516) Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This needed to be fixed after rebase. Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
…work#3547) AbstractWorkflowExecutor called Workflow#getDependentResourcesByName purely to read its size. That allocates a HashMap and walks every node to collect the dependent resources, then discards the map. The executor is constructed on every reconcile and cleanup of a workflow-based reconciler, so use the existing Workflow#size instead.
…perator-framework#3550) ResourceOperations#desiredForJsonPatch takes an Options argument that it never reads - it only clones the actual resource and applies the operator. All six jsonPatch call sites thread the value in for nothing, which suggests the method still honours the option. Remove the parameter.
…ator-framework#3544) No behavior change; each site is replaced by a helper that already exists. - PrimaryUpdateAndCacheUtils#compareResourceVersions (and its private validateResourceVersion) duplicated the entire algorithm of ReconcilerUtilsInternal#validateAndCompareResourceVersions: the length-first compare, the empty check, the leading-zero check and the same exception messages. It had no production caller, so the copy could silently drift from the version all production paths use. Delegate instead. - addFinalizerWithSSA builds its bare SSA skeleton with HasMetadata#initNameAndNamespaceFrom, which ResourceOperations already uses for the same purpose and which is Namespaced-aware. - AbstractInformerPool formats the informer identifier with ReconcilerUtilsInternal#getResourceTypeNameWithVersion instead of concatenating the resource name and version by hand. - EventFilterWindow uses ExtendedResourceEvent#getResourceVersion, which had no callers even though it is exactly the expression used here. - LocallyRunOperatorExtension instantiates reconcilers with Utils#instantiate, so it also supports non-public no-arg constructors and reports the failing class instead of wrapping in a bare RuntimeException.
…perator-framework#3543) The event count is increased at the beginning of the reconciliation, thus waiting for it released the test into the middle of a reconciliation that was still about to remove the finalizer. The subsequent update then raced with that removal and failed with a conflict. Wait for the finalizer removal to actually land, and retry the (optimistically locked) update with a fresh read on conflict.
…rk#3536) Add an opt-in, experimental detectApiVersionChange option on @KubernetesDependent that records the API version the operator applies in the javaoperatorsdk.io/last-applied-api-version annotation. The regular matcher then detects a mismatch when that marker differs from (or is missing relative to) the currently applied API version, causing a one-time update without triggering repeated reconciliations once the resource is up-to-date. Disabled by default, so existing behavior and matching are unaffected unless explicitly enabled. Guard against a null or immutable annotations map (e.g. Map.of(...)) on the desired resource when detectApiVersionChange is enabled, since writing the last-applied-api-version marker (and the pre-existing previous-annotation bookkeeping that runs alongside it) requires a mutable map. Also rename a misleadingly-named test helper and replace a no-op assertion on a primitive boolean with a concrete expectation. Addresses Copilot review feedback on PR operator-framework#3536. Add DetectApiVersionChangeIT covering the end-to-end scenario: a ConfigMap dependent resource configured with detectApiVersionChange is marked with the current API version on creation (without triggering an update), and a stale marker annotation left on the actual resource (simulating an older operator/CRD version) is detected and corrected with exactly one update, after which no further reconciliation loop occurs. Signed-off-by: hej090224 <fc49854985@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…ator-framework#3548) InformerManager is generic over C extends Informable, but getTargetClient type-tested for InformerEventSourceConfiguration to find out whether a specific (e.g. remote cluster) client was configured. Informable has two implementors and only one could answer the question, so a third configuration type wanting its own client would be ignored silently rather than failing to compile. Move the default getKubernetesClient() up from InformerEventSourceConfiguration to Informable and let InformerManager ask the configuration directly. The default still returns Optional.empty(), so existing implementations are unaffected. As a side effect the ConfigurationService client is now only created when no specific client is configured, instead of being created and then discarded.
…rk#3546) - GenericKubernetesResourceMatcher allocated the path-prefix lists (List.of(SPEC), List.of(METADATA), the labels/annotations pair, List.of(STATUS)) once per JSON-diff node while matching, and nodeIsChildOf built a stream per call. Both run for every node of every match, so hoist the lists to constants and use an indexed loop. - SSABasedGenericKubernetesResourceMatcher#sanitizeState nested the StatefulSet volume-claim-template handling four levels deep inside the type ladder; extract it into sanitizeVolumeClaimTemplates so the ladder reads as one dispatch per resource kind.
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Requires Java 21, therefore the module is only part of the build when the JDK used is at least version 21. Configures both the framework executors (reconciliation and workflow) and the Kubernetes client task executor with a virtual thread per task executor.
csviri
marked this pull request as ready for review
August 12, 2026 06:49
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new sample operator demonstrating how to run JOSDK reconciliations and Fabric8 client async callbacks on Java virtual threads (Java 21+), along with documentation and CI updates so the sample is built/tested appropriately.
Changes:
- Introduces a new
sample-operators/virtual-threadsMaven module (Java 21+) with a reconciler, CRD types, logging config, and an E2E test. - Adds a JDK-activated Maven profile in
sample-operators/pom.xmlto include the sample only on JDK 21+. - Updates docs and the E2E GitHub Actions workflow to reference and test the new sample.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sample-operators/virtual-threads/src/test/resources/log4j2.xml | Adds test logging configuration for the virtual threads sample. |
| sample-operators/virtual-threads/src/test/java/io/javaoperatorsdk/operator/sample/VirtualThreadsOperatorE2E.java | Adds an E2E test validating parallel reconciliations and virtual-thread execution. |
| sample-operators/virtual-threads/src/main/resources/log4j2.xml | Adds runtime logging configuration for the sample operator. |
| sample-operators/virtual-threads/src/main/java/io/javaoperatorsdk/operator/sample/VirtualThreadsStatus.java | Adds CR status fields used by the sample to report observed value and virtual-thread usage. |
| sample-operators/virtual-threads/src/main/java/io/javaoperatorsdk/operator/sample/VirtualThreadsSpec.java | Adds CR spec schema for the sample. |
| sample-operators/virtual-threads/src/main/java/io/javaoperatorsdk/operator/sample/VirtualThreadsReconciler.java | Implements a reconciler that simulates blocking work and patches status. |
| sample-operators/virtual-threads/src/main/java/io/javaoperatorsdk/operator/sample/VirtualThreadsOperator.java | Adds the sample operator entry point wiring virtual-thread executors and client config. |
| sample-operators/virtual-threads/src/main/java/io/javaoperatorsdk/operator/sample/VirtualThreadsCustomResource.java | Defines the sample CustomResource type and group/version metadata. |
| sample-operators/virtual-threads/src/main/java/io/javaoperatorsdk/operator/sample/VirtualThreads.java | Centralizes virtual-thread configuration for both framework executors and Fabric8 client task executor. |
| sample-operators/virtual-threads/README.md | Documents the sample, Java 21+ requirement, and how to run it. |
| sample-operators/virtual-threads/pom.xml | Introduces the new Java 21+ sample module build and dependencies. |
| sample-operators/virtual-threads/k8s/virtual-threads-custom-resource.yaml | Adds a sample CR manifest for running the operator manually. |
| sample-operators/virtual-threads/k8s/operator.yaml | Adds a deployment + RBAC manifest used by remote E2E mode. |
| sample-operators/pom.xml | Adds a JDK>=21-activated profile to include the new sample module. |
| docs/content/en/docs/getting-started/bootstrap-and-samples.md | Adds the virtual-threads sample to the list of available samples. |
| docs/content/en/docs/documentation/operations/configuration.md | Documents how to run JOSDK on virtual threads (framework + client executor). |
| .github/workflows/e2e-test.yml | Adds the new sample module to the E2E workflow matrix (running on JDK 25). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Comment on lines
+66
to
+70
| @Override | ||
| public void onClose(Executor executor) { | ||
| ((ExecutorService) executor).shutdownNow(); | ||
| } | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
csviri
marked this pull request as draft
August 12, 2026 08:54
Comment on lines
+30
to
+31
| can be replaced with virtual threads, so that a reconciliation waiting on a remote call does not | ||
| occupy a platform thread: |
Collaborator
There was a problem hiding this comment.
Suggested change
| can be replaced with virtual threads, so that a reconciliation waiting on a remote call does not | |
| occupy a platform thread: | |
| can be replaced with virtual threads. |
Each virtual thread always runs on platform thread in the background.
PR needs rebase. 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. |
csviriforce-pushed
the
next
branch
2 times, most recently
from
September 3, 2026 11:40
5ccac4d to
478199eCompare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds sample (serves as a smoke test) for virtual threads setup of JOSDK.
TODO: