wmedvede commented on code in PR #309:
URL:
https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/309#discussion_r1402679834
##########
controllers/discovery/kubernetes_catalog.go:
##########
@@ -58,21 +72,97 @@ func (c k8sServiceCatalog) resolveServiceQuery(ctx
context.Context, uri Resource
}
func (c k8sServiceCatalog) resolvePodQuery(ctx context.Context, uri
ResourceUri, outputFormat string) (string, error) {
- if pod, service, err := findPodAndReferenceServiceByPodLabels(ctx,
c.Client, uri.Namespace, uri.Name); err != nil {
+ if pod, serviceList, err := findPodAndReferenceServices(ctx, c.Client,
uri.Namespace, uri.Name); err != nil {
return "", err
} else {
- if service != nil {
- if serviceUri, err := resolveServiceUri(service,
uri.GetPort(), outputFormat); err != nil {
+ if serviceList != nil && len(serviceList.Items) > 0 {
+ referenceService :=
selectBestSuitedServiceByCustomLabels(serviceList, uri.CustomLabels)
+ return resolveServiceUri(referenceService,
uri.GetPort(), outputFormat)
+ } else {
+ return resolvePodUri(pod, "", uri.GetPort(),
outputFormat)
+ }
+ }
+}
+
+func (c k8sServiceCatalog) resolveDeploymentQuery(ctx context.Context, uri
ResourceUri, outputFormat string) (string, error) {
Review Comment:
We can change if we believe that we have a better option, no problem, but
let's double check.
Let me explain the calculation:
1) we have a service, all good, we use it.
2) No service is defined?
Ok, we try to pick a pod, but not assuming there is one, we do several
checks instead, look:
No replicas ? deployment.Spec.Replicas == nil || *deployment.Spec.Replicas
== 0
we return a proper message.
Too many replicas: *deployment.Spec.Replicas > 1
we return a proper message.
Only one replica?
good!, we first try to find the corresponding ReplicaSet object, and if
found, we double check again how many pods exists, if it has 0, 1, or > 1 pods.
If no pods, or too many pods, we return a proper message.
Only if one pod exists, then we return the corresponding pod address.
The criteria was borrowed from this diagram:
So we basically do several checks, and only returns a pod if only one pod is
present.
I think it's a matter of how we define the semantic of the service
discovery, and here is where we can revisit it.
From the service discovery perspective, what does it mean to access a
Deployment?
**Proposal 1)** is what we have, i.e., access to it with the Service is
resent, or with the Pod, only if we have one pod etc.
Does it make sense?
**Pros:** A Deployment object might exists without an associated object, so
from this point of view, it make sense.
**Cons:** It can be argued that the solution is kind of limited, since only
deployments with only one pod can be accessed with the service discovery if no
service is present. Additionally, a Pod can go down at any time.
**Proposal 2)** we look for the Service, if we find it, all good, if not,
ok, we throw an error saying that no Service was found. This basically changes
a bit the semantic.
**Pros:** with the service, the associated pods can go up and down at any
time, the service always resolves how to pick one for a given request.
**Cons:** Well, if no service, no access. But it can be fair assumption
considering that having service is a recommended practice.
If we go with Proposal 2), we might probably need to apply same criteria for
the StatefulSet?, or, in the case of the stateful set, in the case of only 1
replica, we might return **always** the corresponding dns name of the
corresponding pod (no chance to return the IP), something like this:
**stateful-set-example-pod-0.stateful-set-example.test-discovery.svc.cluster.local:8080**
In the end, this DNS name, in cases we have 1 pod, will never change, so, if
pod goes down at any time, when restarted, the name DNS name will resolve the
instance name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
