-
Notifications
You must be signed in to change notification settings - Fork 218
feat: initial support for dependent resources #726
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
Conversation
395ddb1
to
d8013e8
Compare
|
||
public DefaultContext(RetryInfo retryInfo) { | ||
public DefaultContext(RetryInfo retryInfo, Controller<P> controller, P primaryResource) { |
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.
Should this go to primary context? Cannot be an implementation of Reconciler
that prodives these?
To have the current default APIs unchanged? Wouldn't that be possible?
401ada2
to
3e479bd
Compare
} | ||
|
||
@Override | ||
public <T extends HasMetadata> T getSecondaryResource(Class<T> expectedType, |
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.
What is difference between dependent and secondary resource?
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.
They're the same. I should probably pick one and stick to it 😄
Based on the question during the last operator sdk call, I think I will go with secondary
resources.
@@ -12,6 +13,6 @@ | |||
* @param eventSourceRegistry the {@link EventSourceRegistry} where event sources can be | |||
* registered. | |||
*/ | |||
void prepareEventSources(EventSourceRegistry<T> eventSourceRegistry); | |||
void prepareEventSources(EventSourceRegistry<T> eventSourceRegistry, Cloner cloner); |
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.
Why is the cloner needed here? Is not generic for the whole operator?
import io.javaoperatorsdk.operator.processing.event.source.ResourceCache; | ||
import io.javaoperatorsdk.operator.processing.event.source.ResourceEventSource; | ||
|
||
public class DependentResource<R extends HasMetadata, P extends HasMetadata> { |
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.
P is the parent? Or what it is used for?
count.getAndIncrement(); | ||
Thread.sleep(waitTime); | ||
} catch (InterruptedException e) { | ||
e.printStackTrace(); |
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.
Should be a log message, or even fail probably
81aeed6
to
e67efac
Compare
9e1e314
to
591c97f
Compare
Closing this in favor of #785 |
This PR provides annotation-based configuration of dependent resources, which the controller then automatically handles, taking care of propagating creation/update of said dependent resources depending on how they are configured. The functional scope is still pretty limited and more changes will be needed but this sets up the architecture for more changes.
In particular, handling of readiness / status reporting of dependent resources will also be tackled at some point.