Initial port of cache functions from java client

[ddieruf]

On the road to informers, this is the first step. Cache is a core component of informers as it stores state of objects. This is a direct port from master in kubernetes-client/java.

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
• Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[k8s-ci-robot]

Welcome @ddieruf!

It looks like this is your first PR to kubernetes-client/csharp 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-client/csharp has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[tg123]

Thanks for the PR

could you please move codes to KubernetesClient.csproj, bc only 1 nupkg was published today (alternative: pack util dll into main pkg)

[ddieruf]

@tg123 understood. It's distributed as a single package. Can you provide a little guidance on your alternate approach? I keep running into cyclical ref errors.

By the way, my motivation behind this design is to follow the Java client. I am happy to adjust things but wanted to keep their patterns as much as possible.

[brendandburns]

We definitely had feedback in the Java library that some users wanted a "simple" package w/o all of the extra informer stuff, which was the motivation for multiple packages.

I suspect that there will be similar feedback in this project, so I'm supportive of having a "simple" client and an "extended" package that depends on that client. @tg123 you know more about dotnet packaging than I do, are there concerns with this approach?

[brendandburns]

A few small comments, but this looks like a great start to me, thank you for getting it going!

[ddieruf]

We definitely had feedback in the Java library that some users wanted a "simple" package w/o all of the extra informer stuff, which was the motivation for multiple packages.

I suspect that there will be similar feedback in this project, so I'm supportive of having a "simple" client and an "extended" package that depends on that client. @tg123 you know more about dotnet packaging than I do, are there concerns with this approach?

It really makes sense to keep this separate fr the core client. They are not required and just add overhead to some.

[tg123]

I suspect that there will be similar feedback in this project, so I'm supportive of having a "simple" client and an "extended" package that depends on that client. @tg123 you know more about dotnet packaging than I do, are there concerns with this approach?

+1 for multiple packages

it is better to have multiple extend packages.
for example, it is better to have some pojo projects like k8s.model.v1.19 and k8s.model.v1.18, which can target lower sdk version such as netstand1.0 and net45 which they can build something else on top of.

searched cross some related issues about multiple package:
#448 a proposal

the first time we were discussing about separate packages is inside #102 i believe
when the weboscket was introduced but no api for netstand2.1-

for compatibility reason, I prefer a multiple package migrating plan and not removing features from main package suddenly.
to do it, just add -IncludeReferencedProjects to nuget pack in nuget.yaml

[ddieruf]

Forgive me if I am missing something obvious but I am not seeing how I can include the Utils project in the KubernetesClient project pack. The build process is using dotnet pack and in the documentation for that command it says...

NuGet dependencies of the packed project are added to the .nuspec file, so they're properly resolved when the package is installed. If the packed project has references to other projects, the other projects are not included in the package. Currently, you must have a package per project if you have project-to-project dependencies.

The Utils project references the KubernetesClient project so I can't make Utils a dependency of KubernetesClient, that would be a circular ref.

The IncludeReferencedProjects flag doesn't appear to be an option of dotnet pack. I do see a no-dependencies flag.

Solution Idea:
Given that the nuget.yaml workflow

