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

migrate controllers and clients to leverage controller-runtime #485

Closed
17 tasks done
zwpaper opened this issue Jan 26, 2023 · 13 comments
Closed
17 tasks done

migrate controllers and clients to leverage controller-runtime #485

zwpaper opened this issue Jan 26, 2023 · 13 comments
Assignees

Comments

@zwpaper
Copy link
Member

zwpaper commented Jan 26, 2023

Scheduler Plugins is working on migrating controllers and clients to leverage controller-runtime,

I have walked through the code base quickly and met some questions, so I am opening this issue to
make sure I am doing things right and figure out how can we work on this.

there should be 2 parts we should work on:

  1. controllers: https://github.com/kubernetes-sigs/scheduler-plugins/tree/056fa7df192d4b49101650f37873338e3d8b96fb/pkg/controller, which is WIP with https://github.com/kubernetes-sigs/scheduler-plugins/tree/056fa7df192d4b49101650f37873338e3d8b96fb/pkg/controllers and not yet being used
  2. generated clients: used everywhere in controllers and schedulers

my questions are:

  1. is the 2 parts above right?
  2. should we use a new command args to choose the legacy or controller-runtime controllers
  3. should all the generated clients used in controllers and schedulers be migrated to controller-runtime clients?

break down

Controllers

clients

this is generated by the following cmd:

rg -c -l -g '!vendor' -g '!pkg/generated' -g '!*_test.go' 'pkg/generated/' . | sort
@zwpaper
Copy link
Member Author

zwpaper commented Jan 26, 2023

cc @Huang-Wei

@Huang-Wei
Copy link
Contributor

  1. is the 2 parts above right?

pkg/controller adopts typed clientset, which is currently used; pkg/controllers is WIP, and hence not being used in real logic for now. Our goal is to migrate to the general client (controller-runtime's), in other words, get rid of current typed clientsets (both usage and codegen).

  1. should we use a new command args to choose the legacy or controller-runtime controllers

I'd prefer to adopt the latest conversion/best practice of adopting controller-runtime. No need to stick to the legacy way.

  1. should all the generated clients used in controllers and schedulers be migrated to controller-runtime clients?

Yes. That's the point of migration - getting rid of typed clients.

@zwpaper
Copy link
Member Author

zwpaper commented Jan 31, 2023

I will start to work on migrating Pod Group controller

/assign

@nayihz
Copy link
Contributor

nayihz commented Feb 22, 2023

assign for migrating ElasticQuota controller to ctrl runtime
/assign

@Huang-Wei
Copy link
Contributor

@czybjtu you may want to contact @zwpaper first. (just to ensure you are not working on the same item)

@nayihz
Copy link
Contributor

nayihz commented Feb 24, 2023

sorry for my oversight. Please let me know if you have started working on it. @zwpaper

@zwpaper
Copy link
Member Author

zwpaper commented Feb 24, 2023

Oops, I should leave a comment rather than just updating the issue description.

it is totally great for anyone to participate, and accelerate the migration.

I have done the podGroup controller migration, you can refer to it and it may save some of your time. @czybjtu

Now I am starting to work on the migration of scheduler clients, feel free to ping me if anyone would like to participate.

cc @Huang-Wei

@ffromani
Copy link
Contributor

ffromani commented Mar 6, 2023

/cc
I'd be happy to check if noderesourcetopology (and later other plugins hopefully) need some work

@k8s-triage-robot
Copy link

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

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

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 7, 2023
@Huang-Wei
Copy link
Contributor

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 7, 2023
@Huang-Wei
Copy link
Contributor

@zwpaper I'm working on coscheduling's migration to controller-runtime.

@Huang-Wei
Copy link
Contributor

Last piece of migrate elasticquota/capacityscheduling #665

@Huang-Wei
Copy link
Contributor

All work completed. Thanks a ton for @zwpaper to lead the migration work.

I added some notes in the release note: https://github.com/kubernetes-sigs/scheduler-plugins/releases/tag/v0.27.8. In particular:


One major change completed in this release is #485 which migrates from typed client to generic controller-runtime client.

  • Impact to end-users: N/A
  • Impact to developers:
    • for CR object's manipulation, new plugin's development should stick to current codebase convention to use controller-runtime's client or informer
    • for core API object's manipulation, esp. Pods and Nodes, it's still recommended to read/watch from frameworkHandle's sharedInformerFactory; for other core API objects, it's up to plugin developers to choose which client to use
    • ⚠️ generated typed client/informer under pkg/generated will be removed after 3 releases (v0.30.x). External repos that vendor this repo's typed generation code should either generate typed client/informer themselves or start migrating to generic controller-runtime client.

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

6 participants