From 642084d10c8d67a8b573913842f10667c5554b39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 8 Oct 2024 17:23:52 +0200 Subject: [PATCH 1/2] improve: naming target resource selector method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also add a sample usage. Signed-off-by: Attila Mészáros --- .../dependent/AbstractDependentResource.java | 4 ++-- .../kubernetes/KubernetesDependentResource.java | 6 +++--- .../ExternalWithStateDependentResource.java | 2 +- ...ipleManagedDependentNoDiscriminatorConfigMap1.java | 11 +++++++++++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java index 63223a01ff..db69d8134b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java @@ -114,7 +114,7 @@ public Optional getSecondaryResource(P primary, Context

context) { if (secondaryResources.isEmpty()) { return Optional.empty(); } else { - return selectManagedSecondaryResource(secondaryResources, primary, context); + return selectTargetSecondaryResource(secondaryResources, primary, context); } } @@ -132,7 +132,7 @@ public Optional getSecondaryResource(P primary, Context

context) { * @throws IllegalStateException if more than one candidate is found, in which case some other * mechanism might be necessary to distinguish between candidate secondary resources */ - protected Optional selectManagedSecondaryResource(Set secondaryResources, P primary, + protected Optional selectTargetSecondaryResource(Set secondaryResources, P primary, Context

context) { R desired = desired(primary, context); var targetResources = secondaryResources.stream().filter(r -> r.equals(desired)).toList(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index e629feb329..7b228df881 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -221,9 +221,9 @@ protected void addSecondaryToPrimaryMapperAnnotations(R desired, P primary, Stri } @Override - protected Optional selectManagedSecondaryResource(Set secondaryResources, P primary, + protected Optional selectTargetSecondaryResource(Set secondaryResources, P primary, Context

context) { - ResourceID managedResourceID = managedSecondaryResourceID(primary, context); + ResourceID managedResourceID = targetSecondaryResourceID(primary, context); return secondaryResources.stream() .filter(r -> r.getMetadata().getName().equals(managedResourceID.getName()) && Objects.equals(r.getMetadata().getNamespace(), @@ -239,7 +239,7 @@ protected Optional selectManagedSecondaryResource(Set secondaryResources, * @param context of current reconciliation * @return id of the target managed resource */ - protected ResourceID managedSecondaryResourceID(P primary, Context

context) { + protected ResourceID targetSecondaryResourceID(P primary, Context

context) { return ResourceID.fromResource(desired(primary, context)); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstate/ExternalWithStateDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstate/ExternalWithStateDependentResource.java index 9bdeea93cd..47d6a25144 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstate/ExternalWithStateDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstate/ExternalWithStateDependentResource.java @@ -40,7 +40,7 @@ public Set fetchResources( } @Override - protected Optional selectManagedSecondaryResource( + protected Optional selectTargetSecondaryResource( Set secondaryResources, ExternalStateCustomResource primary, Context context) { var id = getResourceID(primary); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/multipledrsametypenodiscriminator/MultipleManagedDependentNoDiscriminatorConfigMap1.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/multipledrsametypenodiscriminator/MultipleManagedDependentNoDiscriminatorConfigMap1.java index 750f8489b7..4e342d5351 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/multipledrsametypenodiscriminator/MultipleManagedDependentNoDiscriminatorConfigMap1.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/multipledrsametypenodiscriminator/MultipleManagedDependentNoDiscriminatorConfigMap1.java @@ -8,6 +8,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; +import io.javaoperatorsdk.operator.processing.event.ResourceID; @KubernetesDependent public class MultipleManagedDependentNoDiscriminatorConfigMap1 @@ -20,6 +21,16 @@ public MultipleManagedDependentNoDiscriminatorConfigMap1() { super(ConfigMap.class); } + // This is to showcase optimization to not compute the whole desired state only the target + // resource ID. In this particular case this would not be necessary, since desired state creation + // is pretty light weigh. Use it in more heavyweight situations. + protected ResourceID targetSecondaryResourceID( + MultipleManagedDependentNoDiscriminatorCustomResource primary, + Context context) { + return new ResourceID(primary.getMetadata().getName() + NAME_SUFFIX, + primary.getMetadata().getNamespace()); + } + @Override protected ConfigMap desired(MultipleManagedDependentNoDiscriminatorCustomResource primary, Context context) { From 4ae3d778d78bb4e0d53f70552a20f62468d59b80 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 9 Oct 2024 19:11:02 +0200 Subject: [PATCH 2/2] refactor: wording Signed-off-by: Chris Laprun --- ...ultipleManagedDependentNoDiscriminatorConfigMap1.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/multipledrsametypenodiscriminator/MultipleManagedDependentNoDiscriminatorConfigMap1.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/multipledrsametypenodiscriminator/MultipleManagedDependentNoDiscriminatorConfigMap1.java index 4e342d5351..6b83593173 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/multipledrsametypenodiscriminator/MultipleManagedDependentNoDiscriminatorConfigMap1.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/multipledrsametypenodiscriminator/MultipleManagedDependentNoDiscriminatorConfigMap1.java @@ -21,9 +21,12 @@ public MultipleManagedDependentNoDiscriminatorConfigMap1() { super(ConfigMap.class); } - // This is to showcase optimization to not compute the whole desired state only the target - // resource ID. In this particular case this would not be necessary, since desired state creation - // is pretty light weigh. Use it in more heavyweight situations. + /* + * Showcases optimization to avoid computing the whole desired state by providing the ResourceID + * of the target resource. In this particular case this would not be necessary, since desired + * state creation is pretty lightweight. However, this might make sense in situation where the + * desired state is more costly + */ protected ResourceID targetSecondaryResourceID( MultipleManagedDependentNoDiscriminatorCustomResource primary, Context context) {