• Already runs tests for all projects in the solution
• The pack for the core client outputs to /pkg
• The push includes pkg/*.nupkg

Could I add the following step in the workflow to distribute the Util package as a separate thing? This would not disrupt the distribution of the core client in any way and would let the Util package follow its own versioning.

- name: dotnet pack
  run: dotnet pack -c Release util/src/KubernetesClient.Util -o pkg --include-symbols

[brendandburns]

I agree w/ @tg123 that we should definitely not remove anything from the main library, as it will definitely break existing users.

I had missed that we were removing things from the library? I thought we were just adding the Informer logic as an extended package.

[ddieruf]

I agree w/ @tg123 that we should definitely not remove anything from the main library, as it will definitely break existing users.

I had missed that we were removing things from the library? I thought we were just adding the Informer logic as an extended package.

This is 100% addition. I didn't change anything in the core client.

[tg123]

@ddieruf seems I am misleading

my idea about -IncludeReferencedProjects is to create a meta package, kubernetesclient, which ref to kubernetesclient.old kubernetesclient.util and it will yield a single all-in-one package same as today's

after some test and planning, we should have confident to push new name to nuget.org.
it is better to do more name planning before pushing new packages or it would confuse users.

@brendandburns nothing removed in this PR, I am thinking about the roadmap to migrate to multiple nuget package
first, users will still see 1 all-in-one package while we are cleaning and splitting projects.
then, publish matured packages to nuget.org, for exmaple, kubernetesclient.auth.oid or kubernetesclient.model
finally, deprecate kubernetesclient or make it a meta package depends on all other packages

[ddieruf]

@tg123 thank you for the clarification. I see you are thinking about a bigger picture than what I was.

I totally understand the name planning. If you all are confident that this PR is ok, I am going to continue on to the next.

Upcoming PRs:

• bring in DeltaFifo, ListerWatcher, and Reflector (this will complete cache)
• bring in informer classes examples using informer

[brendandburns]

This PR looks fine to me, CI/CD is failing though, and I want @tg123 to confirm it's ok too.

[ddieruf]

@tg123 and @brendandburns I am seeking a little advice on how to proceed...

As I get into Cache.Controller and Cache.SharedProcessor they utilize Java's ThreadPoolExecutor and ScheduledExecutorServices. The internals of these two classes are definitely achievable in C# but are going to make the PR considerably larger.

If you are not familiar with the executor services they introduce the ability to control background threads (pool sizes, queues, workers, etc). Also they give you options like scheduling recurring processes.

C# of course has Task and TaskFactory abilities. As an alternate solution, I could couple those with things like Semaphore and Threading.Timer to create a smaller less versatile version of executors (notably, I probably wouldn't build the notion of workers as ThreadPoolExecutor has).

A third option would be to convert the things that Controller and SharedProcessor are meant to manage into tasks but this will offer even less fine-grain thread control. Also, this will deviate the code base considerably from Java. So keeping things in-sync will be more of a challenge.

I haven't spent much time getting deep into Rx but if I was a betting man (which I am), I would say this is an inflection point. What are your thoughts about all this? Have I maybe missed another option?

[brendandburns]

@ddieruf I feel like we should get this PR in with it's current scope and discuss the next steps in either an issue or another PR?

I think incremental progress will help us get to the right place. I don't think we have to perfectly follow the Java implementation if it doesn't make sense.

[tg123]

In short:
Task.Run(() => {}) and Task.Run(async () => { for (;;) { dosomething() ; await Task.Delay(xxx timespan); } } )

Per my experience, I am quite into C#'s async/await or Task design.

you do not have to worry about anything like threadpool, .net runtime will take care of it, and the only thing is to
https://docs.microsoft.com/en-us/dotnet/api/system.threading.threadpool.setmaxthreads?view=net-5.0 and it is definitely optional and can be set by end users.

Task in C# is something, not sure if you are familiar with, much like Go's goroutine.
It is smaller than thread and being scheduled by the runtime. await will be compiled into state machine to have better coding experience.

you may want to take a look at Wather's https://github.com/kubernetes-client/csharp/blob/master/src/KubernetesClient/Watcher.cs#L100 loop

[ddieruf]

@brendandburns we can use my open original issue to chat.

@tg123 thank you for the thoughts. I would never claim to be an expert with C# tasks but have been using them for some time. Your direction makes sense. Since we aren't concerned about drifting away from the patterns Java is using, I'll get something going.

[ddieruf]

What can I do to help get this PR moved along? Anything I am missing?

[brendandburns]

I'm ready to merge this, assuming that @tg123 is ok with it and we have any packaging concerns straightened out.

[brendandburns]

@tg123 I'm going to merge this in 2 days if there are no further comments.

Thanks!

[tg123]

could you please move util/src/KubernetesClient .Util to src/KubernetesClient .Util ?

@brendanburns in current PR, no new nuget will be published, which means no KubernetesClient .Util on nuget.org
do you feel ok to add it to nuget.org?

I suggest to move the new code directly to KubernetesClient before we know how to do the multiple nugets (keep single nuget for a while)

[tg123]

should be ignore by git

[ddieruf]

@tg123 as I mentioned before whatever works for publishing. If it's one package or multiple.

I have just about completed everything and I can safely say there is a lot of code coming. I have not changed a single file in the client, so it's a very clean separation. Cache, informers, and all the supporting classes are quite a bit to add to the original project.

Again, I will go with whatever you feel is best.

[tg123]

@ddieruf can you just drop the folders into KubernetesClient project?

[ddieruf]

Will do

[ddieruf]

@tg123 it is done. Moved everything into the client project :)

[tg123]

/LGTM

[brendandburns]

/lgtm
/approve

One test flaked, I re-ran it, hopefully it passes this time.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, ddieruf

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [brendandburns]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment