kubernetes-client / csharp

Officially supported dotnet Kubernetes Client library
Apache License 2.0
1.06k stars 290 forks source link

[KubernetesClient.Aot] .NET 8 AOT compatibility for Scale functions #1518

Open guillaume-chervet opened 5 months ago

guillaume-chervet commented 5 months ago

I @tg123 ,

I'am using KubernetesClient.Aot --version 13.0.12 Thank you so much for your help !

Just an issue to trace it. It would be awesome to implement AOT compatibility with Scale (StatefulSet and Deployment) functions :)

2024-02-02 09:22:36 fail: SlimFaas.ScaleReplicasWorker[0]
2024-02-02 09:22:36       Global Error in ScaleReplicasWorker
2024-02-02 09:22:36       System.ArgumentNullException: Value cannot be null. (Parameter 'jsonTypeInfo')
2024-02-02 09:22:36          at System.Text.Json.ThrowHelper.ThrowArgumentNullException(String)
2024-02-02 09:22:36          at k8s.Kubernetes.SendRequest[T](String, HttpMethod, IReadOnlyDictionary`2, T, CancellationToken)
2024-02-02 09:22:36          at k8s.AbstractKubernetes.IAppsV1Operations_PatchNamespacedDeploymentScaleWithHttpMessagesAsync[T](V1Patch, String, String, String, String, String, Nullable`1, Nullable`1, IReadOnlyDictionary`2, CancellationToken)
2024-02-02 09:22:36          at k8s.AbstractKubernetes.k8s.IAppsV1Operations.PatchNamespacedDeploymentScaleWithHttpMessagesAsync(V1Patch, String, String, String, String, String, Nullable`1, Nullable`1, IReadOnlyDictionary`2, CancellationToken)
2024-02-02 09:22:36          at k8s.AppsV1OperationsExtensions.PatchNamespacedDeploymentScaleAsync(IAppsV1Operations, V1Patch, String, String, String , String , String , Nullable`1 , Nullable`1 , CancellationToken )
2024-02-02 09:22:36          at SlimFaas.Kubernetes.KubernetesService.ScaleAsync(ReplicaRequest request)
2024-02-02 09:22:36          at SlimFaas.ReplicasService.CheckScaleAsync(String kubeNamespace)
2024-02-02 09:22:36          at SlimFaas.ScaleReplicasWorker.ExecuteAsync(CancellationToken stoppingToken)

The code : image

tg123 commented 5 months ago

the patch banned for aot is because that patch is so dynamic

i looked into it but did not have better idea any suggestion?

guillaume-chervet commented 5 months ago

Hi @tg123 is patch not just a string? Does it require json formatting?

guillaume-chervet commented 5 months ago

hi @tg123 , I look at the code but I did not find any PatchNamespaceFunction. I do not very understand how is organized the whole code.

Do you have a youtube or some notes to help to understand this repository?

tg123 commented 4 months ago

here is the reason why V1Patch is not easy to fit AOT

https://github.com/kubernetes-client/csharp/blob/cdf5398e2d017f4687a15ba5f644aac47b469fc7/src/KubernetesClient/Models/V1PatchJsonConverter.cs#L24 V1Patch.body is a type-less object which AOT does not like

a workaround is to make V1Patch body string only but it breaks the compatibility with the non-aot sdk

guillaume-chervet commented 4 months ago

I am for adding a specific method for AOT. Every project i have converted to .NET 8, i had to remove dynamic behavior in favor of performance and final ram cost benefice. Do you think it is possible to do it? @tg123

tg123 commented 4 months ago

imho, dynamic will still play an important role in future .net projects. keep patient, a bit busy there days, will get it done as soon as i got time

guillaume-chervet commented 4 months ago

Thank you @tg123 ,

I do not know how to help you. It would be awesome to have a solution :)

guillaume-chervet commented 4 months ago

hi @tg123 , when do you think you will have some time ?

How can I help you?

tg123 commented 4 months ago

thanks @guillaume-chervet

if you can port https://github.com/kubernetes-client/csharp/blob/master/src/KubernetesClient/Models/V1Patch.cs and https://github.com/kubernetes-client/csharp/blob/master/src/KubernetesClient/Models/V1PatchJsonConverter.cs to AOT project, then patch will be good

i am thinking to support string only, but the behavior changed from non-aot project

guillaume-chervet commented 4 months ago

Thank you @tg123 , i will try to look at. I am lacking of time too :(

guillaume-chervet commented 4 months ago

hi @tg123 , I do knot understand well the project.

Another idea, is it possible to retrieve an HttpClient already configured to pass authentication header ? So it will be possible to build AOT compatible rest request.

tg123 commented 4 months ago

see here https://github.com/kubernetes-client/csharp/discussions/1216

guillaume-chervet commented 4 months ago

Thank you very much @tg123 . It works, I can activate Trimming now !

    public async Task<ReplicaRequest?> ScaleAsync(ReplicaRequest request)
    {
        try
        {
            using k8s.Kubernetes client = new(_k8SConfig);
            string patchString = $"{{\"spec\": {{\"replicas\": {request.Replicas}}}}}";
            var httpContent = new StringContent(patchString, Encoding.UTF8, "application/merge-patch+json");
            // we need to get the base uri, as it's not set on the HttpClient
            switch (request.PodType)
            {
                case PodType.Deployment:
                    {
                        var url = string.Concat(client.BaseUri, $"apis/apps/v1/namespaces/{request.Namespace}/deployments/{request.Deployment}/scale" );
                        HttpRequestMessage httpRequest = new(HttpMethod.Patch,
                            new Uri(url));
                        httpRequest.Content = httpContent;
                        if ( client.Credentials != null )
                        {
                            await client.Credentials.ProcessHttpRequestAsync( httpRequest, CancellationToken.None );
                        }
                        var response = await client.HttpClient.SendAsync(httpRequest, HttpCompletionOption.ResponseHeadersRead);
                        if(response.StatusCode != HttpStatusCode.OK)
                        {
                            throw new HttpOperationException("Error while scaling deployment");
                        }
                        break;
                    }
                case PodType.StatefulSet:
                    {
                        var url = string.Concat(client.BaseUri, $"apis/apps/v1/namespaces/{request.Namespace}/statefulsets/{request.Deployment}/scale" );
                        HttpRequestMessage httpRequest = new(HttpMethod.Patch,
                            new Uri(url));
                        httpRequest.Content = httpContent;
                        if ( client.Credentials != null )
                        {
                            await client.Credentials.ProcessHttpRequestAsync( httpRequest, CancellationToken.None );
                        }
                        var response = await client.HttpClient.SendAsync(httpRequest, HttpCompletionOption.ResponseHeadersRead );
                        if(response.StatusCode != HttpStatusCode.OK)
                        {
                            throw new HttpOperationException("Error while scaling deployment");
                        }
                        break;
                    }
                default:
                    throw new ArgumentOutOfRangeException();
            }
        }
        catch (HttpOperationException e)
        {
            _logger.LogError(e, "Error while scaling kubernetes deployment {RequestDeployment}", request.Deployment);
            return request;
        }
        return request;
    }
k8s-triage-robot commented 1 month ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 1 week ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten