-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Kubernetes provider should identify different ports #1967
Comments
I don't understand you! Describe your user scenario in details please! |
Is this issue a bug, a feature or an idea? |
@RaynaldM Any ideas? |
I'm sorry for the late reply. For example, my web service on k8s expose two ports: https for 443 and http for 80, like this:
I use ocelot as the entry of my service. If I access some API like 'http://example-web/api/example/1', ocelot will redirect the request to the address like 'http://10.1.161.59:443' . The hostname can be parsed correctly, but port is wrong. I try to specify DownstreamScheme=http in ocelot but k8s provider still choose the https port: {
"DownstreamPathTemplate": "/{url}",
"DownstreamScheme": "http",
"UpstreamPathTemplate": "/api/example/{url}",
"ServiceName": "example-web",
"UpstreamHttpMethod": [ "Get"]
} Then I check the source code, find k8s service discovery only select the first one : services.AddRange(subset.Addresses.Select(address => new Service(endpoint.Metadata.Name,
new ServiceHostAndPort(address.Ip, subset.Ports.First().Port),
endpoint.Metadata.Uid, string.Empty, Enumerable.Empty<string>()))); It will be fine if we can use the port with specific name, like 'https' or 'http'. |
Thank you for explanation! So, the problematic method: Ocelot/src/Ocelot.Provider.Kubernetes/Kube.cs Lines 42 to 54 in d54836d
and especially this line of the code:
where we read subset.Ports.First().Port from the 1st element of Ports collection.So, the issue is clear now in general: the code works correctly if collection has single port only (count = 1), if count > 1 then the code can behave incorrectly. But from the view point of the design it is unclear how to build Service object having multiple ports?Should we create a separate Service objects? And will these Service object be mapped to DownstreamHostAndPort objects?A lot of questions... Will you contribute to redesign My understanding, we have to decouple Design 1using KubeClient.Models;
public interface IKubeServiceCreator
{
IEnumerable<Service> Create(EndpointsV1 endpoint, EndpointSubsetV1 subset);
}
// class Kube
private readonly IKubeServiceCreator _serviceCreator;
public Kube(
KubeRegistryConfiguration kubeRegistryConfiguration, IOcelotLoggerFactory factory, IKubeApiClient kubeApi, // old injections
IKubeServiceCreator serviceCreator) // new injected service object
{
// ...
_serviceCreator = serviceCreator;
}
// We have to rewrite Build method
private List<Service> BuildServices(EndpointsV1 endpoint)
{
var services = new List<Service>();
foreach (var subset in endpoint.Subsets)
{
services.AddRange(_serviceCreator.Create(endpoint, subset));
}
return services;
} But... My personal preference is wrapping logic of entire Design 2using KubeClient.Models;
public interface IKubeServiceBuilder
{
IEnumerable<Service> BuildServices(EndpointsV1 endpoint);
}
// class Kube
private readonly IKubeServiceBuilder _serviceBuilder;
public Kube(
KubeRegistryConfiguration kubeRegistryConfiguration, IOcelotLoggerFactory factory, IKubeApiClient kubeApi, // old injections
IKubeServiceBuilder serviceBuilder) // new injected service object
{
// ...
_serviceBuilder = serviceBuilder;
}
// We have to remove static Build method and rewrite GetAsync method
public async Task<List<Service>> GetAsync()
{
// ...
if (endpoint != null && endpoint.Subsets.Any())
{
services.AddRange(_serviceBuilder.BuildServices(endpoint));
}
// ...
return services;
} @shcant I have additional 3rd idea for Design 3... 😉 having an options of feature. |
Oh, I forgot about absolutely independent design and development by your own of a Custom Provider! 😄 You can develop new type of SD provider even without us. You need to copy-paste |
But you define multiple downstream service ports of one route. public List<DownstreamHostAndPort> DownstreamAddresses { get; } if the host will be the same part ports are different then it is a bit strange for load balancing scenario... |
You are right. It will be better if ports have different hosts, but in some special cases we can use custom provider to override it. I would like once again to thank you for your patience. |
Hi @shcant! What's your full name and LinkedIn? I've decided to re-open the issue because I want to re-design K8s provider and introduce more flexibility in the provider. So, we will decouple the current default implementation into 1 or 2 additional interfaces with adding to DI. |
Hi raman, thank you for your kindness again. I may not contribute due to my personal schedule, but still pay attention to the feature and provide feedback. I am very honored to your invitation. @raman-m |
It appears you've shifted your mood! 😄 Working on this... 👨💻 |
@shcant Hi! |
…vider (#2052) * Initial refactoring * Interfaces namespace * `IKubeServiceBuilder` interface vs `KubeServiceBuilder` class * `IKubeServiceCreator` interface vs `KubeServiceCreator` class * Customize K8s services creation * Add logger * namespace Ocelot.AcceptanceTests.ServiceDiscovery * Add `KubernetesServiceDiscoveryTests` * Unit tests * AAA pattern * Acceptance tests * Update kubernetes.rst * Check docs
Hey @shcant! The feature has been merged ❗ |
Expected Behavior / New Feature
k8s endpoint may have multiple ports. For example, https for 443 and http for 80. It will be better if ocelot can specify a port.
Actual Behavior / Motivation for New Feature
Ocelot k8s provider doesn't support to choose specific port, it use first port as service default port.
The text was updated successfully, but these errors were encountered: