proposal for native service discovery #3694
Conversation
karmada-bot
added
the
size/L
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #3694 +/- ##
=======================================
Coverage 56.61% 56.61%
=======================================
Files 221 221
Lines 20831 20831
=======================================
Hits 11794 11794
Misses 8413 8413
Partials 624 624
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Thanks a lot, @bivas I Just passed through the holiday season, and I am delighted to see your proposal. Let's work together to promote this proposal.
|
|
||
| 1. **Local** only - In case there's a local service by the name `foo` Karmada never attempts to import the remote service and doesn't create an `EndPointSlice` | ||
| 2. **Local** and **Remote** - Users accessing the `foo` service will reach either member1 or member2 | ||
| 3. **Remote** only - in case there's a local service by the name `foo` Karmada will remove the local `EndPointSlice` and will create an `EndPointSlice` pointing to the other cluster (e.g. instead of resolving member2 cluster is will reach member1) |
There was a problem hiding this comment.
Remote under what circumstances would this be used? I feel like we could consider temporarily not supporting this method.
There was a problem hiding this comment.
Remote can be used for cases like privacy or business consideration that doesn't allow processing of customer data in specific location and we must pass the traffic to privacy controlled cluster.
There was a problem hiding this comment.
For users, if they want to perform remote processing, why do they need to deploy a deployment in the current cluster? They can simply deploy it in another cluster.
Furthermore, from an implementation perspective, how can we ensure the deletion of EndpointSlice? This is controlled by controllers in the cluster, so even if we delete it, a new one will be created immediately.
There was a problem hiding this comment.
I agree it is a rare case. I can think of a monolith that has multiple entry points and it can read messages of Kafka broker but isn't allowed to get REST API calls. So the user will deploy the code using a Deployment but will not want it exposed as REST endpoint. Faulty user architecture, but can work with Karmada.
I don't have a good idea how to delete it, maybe we can push that feature for later.
There was a problem hiding this comment.
Thanks for your response!
There was a problem hiding this comment.
I basically agree with @XiShanYongYe-Chang that we might not touch the local EndpointSlice which is managed by the member cluster.
I understand the case for Remote Only should be that:
There are no Pods on that cluster, but the cluster needs to consume the service.
For example, a user might deploy DB services on a cluster, and allow other clusters to access the service.
|
/assign |
| With the current `ServiceImportController` when a `ServiceImport` object is reconciled, the derived service is prefixed with `derived-` prefix. | ||
| This seems counterintuitive when thinking about service discovery: | ||
| - Assuming the pod is exported as the service `foo` | ||
| - Another pod that wishes to access it on the same cluster will simply call `foo` and Kubernetes will bind to the correct one | ||
| - If that pod is scheduled to another cluster, the original service discovery will fail as there's no service by the name `foo` | ||
| - To find the original pod, the other pod is required to know it is in another cluster and use `derived-foo` to work properly |
There was a problem hiding this comment.
Putting this description under "Motivation" might be better, as it can explain what problem we are trying to solve with the current proposal.
Furthermore, in the Summary section, a brief description can be provided on what problem the current proposal aims to address and how it intends to solve it.
There was a problem hiding this comment.
By the way, personally I think it's best to use the following approach for multi-cluster services.
The image is sourced from GKE.
There was a problem hiding this comment.
youtube video:
There was a problem hiding this comment.
@chaunceyjiang I actually wish to avoid using clusterset so the users will not have to know about Service placement and call it locally as with any other Service deployed locally
|
|
||
| 1. Service Import Controller Changes: The Service Import Controller needs to be updated to remove the `"derived-"` prefix from the derived service. This ensures that the imported service retains its original name when accessed within the importing cluster. | ||
|
|
||
| 2. Local-Only Service Discovery: To support local-only service discovery, Karmada should be enhanced to check if there is a local service with the same name as the imported service. If a local service exists, the remote service import should be skipped, and no `EndPointSlice` should be created for the remote service. This allows local services to be accessed directly without going through the remote service. |
There was a problem hiding this comment.
How do we distinguish which type of service it is? Local-only, or local-and-remote?
There was a problem hiding this comment.
I think the most suitable location for that is the multi cluster services API with a Karmda annotation to control the behavior. Something like:
apiVersion: multicluster.x-k8s.io/v1alpha1
kind: ServiceImport
metadata:
name: serve
namespace: demo
annotations:
discovery.karmada.io/strategy: LocalAndRemote
spec:
type: ClusterSetIP
ports:
- port: 80
protocol: TCP
The default could be LocalAndRemote.
I don't think that the propagation policy spec can fit as it a more generic spec.
There was a problem hiding this comment.
It feels like a feasible approach. Can it be included in the proposal?
Once the proposal is refined, would you be invited to share this proposal at the community meeting?
There was a problem hiding this comment.
Yes, I would be happy to share it
There was a problem hiding this comment.
Here is the Meeting Notes and Agenda, please feel free to add the agenda with your time.
There was a problem hiding this comment.
It seems that with my summer vacation coming up, I will be able to attend it only mid-September 😕
|
We also need this feature in our products. How's this feature going now? |
RainbowMango
added
the
tide/merge-method-squash
|
Hi @bivas Thanks for sending the proposal, it's excellent! Given that you are on vocation, I think we can move this PR forward, and @XiShanYongYe-Chang could continue to iterate it based on this version. Hope you don't mind. |
Signed-off-by: Eliran Bivas <eliran.bivas@appsflyer.com>
RainbowMango
force-pushed
the
service-discovery-proposal-2384
branch
from
1c70d1f
to
a84cff0
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
karmada-bot
added
the
approved
|
Ok, I will continue to iterate it based on this version. |
What type of PR is this?
/kind design
Which issue(s) this PR fixes:
Fixes #2384