-
Notifications
You must be signed in to change notification settings - Fork 223
fix: startup all resource indexing #2881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 22 commits
fa459d9
12c2168
abde7f6
6f5e6e3
37fc1e0
240099a
1e771e2
1da8b91
53c2155
08c0d5e
e5aef30
0c1b210
3d797ce
e003db3
17d40cc
9e41f50
faf9bdf
f9aafb8
f862506
d6c1a97
0c81798
559845a
81073ca
e0acdbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package io.javaoperatorsdk.operator.baseapi.startsecondaryaccess; | ||
|
||
import io.fabric8.kubernetes.api.model.Namespaced; | ||
import io.fabric8.kubernetes.client.CustomResource; | ||
import io.fabric8.kubernetes.model.annotation.Group; | ||
import io.fabric8.kubernetes.model.annotation.ShortNames; | ||
import io.fabric8.kubernetes.model.annotation.Version; | ||
|
||
@Group("sample.javaoperatorsdk") | ||
@Version("v1") | ||
@ShortNames("ssac") | ||
public class StartupSecondaryAccessCustomResource extends CustomResource<Void, Void> | ||
implements Namespaced {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
package io.javaoperatorsdk.operator.baseapi.startsecondaryaccess; | ||
|
||
import java.util.Map; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.RegisterExtension; | ||
|
||
import io.fabric8.kubernetes.api.model.ConfigMap; | ||
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; | ||
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; | ||
|
||
import static io.javaoperatorsdk.operator.baseapi.startsecondaryaccess.StartupSecondaryAccessReconciler.LABEL_KEY; | ||
import static io.javaoperatorsdk.operator.baseapi.startsecondaryaccess.StartupSecondaryAccessReconciler.LABEL_VALUE; | ||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.awaitility.Awaitility.await; | ||
|
||
class StartupSecondaryAccessIT { | ||
|
||
public static final int SECONDARY_NUMBER = 200; | ||
|
||
@RegisterExtension | ||
static LocallyRunOperatorExtension extension = | ||
LocallyRunOperatorExtension.builder() | ||
.withReconciler(new StartupSecondaryAccessReconciler()) | ||
.withBeforeStartHook( | ||
ex -> { | ||
var primary = new StartupSecondaryAccessCustomResource(); | ||
primary.setMetadata(new ObjectMetaBuilder().withName("test1").build()); | ||
primary = ex.serverSideApply(primary); | ||
|
||
for (int i = 0; i < SECONDARY_NUMBER; i++) { | ||
ConfigMap cm = new ConfigMap(); | ||
cm.setMetadata( | ||
new ObjectMetaBuilder() | ||
.withLabels(Map.of(LABEL_KEY, LABEL_VALUE)) | ||
.withNamespace(ex.getNamespace()) | ||
.withName("cm" + i) | ||
.build()); | ||
cm.addOwnerReference(primary); | ||
ex.serverSideApply(cm); | ||
} | ||
}) | ||
.build(); | ||
|
||
@Test | ||
void reconcilerSeeAllSecondaryResources() { | ||
var reconciler = extension.getReconcilerOfType(StartupSecondaryAccessReconciler.class); | ||
|
||
await().untilAsserted(() -> assertThat(reconciler.isReconciled()).isTrue()); | ||
|
||
assertThat(reconciler.isSecondaryAndCacheSameAmount()).isTrue(); | ||
xstefank marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
package io.javaoperatorsdk.operator.baseapi.startsecondaryaccess; | ||
|
||
import java.util.List; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import io.fabric8.kubernetes.api.model.ConfigMap; | ||
import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; | ||
import io.javaoperatorsdk.operator.api.reconciler.Context; | ||
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; | ||
import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; | ||
import io.javaoperatorsdk.operator.api.reconciler.Reconciler; | ||
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; | ||
import io.javaoperatorsdk.operator.processing.event.source.EventSource; | ||
import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; | ||
|
||
import static io.javaoperatorsdk.operator.baseapi.startsecondaryaccess.StartupSecondaryAccessIT.SECONDARY_NUMBER; | ||
|
||
@ControllerConfiguration | ||
public class StartupSecondaryAccessReconciler | ||
implements Reconciler<StartupSecondaryAccessCustomResource> { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(StartupSecondaryAccessReconciler.class); | ||
|
||
public static final String LABEL_KEY = "app"; | ||
public static final String LABEL_VALUE = "secondary-test"; | ||
|
||
private InformerEventSource<ConfigMap, StartupSecondaryAccessCustomResource> cmInformer; | ||
|
||
private boolean secondaryAndCacheSameAmount = true; | ||
private boolean reconciled = false; | ||
|
||
@Override | ||
public UpdateControl<StartupSecondaryAccessCustomResource> reconcile( | ||
StartupSecondaryAccessCustomResource resource, | ||
Context<StartupSecondaryAccessCustomResource> context) { | ||
|
||
var secondary = context.getSecondaryResources(ConfigMap.class); | ||
var cached = cmInformer.list().toList(); | ||
|
||
log.info( | ||
"Secondary number: {}, cached: {}, expected: {}", | ||
secondary.size(), | ||
cached.size(), | ||
SECONDARY_NUMBER); | ||
|
||
if (secondary.size() != cached.size()) { | ||
secondaryAndCacheSameAmount = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not simply: secondaryAndCacheSameAmount = secondary.size() == cache.size(); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would not work if later is the same value would overwrite the false, so here the aim is that if once false therefore always false |
||
} | ||
reconciled = true; | ||
return UpdateControl.noUpdate(); | ||
} | ||
|
||
@Override | ||
public List<EventSource<?, StartupSecondaryAccessCustomResource>> prepareEventSources( | ||
EventSourceContext<StartupSecondaryAccessCustomResource> context) { | ||
cmInformer = | ||
new InformerEventSource<>( | ||
InformerEventSourceConfiguration.from( | ||
ConfigMap.class, StartupSecondaryAccessCustomResource.class) | ||
.withLabelSelector(LABEL_KEY + "=" + LABEL_VALUE) | ||
.build(), | ||
context); | ||
return List.of(cmInformer); | ||
} | ||
|
||
public boolean isSecondaryAndCacheSameAmount() { | ||
return secondaryAndCacheSameAmount; | ||
} | ||
|
||
public boolean isReconciled() { | ||
return reconciled; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ | |
class DependentResourceCrossRefIT { | ||
|
||
public static final String TEST_RESOURCE_NAME = "test"; | ||
public static final int EXECUTION_NUMBER = 50; | ||
public static final int EXECUTION_NUMBER = 250; | ||
|
||
@RegisterExtension | ||
LocallyRunOperatorExtension operator = | ||
|
@@ -44,6 +44,22 @@ void dependentResourceCanReferenceEachOther() { | |
assertThat(operator.get(Secret.class, TEST_RESOURCE_NAME + i)).isNotNull(); | ||
} | ||
}); | ||
|
||
for (int i = 0; i < EXECUTION_NUMBER; i++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems unrelated? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this test was failing with a different version of the fix, and improved it while investigating, I can make it as a separate PR. |
||
operator.delete(testResource(i)); | ||
} | ||
await() | ||
.timeout(Duration.ofSeconds(30)) | ||
.untilAsserted( | ||
() -> { | ||
for (int i = 0; i < EXECUTION_NUMBER; i++) { | ||
assertThat( | ||
operator.get( | ||
DependentResourceCrossRefResource.class, | ||
testResource(i).getMetadata().getName())) | ||
.isNull(); | ||
} | ||
}); | ||
} | ||
|
||
DependentResourceCrossRefResource testResource(int n) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might need a "before-shutdown" hook as well to perform clean-up if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is usually not needed strictly , since one can clean the resources at the end of the test, so functionally that is doable. But agree that might be a nicer api.