Kubernetes provider should identify different ports

[shcant]

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.

[raman-m]

I don't understand you!

Describe your user scenario in details please! Attach all artifacts!

[raman-m]

Is this issue a bug, a feature or an idea?

[raman-m]

@RaynaldM Any ideas? What is this author talking about? 😄

[shcant]

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:

Name:         example-web
Namespace:    default
Subsets:
  Addresses:          10.1.161.59
  Ports:
    Name   Port  Protocol
    ----   ----  --------
    https  443  TCP
    http   80  TCP

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'.

[raman-m]

Thank you for explanation!

So, the problematic method: https://github.com/ThreeMammals/Ocelot/blob/d54836d51f6ea29a2013967e3649a23e19db28b3/src/Ocelot.Provider.Kubernetes/Kube.cs#L42-L54

and especially this line of the code: https://github.com/ThreeMammals/Ocelot/blob/d54836d51f6ea29a2013967e3649a23e19db28b3/src/Ocelot.Provider.Kubernetes/Kube.cs#L49 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 Kube provider? Do you have draft version of a new solution?

My understanding, we have to decouple Service object creation logic to a new interface and inject it, or introduce a new interface with a new BuildServices method, like this 👉

Design 1

using 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 BuildServices method

Design 2

using 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 Which design do you like?

I have additional 3rd idea for Design 3... 😉 having an options of feature.

[raman-m]

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 Kube class and redevelop, or inherit Kube class and override methods, and inject required objects by instructions in our docs

[raman-m]

But you define multiple downstream service ports of one route. In theory one endpoint can have multiple bounded protocols (ports) but it is overhead deployments. Seems it should be one route with multiple downstream pairs of host & port of this property of the DownstreamRoute class:

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... My understanding hosts should be different. But Yes, in theory one service instances have different ports of the same host... Seems there is no issue...

[shcant]

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.

[raman-m]

Hi @shcant! What's your full name and LinkedIn? Did/Do not you have an intention to contribute?

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. My proposals are here in my past comment with draft design!

[shcant]

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

[raman-m]

@shcant Lazy bones! 😉 🆗 I will open a PR when started work for March'24 release by my own. The issue is assigned to me. So, I'll care about... But I will ask you to review hoping that you'll have enough time for that, cause you are the author of the issue.

[raman-m]

@shcant commented on Mar 5:

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.

It appears you've shifted your mood! 😄 Indeed, it varies depending on the deployment scenario. You can define various Consul services with distinct service names. As noted, it's also possible to assign the same host to a single service name but differentiate them using various ports. I'll consider all these scenarios when designing the "decoupled DI services" version of the provider.

Working on this... 👨‍💻

[raman-m]

@shcant Hi! Does PR #2052 solve your problem with port names? I believe Yes...

[raman-m]

Hey @shcant! The feature has been merged ❗ The likely delivery date is the beginning of May, coinciding with the publication of the new version of the NuGet package. Would you like any explanations on how to use the feature?