add controller gate flag

[googs1025]

What type of PR is this?

/kind feature

What this PR does / why we need it:

• add controller gate flag

Which issue(s) this PR fixes:

Fix #3485

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

None

[Monokaix]

We should wrap a methed like KnownControllers which will call ForeachController to get all controllers' name dynamically : )

[googs1025]

Sorry, I don't quite understand what you mean.
I use isControllerEnabled to determine whether a specific controller needs to be started.

func startControllers(config *rest.Config, opt *options.ServerOption) func(ctx context.Context) {
	controllerOpt := &framework.ControllerOption{}

	controllerOpt.SchedulerNames = opt.SchedulerNames
	controllerOpt.WorkerNum = opt.WorkerThreads
	controllerOpt.MaxRequeueNum = opt.MaxRequeueNum

	// TODO: add user agent for different controllers
	controllerOpt.KubeClient = kubeclientset.NewForConfigOrDie(config)
	controllerOpt.VolcanoClient = vcclientset.NewForConfigOrDie(config)
	controllerOpt.SharedInformerFactory = informers.NewSharedInformerFactory(controllerOpt.KubeClient, 0)
	controllerOpt.InheritOwnerAnnotations = opt.InheritOwnerAnnotations
	controllerOpt.WorkerThreadsForPG = opt.WorkerThreadsForPG
	controllerOpt.WorkerThreadsForGC = opt.WorkerThreadsForGC

	return func(ctx context.Context) {
		framework.ForeachController(func(c framework.Controller) {
			// if controller is not enabled, skip it
			if !isControllerEnabled(c.Name(), opt.Controllers) {
				klog.Infof("Controller <%s> is not enabled", c.Name())
				return
			}
			if err := c.Initialize(controllerOpt); err != nil {
				klog.Errorf("Failed to initialize controller <%s>: %v", c.Name(), err)
				return
			}

			go c.Run(ctx.Done())
		})

		<-ctx.Done()
	}
}

[Monokaix]

Yeah，it's ok to your implement，I mean we should call a method in flag description，the method will print all controllers，instead of directly write all controllers in a hard code way, you can refer to kube-controller-manager's flag
description.

[googs1025]

OK, I understand. Thanks for your suggestion.

[googs1025]

I have read the code related to kube-controller-manager. Do we need to use a similar method?

// AddFlags adds flags related to generic for controller manager to the specified FlagSet.
func (o *GenericControllerManagerConfigurationOptions) AddFlags(fss *cliflag.NamedFlagSets, allControllers, disabledByDefaultControllers []string, controllerAliases map[string]string) {
	if o == nil {
		return
	}

	o.Debugging.AddFlags(fss.FlagSet("debugging"))
	o.LeaderMigration.AddFlags(fss.FlagSet("leader-migration"))
	genericfs := fss.FlagSet("generic")
	genericfs.DurationVar(&o.MinResyncPeriod.Duration, "min-resync-period", o.MinResyncPeriod.Duration, "The resync period in reflectors will be random between MinResyncPeriod and 2*MinResyncPeriod.")
	genericfs.StringVar(&o.ClientConnection.ContentType, "kube-api-content-type", o.ClientConnection.ContentType, "Content type of requests sent to apiserver.")
	genericfs.Float32Var(&o.ClientConnection.QPS, "kube-api-qps", o.ClientConnection.QPS, "QPS to use while talking with kubernetes apiserver.")
	genericfs.Int32Var(&o.ClientConnection.Burst, "kube-api-burst", o.ClientConnection.Burst, "Burst to use while talking with kubernetes apiserver.")
	genericfs.DurationVar(&o.ControllerStartInterval.Duration, "controller-start-interval", o.ControllerStartInterval.Duration, "Interval between starting controller managers.")
	genericfs.StringSliceVar(&o.Controllers, "controllers", o.Controllers, fmt.Sprintf(""+
		"A list of controllers to enable. '*' enables all on-by-default controllers, 'foo' enables the controller "+
		"named 'foo', '-foo' disables the controller named 'foo'.\nAll controllers: %s\nDisabled-by-default controllers: %s",
		strings.Join(allControllers, ", "), strings.Join(disabledByDefaultControllers, ", ")))

	options.BindLeaderElectionFlags(&o.LeaderElection, genericfs)
}

[Monokaix]

Yeah we need a method to get all controllers name list, and we can new a func like KnownControllers, and we can use ForeachController to do that.

[Monokaix]

Just like this:

func KnownControllers() []string {
	controllerNames := []string{}
	fn := func(controller framework.Controller) {
		controllerNames = append(controllerNames, controller.Name())
	}
	framework.ForeachController(fn)
	return controllerNames
}

[googs1025]

OK, I will make some changes in the next few days.

[Monokaix]

Maybe move the func to cmd/controller-manager/main.go is better, because it's where the controllers are registered, and we should remove importing controllers in cmd/controller-manager/app/options/options.go.

[Monokaix]

Hi, please also notice this piece, we can merge it after this is done: ）

[googs1025]

I will deal with this issue in the next two days. Thanks for the reminder.

[googs1025]

In main.go, I use the knownControllers function variable to ensure the simplicity of main.go

// knownControllers is a list of all known controllers.
	var knownControllers = func() []string {
		controllerNames := []string{}
		fn := func(controller framework.Controller) {
			controllerNames = append(controllerNames, controller.Name())
		}
		framework.ForeachController(fn)
		sort.Strings(controllerNames)
		return controllerNames
	}

[hwdef]

"+gc-controller, -gc-controller, *"

Have you considered this case?

I think a function should be added to check for incorrect settings, such as a controller being set open and close at the same time.

[googs1025]

"+gc-controller, -gc-controller, *"

Have you considered this case?

I think a function should be added to check for incorrect settings, such as a controller being set open and close at the same time.

good catch! thank @hwdef i will fixed this case

[Monokaix]

IMO, it's a little complicated and not so readable here, the controller in front has higher priority is just simple and acceptable.
And if this check is truly needed, I think use two maps to check is better.

[googs1025]

This check only verifies whether the same controller exists at the same time. For example, if "+job-controller" and "-job-controller" exist at the same time. No priority is considered.

[hwdef]

/lgtm

[hwdef]

/lgtm

[william-wang]

/approve

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: william-wang

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 [william-wang]

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