Utilize and expose functions for starting up the manager

[tenzen-y]

What would you like to be cleaned:
I would like to utilize and expose the below functions for starting up the manager:
We can probably cut an initialize package under the pkg directory, and then we can put the below functions there.

kueue/cmd/kueue/main.go

Line 194 in 60177bf

func setupIndexes(ctx context.Context, mgr ctrl.Manager, cfg *configapi.Configuration) error {

kueue/cmd/kueue/main.go

Line 221 in 60177bf

func setupControllers(mgr ctrl.Manager, cCache *cache.Cache, queues *queue.Manager, certsReady chan struct{}, cfg *configapi.Configuration, serverVersionFetcher *kubeversion.ServerVersionFetcher) {

kueue/cmd/kueue/main.go

Line 317 in 60177bf

func setupProbeEndpoints(mgr ctrl.Manager) {

kueue/cmd/kueue/main.go

Line 343 in 60177bf

func setupServerVersionFetcher(mgr ctrl.Manager, kubeConfig *rest.Config) *kubeversion.ServerVersionFetcher {

kueue/cmd/kueue/main.go

Line 365 in 60177bf

func blockForPodsReady(cfg *configapi.Configuration) bool {

kueue/cmd/kueue/main.go

Line 369 in 60177bf

func waitForPodsReady(cfg *configapi.Configuration) bool {

kueue/cmd/kueue/main.go

Line 373 in 60177bf

func apply(configFile string) (ctrl.Options, configapi.Configuration, error) {

kueue/cmd/kueue/main.go

Line 403 in 60177bf

func isFrameworkEnabled(cfg *configapi.Configuration, name string) bool {

Why is this needed:

When the platform developers implement the managers for the in-house custom jobs, they can avoid copying and pasting those functions to their manager.

[tenzen-y]

/assign

[alculquicondor]

Some of them don't seem to be necessary for a custom integration, unless you are essentially forking kueue?

My general recommendation would be run the custom integration in a separate binary that can be released independently, similar to https://github.com/kubernetes-sigs/kueue/blob/main/cmd/experimental/podtaintstolerations/main.go

[tenzen-y]

My general recommendation would be run the custom integration in a separate binary that can be released independently, similar to https://github.com/kubernetes-sigs/kueue/blob/main/cmd/experimental/podtaintstolerations/main.go

Yes, I meant a separate binary. Currently, separate main function can not import functions like setupProbeEndpoints since those functions aren't exported.

It means that the platform developer implements the main function like the following:
I would like to avoid copying these similar functions. @alculquicondor Does this make sense?

func main() {
...
	ctx := ctrl.SetupSignalHandler()
	setupIndexes(ctx, mgr)
	setupProbeEndpoints(mgr)
	go setupControllers(mgr, certsReady, &cfg)

	setupLog.Info("starting manager")
	if err := mgr.Start(ctx); err != nil {
		setupLog.Error(err, "could not run manager")
		os.Exit(1)
	}

func setupIndexes(ctx context.Context, mgr ctrl.Manager) {
	err := jobframework.ForEachIntegration(func(name string, cb jobframework.IntegrationCallbacks) error {
		if err := cb.SetupIndexes(ctx, mgr.GetFieldIndexer()); err != nil {
			return fmt.Errorf("integration %s: %w", name, err)
		}
		return nil
	})
	if err != nil {
		setupLog.Error(err, "unable to setup jobs indexes")
	}
}

func setupProbeEndpoints(mgr ctrl.Manager) {
	defer setupLog.Info("probe endpoints are configured on healthz and readyz")

	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
		setupLog.Error(err, "unable to set up health check")
		os.Exit(1)
	}
	if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
		setupLog.Error(err, "unable to set up ready check")
		os.Exit(1)
	}
}

func setupControllers(mgr ctrl.Manager, certsReady chan struct{}, cfg *configapi.Configuration) {
...
	opts := []jobframework.Option{
		jobframework.WithManageJobsWithoutQueueName(cfg.ManageJobsWithoutQueueName),
		jobframework.WithWaitForPodsReady(waitForPodsReady(cfg)),
	}
	err := jobframework.ForEachIntegration(func(name string, cb jobframework.IntegrationCallbacks) error {
		log := setupLog.WithValues("jobFrameworkName", name)
		if err := cb.NewReconciler(
			mgr.GetClient(),
			mgr.GetEventRecorderFor(fmt.Sprintf("%s-%s-controller", name, constants.ManagerName)),
			opts...,
...
	if err != nil {
		os.Exit(1)
	}
	// +kubebuilder:scaffold:builder
}

func waitForPodsReady(cfg *configapi.Configuration) bool {
	return cfg.WaitForPodsReady != nil && cfg.WaitForPodsReady.Enable
}

[alculquicondor]

oh I see. So you have a single binary to add multiple integrations. Then it might make sense to export things like SetupIndexes and SetupControllers.

But I don't think you should be using Kueue's Configuration API. You should probably have your own.

[tenzen-y]

oh I see. So you have a single binary to add multiple integrations. Then it might make sense to export things like SetupIndexes and SetupControllers.

Yes, I will expose the only needed functions.

But I don't think you should be using Kueue's Configuration API. You should probably have your own.

In my env, I deployed another ConfigMap embedded Kueue Config to cluster and then the another Kueue Config is mounted on the inhouse kueue manager.

It means that I have 2 configMaps embedded kueue config.

[alculquicondor]

I think we shouldn't make the same assumption in the exported functions. The functions could take some specific fields.

[tenzen-y]

I think we shouldn't make the same assumption in the exported functions. The functions could take some specific fields.

Maybe, I couldn't understand correctly what you want to mean.

But I don't think you should be using Kueue's Configuration API. You should probably have your own.

Does this mean that you suggested not mounting another Kueue's Configuration on the in-house Kueue manager?
Or you meant that we shouldn't expose functions with Kueue's Configuration as args?

[alculquicondor]

I think we should export something like SetupControllers(mgr ctrl.Manager, certsReady chan struct{}, opts... jobframework.Options), without making assumptions about how your configuration API looks like.

[tenzen-y]

I think we should export something like SetupControllers(mgr ctrl.Manager, certsReady chan struct{}, opts... jobframework.Options), without making assumptions about how your configuration API looks like.

Ah, it makes sense. Thank you for the clarifications!

[mimowo]

@tenzen-y I see #1630 merged. Is there more to do, or we can close?

[tenzen-y]

@tenzen-y I see #1630 merged. Is there more to do, or we can close?

I'm still working on this, but I'm not sure if we can finalize this by the deadline for v0.6.

[k8s-triage-robot]

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

[tenzen-y]

/remove-lifecycle stale

[k8s-triage-robot]

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

[tenzen-y]

/remove-lifecycle stale

[k8s-triage-robot]

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

[tenzen-y]

/remove-lifecycle stale