Skip to content
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

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

Open
guillaume-chervet opened this issue Feb 5, 2024 · 13 comments

Comments

@guillaume-chervet
Copy link

guillaume-chervet commented Feb 5, 2024

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

@guillaume-chervet guillaume-chervet changed the title [KubernetesClient.Aot] .NET 8 AOT compatibility for Patch function [KubernetesClient.Aot] .NET 8 AOT compatibility for Scale function Feb 5, 2024
@guillaume-chervet guillaume-chervet changed the title [KubernetesClient.Aot] .NET 8 AOT compatibility for Scale function [KubernetesClient.Aot] .NET 8 AOT compatibility for Scale functions Feb 5, 2024
@tg123
Copy link
Member

tg123 commented Feb 5, 2024

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
Copy link
Author

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

@guillaume-chervet
Copy link
Author

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
Copy link
Member

tg123 commented Feb 11, 2024

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

JsonSerializer.Serialize(writer, content, options);

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
Copy link
Author

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
Copy link
Member

tg123 commented Feb 15, 2024

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
Copy link
Author

Thank you @tg123 ,

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

@guillaume-chervet
Copy link
Author

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

How can I help you?

@tg123
Copy link
Member

tg123 commented Feb 21, 2024

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
Copy link
Author

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

@guillaume-chervet
Copy link
Author

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
Copy link
Member

tg123 commented Feb 29, 2024

see here #1216

@guillaume-chervet
Copy link
Author

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;
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants