tintoy / dotnet-kube-client

A Kubernetes API client for .NET Standard / .NET Core
MIT License
192 stars 33 forks source link

Respect groupVersion when dynamically loading resource-type metadata from cluster API #139

Open tintoy opened 3 years ago

tintoy commented 3 years ago

Currently, resource type metadata for the dynamic resource client ignores groupVersion (e.g. networking.istio.io/v1beta1) in favour of version (e.g. v1beta1) when loading resource type metadata from the k8s API (loading from type annotations on model metadata works correctly however).

The end result of this is that resource types that have a groupVersion rather than just a version (i.e. version != groupVersion) don't seem to work correctly with the dynamic client.

I think the original idea was to enable consumers to specify kind and apiVersion without having to specify group as well (i.e. kind and apiGroupVersion) but this was probably due to a lack of understanding (on my part, I suspect) of the semantics of API group and version.

I have some mild concerns that this may constitute a breaking change but considering that the existing behaviour was both wrong and non-functional I think I am OK with that 🙂

PS. @felixfbecker - figured you might care since the powershell stuff was one of the original consumers of this functionality 🙂

Relates to tintoy/dotnet-kube-client#114 and tintoy/dotnet-kube-client#138

linjmeyer commented 3 years ago

Hey @tintoy I'm unfortunately still getting the same error when using this branch (#138)

tintoy commented 3 years ago

Ok, I’ll have a look this morning and see if I can figure out what’s going on.

tintoy commented 3 years ago

Just to confirm - you’re calling ApiMetadata.Load() to get metadata from the cluster API before trying to call List()?

linjmeyer commented 3 years ago

Ah I wasn't, but with that added I am getting a different error for both the custom resource and HorizontalPodAutoscalers which was previously working:

HPA:

Cannot find resource API for kind 'HorizontalPodAutoscaler  apiVersion 'autoscaling/v1

Istio CRD:

Cannot determine the list model type that corresponds to networking.istio.io/v1beta1/DestinationRule.

I've tried tweaking a few things, like not passing in the API group in the version (ie just v1beta1) to the annotations and List call, same result in all cases. Interesting this does work tho:

var cached = _kubeClient.ApiMetadata.Get(ResourceKind, ResourceApi);
// ApiMetadata has 400+ resources when using breakpoints, so the data seems to be there

Any ideas? Thanks again for the time, no real rush on this.

Istio classes (tried using networking.istio.io/v1beta1 and just v1beta1)

[KubeListItem("DestinationRule", "networking.istio.io/v1beta1")]
[KubeObject("DestinationRuleList", "networking.istio.io/v1beta1")]
public class DestinationRuleListV1 : KubeResourceListV1<DestinationRuleV1>
{
    public override List<DestinationRuleV1> Items { get; }
}

---

[KubeObject("DestinationRule", "networking.istio.io/v1beta1")]
[KubeApi(KubeAction.List, "apis/networking.istio.io/v1beta1/destinationrules")]
[KubeApi(KubeAction.List, "apis/networking.istio.io/v1beta1/namespaces/{namespace}/destinationrules")]
[KubeApi(KubeAction.Get, "apis/networking.istio.io/v1beta1/destinationrules/status")]
[KubeApi(KubeAction.Get, "apis/networking.istio.io/v1beta1/namespaces/{namespace}/destinationrules/status")]
public class DestinationRuleV1 : KubeResourceV1
{

}
tintoy commented 3 years ago

The error Cannot determine the list model type that corresponds to networking.istio.io/v1beta1/DestinationRule is saying that the client has received a response from the K8s API but doesn't know how to deserialise it (because the client doesn't know what CLR type to use as the deserialisation target). So declaring your type (as you've done above) is the right approach, you just need to tell the client where your model type lives:

clientOptions.ModelTypeAssemblies.Add(
        typeof(DestinationRuleV1).Assembly
);
tintoy commented 3 years ago

To be honest, I'm not entirely happy with how the dynamic resource client works; it was originally built to support the Powershell provider (where specifying type info is more painful) and has plenty of room for improvement in terms of ergonomics :)

Philosophically speaking, KubeClient is mainly designed for use with strongly-typed models and clients and I suspect what has led you to the dynamic client is that there is no client defined for the Istio resource types (feel free to correct me if I'm wrong on this).

The way the library was originally envisioned, you'd extend the client by creating some models and an additional KubeResourceClient implementation (see https://github.com/tintoy/dotnet-kube-client#extensibility for an example of this). And possibly even package that up as a library called KubeClient.Extensions.Istio (if you're feeling particularly exuberant 😉).

But perhaps the dynamic client provides a halfway point in terms of investment of effort so it might be worth refining its ergonomics for this sort of use case.

I'll keep thinking about that 🙂

linjmeyer commented 3 years ago

ahh got it, yeah this was a while ago I think there were no statically typed options for HPA v2 so I ended up using the dynamic client. I forget exactly as it was a while ago, but this is a new but similar use case for Istio resources so I started with the same approach. I took a look at the docs you linked and switched to the statically typed approach, may as well since I need to declare classes for the CRD resources anyway. It's working well now, so thank you for the help and suggestions!

tintoy commented 3 years ago

Great, glad to hear it! 🙂

I’ll see what I can do to improve that error message, BTW - it doesn’t really give a good indication of how to resolve the problem.