WebPage better showcase Standalone Dependent Resource #1111
Conversation
| @@ -113,6 +118,16 @@ public WebPageReconciler(KubernetesClient kubernetesClient) { | |||
| kubernetesClient.services().inNamespace(ns).createOrReplace(desiredService); | |||
| } | |||
|
|
|||
| var existingIngress = context.getSecondaryResource(Ingress.class); | |||
| if (Boolean.TRUE.equals(webPage.getSpec().getExposed())) { | |||
| var desiredIngress = makeDesiredIngress(webPage); | |||
There was a problem hiding this comment.
Maybe we should put the desired state in the context as well?
There was a problem hiding this comment.
Hmm, I don't see why, or what would be the purpose/benefit?
it can change based on the actual custom resource values.
There was a problem hiding this comment.
The purpose would be to avoid that rather ugly makeDesiredIngress static method :)
Outside of the scope for this release, though, for sure.
There was a problem hiding this comment.
That is there just to share code between WebPageReconciler and WebPageStandaloneReconciler
There was a problem hiding this comment.
We should avoid recomputing the desired state several times.
There was a problem hiding this comment.
I don't get it, it's just computed once in single a run. We have to recompute on each run since the custom resource might change that means the desired state might change (generally speaking).
There was a problem hiding this comment.
But we can take a look later, pls create an issue if you think this is important to discuss.
There was a problem hiding this comment.
That's the kind of use case that the AbstractDependentResource.isCreatable method was supposed to fix: make it possible for dependents to be created based not only statically (whether the associated implementation implements Creator) but also dynamically based on the primary resource content. Not sure why you wanted to remove it, actually…
There was a problem hiding this comment.
I really don't understand what is the problem we are solving. This sample does not use dependent resources at all.
There was a problem hiding this comment.
Ah yes, the comment should have been on this line: https://github.com/java-operator-sdk/java-operator-sdk/pull/1111/files#diff-99e8e89794964dedb781ec1236a8c692751b8fd96ca460d1e8b017406f1fda8bR68 instead.
csviri
force-pushed
the
webpage-standalone
branch
from
0f34d36 to
988e13a
Compare
|
Kudos, SonarCloud Quality Gate passed! |
No description provided.