buehler/dotnet-operator-sdk

Latest release (6.4.0) does not work and will fail with default template

Closed this issue ยท 11 comments

Describe the bug

There are several issues running the latest release on an existing or new templated project

The first is the scoped services that no longer work with Hosted services.
ie. Leader Election and LocalTunnerl.

Without explicitly turning off leader election, the code will not run due to the following:
Cannot consume scoped service 'DotnetKubernetesClient.IKubernetesClient' from singleton 'Microsoft.Extensions.Hosting.IHostedService'.


another issue is that IResourceCache is not required for EventQueue which is not part of the controller, however, there is no service registration for Resourcecache and the
services.AddScoped<typeof(ResourceCache<>>) doesn't do it.
The error is

'Unable to resolve service for type 'KubeOps.Operator.Caching.IResourceCache1[Operator3.Entities.V1DemoEntity]' while attempting to activate 'KubeOps.Operator.Controller.EventQueue1[Operator3.Entities.V1DemoEntity]'.'

To Reproduce
create a new project, reference .net6 language version 10 and the package version 6.4.0. Hit play

Is this targeting the templates or the logic itself? Because there was a bug introduced in a refactoring and now fixed with #437

I am fairly sure this is the logic itself; I had changed a couple service lifetimes to make things play nice for testing and clearly there were some issues that the tests didn't catch.

One part of this was identical to #437, but the hosted service issues are separate.

Ah, I'm surprised I didn't run into this, but I'm binding IKubernetesClient as a singleton in our project (also using AutoFac).

I'm assuming the workaround right now is to upgrade to at least v6.4.1 and change the IKubernetesClient registration to a singleton.

Personally, I think IKubernetesClient should always be a singleton, since the referenced Kubernetes class does PEM parsing in the ctor (IIRC), which can just tank performance.

I'm assuming the workaround right now is to upgrade to at least v6.4.1 and change the IKubernetesClient registration to a singleton.

Yeah, I'm going to see if I can change that back and just adjust registrations in the test startup.

Personally, I think IKubernetesClient should always be a singleton, since the referenced Kubernetes class does PEM parsing in the ctor (IIRC), which can just tank performance.

I am a bit concerned about this for compatibility's sake, because it doesn't look like the client has any logic for reloading the token, so if your service account token expires (i.e. if you aren't using the default deployment spec/RBAC rules) that could cause issues. I don't have tests/evidence to support that right now, so I'm just going to note it here in case we come across this down the line.

On second thought... I have no clue why I did what I did for that... I changed IKubernetesClient to scoped in the extensions and then overrode it back to singleton in the test stuff ๐Ÿ˜•

Well, I've got a fix for it now

reloading the token

Whoa, I always thought the tokens never expired e.g. https://stackoverflow.com/a/62830897/2001966

But at least some flavors (if not vanilla) do rotate tokens: zalando/skipper#1277

Actually @erin-allison, we look safe for this to be a singleton.

https://github.com/kubernetes-client/csharp/blob/7936814751dd0b6808b819c27b1c8c16691fe073/src/KubernetesClient/Authentication/TokenFileAuth.cs#L26

The change was added in v7.0.5, which this project requires >= 7.0.15.

Ah, perfect!

I had just seen this referencing how bound token volumes can/do have an expiration and didn't see how the C# library handled that.

๐ŸŽ‰ This issue has been resolved in version 6.5.1 ๐ŸŽ‰

The release is available on:

Your semantic-release bot ๐Ÿ“ฆ๐Ÿš€

This looks to still be broken:

'Cannot resolve scoped service 'KubeOps.Operator.Controller.IEventQueue1[******]' from root provider.'`

Please reopen

actually... tracked in #450