Skip to content

Remove deprecated flags in controller manager options #7271

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.

Sign up for GitHub

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

Merged
karmada-bot merged 1 commit into from
Mar 31, 2026
Merged

Remove deprecated flags in controller manager options #7271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 32 additions & 36 deletions cmd/controller-manager/app/controllermanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,23 +353,21 @@ func startClusterStatusController(ctx controllerscontext.Context) (enabled bool,
},
}
clusterStatusController := &status.ClusterStatusController{
Client: mgr.GetClient(),
KubeClient: kubeclientset.NewForConfigOrDie(mgr.GetConfig()),
EventRecorder: mgr.GetEventRecorderFor(status.ControllerName), //nolint:staticcheck // Note: GetEventRecorderFor is deprecated in controller-runtime v0.23.0 in favor of GetEventRecorder. This changes event API from v1 events to events.k8s.io. We need to migrate carefully, especially considering the impact on users and RBAC permission changes in installation/deployment tools.
PredicateFunc: clusterPredicateFunc,
TypedInformerManager: typedmanager.GetInstance(),
GenericInformerManager: genericmanager.GetInstance(),
ClusterClientSetFunc: util.NewClusterClientSet,
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSet,
ClusterClientOption: ctx.ClusterClientOption,
ClusterStatusUpdateFrequency: opts.ClusterStatusUpdateFrequency,
ClusterLeaseDuration: opts.ClusterLeaseDuration,
ClusterLeaseRenewIntervalFraction: opts.ClusterLeaseRenewIntervalFraction,
ClusterSuccessThreshold: opts.ClusterSuccessThreshold,
ClusterFailureThreshold: opts.ClusterFailureThreshold,
ClusterCacheSyncTimeout: opts.ClusterCacheSyncTimeout,
RateLimiterOptions: ctx.Opts.RateLimiterOptions,
EnableClusterResourceModeling: ctx.Opts.EnableClusterResourceModeling,
Client: mgr.GetClient(),
KubeClient: kubeclientset.NewForConfigOrDie(mgr.GetConfig()),
EventRecorder: mgr.GetEventRecorderFor(status.ControllerName), //nolint:staticcheck // Note: GetEventRecorderFor is deprecated in controller-runtime v0.23.0 in favor of GetEventRecorder. This changes event API from v1 events to events.k8s.io. We need to migrate carefully, especially considering the impact on users and RBAC permission changes in installation/deployment tools.
PredicateFunc: clusterPredicateFunc,
TypedInformerManager: typedmanager.GetInstance(),
GenericInformerManager: genericmanager.GetInstance(),
ClusterClientSetFunc: util.NewClusterClientSet,
ClusterDynamicClientSetFunc: util.NewClusterDynamicClientSet,
ClusterClientOption: ctx.ClusterClientOption,
ClusterStatusUpdateFrequency: opts.ClusterStatusUpdateFrequency,
ClusterSuccessThreshold: opts.ClusterSuccessThreshold,
ClusterFailureThreshold: opts.ClusterFailureThreshold,
ClusterCacheSyncTimeout: opts.ClusterCacheSyncTimeout,
RateLimiterOptions: ctx.Opts.RateLimiterOptions,
EnableClusterResourceModeling: ctx.Opts.EnableClusterResourceModeling,
}
if err := clusterStatusController.SetupWithManager(mgr); err != nil {
return false, err
Expand Down Expand Up @@ -912,25 +910,23 @@ func setupControllers(ctx context.Context, mgr controllerruntime.Manager, opts *
Mgr: mgr,
ObjectWatcher: objectWatcher,
Opts: controllerscontext.Options{
Controllers: opts.Controllers,
ClusterMonitorPeriod: opts.ClusterMonitorPeriod,
ClusterMonitorGracePeriod: opts.ClusterMonitorGracePeriod,
ClusterStartupGracePeriod: opts.ClusterStartupGracePeriod,
ClusterStatusUpdateFrequency: opts.ClusterStatusUpdateFrequency,
ClusterLeaseDuration: opts.ClusterLeaseDuration,
ClusterLeaseRenewIntervalFraction: opts.ClusterLeaseRenewIntervalFraction,
ClusterSuccessThreshold: opts.ClusterSuccessThreshold,
ClusterFailureThreshold: opts.ClusterFailureThreshold,
ClusterCacheSyncTimeout: opts.ClusterCacheSyncTimeout,
SkippedPropagatingNamespaces: opts.SkippedNamespacesRegexps(),
ConcurrentWorkSyncs: opts.ConcurrentWorkSyncs,
EnableTaintManager: opts.EnableTaintManager,
RateLimiterOptions: opts.RateLimiterOpts,
GracefulEvictionTimeout: opts.GracefulEvictionTimeout,
EnableClusterResourceModeling: opts.EnableClusterResourceModeling,
HPAControllerConfiguration: opts.HPAControllerConfiguration,
FederatedResourceQuotaOptions: opts.FederatedResourceQuotaOptions,
ClusterFailoverConfiguration: opts.ClusterFailoverOptions,
Controllers: opts.Controllers,
ClusterMonitorPeriod: opts.ClusterMonitorPeriod,
ClusterMonitorGracePeriod: opts.ClusterMonitorGracePeriod,
ClusterStartupGracePeriod: opts.ClusterStartupGracePeriod,
ClusterStatusUpdateFrequency: opts.ClusterStatusUpdateFrequency,
ClusterSuccessThreshold: opts.ClusterSuccessThreshold,
ClusterFailureThreshold: opts.ClusterFailureThreshold,
ClusterCacheSyncTimeout: opts.ClusterCacheSyncTimeout,
SkippedPropagatingNamespaces: opts.SkippedNamespacesRegexps(),
ConcurrentWorkSyncs: opts.ConcurrentWorkSyncs,
EnableTaintManager: opts.EnableTaintManager,
RateLimiterOptions: opts.RateLimiterOpts,
GracefulEvictionTimeout: opts.GracefulEvictionTimeout,
EnableClusterResourceModeling: opts.EnableClusterResourceModeling,
HPAControllerConfiguration: opts.HPAControllerConfiguration,
FederatedResourceQuotaOptions: opts.FederatedResourceQuotaOptions,
ClusterFailoverConfiguration: opts.ClusterFailoverOptions,
},
Context: ctx,
DynamicClientSet: dynamicClientSet,
Expand Down
12 changes: 0 additions & 12 deletions cmd/controller-manager/app/options/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,6 @@ type Options struct {
// ClusterStatusUpdateFrequency is the frequency that controller computes and report cluster status.
// It must work with ClusterMonitorGracePeriod(--cluster-monitor-grace-period) in karmada-controller-manager.
ClusterStatusUpdateFrequency metav1.Duration
// ClusterLeaseDuration is a duration that candidates for a lease need to wait to force acquire it.
// This is measure against time of last observed lease RenewTime.
ClusterLeaseDuration metav1.Duration
// ClusterLeaseRenewIntervalFraction is a fraction coordinated with ClusterLeaseDuration that
// how long the current holder of a lease has last updated the lease.
ClusterLeaseRenewIntervalFraction float64
// ClusterSuccessThreshold is the duration of successes for the cluster to be considered healthy after recovery.
ClusterSuccessThreshold metav1.Duration
// ClusterFailureThreshold is the duration of failure for the cluster to be considered unhealthy.
Expand Down Expand Up @@ -185,12 +179,6 @@ func (o *Options) AddFlags(flags *pflag.FlagSet, allControllers, disabledByDefau
flags.DurationVar(&o.LeaderElection.RetryPeriod.Duration, "leader-elect-retry-period", defaultElectionRetryPeriod.Duration, ""+
"The duration the clients should wait between attempting acquisition and renewal "+
"of a leadership. This is only applicable if leader election is enabled.")
flags.DurationVar(&o.ClusterLeaseDuration.Duration, "cluster-lease-duration", 40*time.Second,
"Specifies the expiration period of a cluster lease.")
_ = flags.MarkDeprecated("cluster-lease-duration", "The flag --cluster-lease-duration has been marked deprecated because it has never been used, and will be removed in a future release.")
flags.Float64Var(&o.ClusterLeaseRenewIntervalFraction, "cluster-lease-renew-interval-fraction", 0.25,
"Specifies the cluster lease renew interval fraction.")
_ = flags.MarkDeprecated("cluster-lease-renew-interval-fraction", "The flag --cluster-lease-renew-interval-fraction has been marked deprecated because it has never been used, and will be removed in a future release.")
flags.DurationVar(&o.ClusterSuccessThreshold.Duration, "cluster-success-threshold", 30*time.Second, "The duration of successes for the cluster to be considered healthy after recovery.")
flags.DurationVar(&o.ClusterFailureThreshold.Duration, "cluster-failure-threshold", 30*time.Second, "The duration of failure for the cluster to be considered unhealthy.")
Comment on lines 179 to 183
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing these flag bindings also removes the only place ClusterLeaseDuration and ClusterLeaseRenewIntervalFraction get non-zero defaults. NewOptions() currently leaves them at zero values, but opts.Validate() requires ClusterLeaseDuration > 0 and 0 < ClusterLeaseRenewIntervalFraction < 1, so karmada-controller-manager will now fail to start. Set defaults for these fields in NewOptions() (e.g., 40s and 0.25 as before) or remove/adjust the validation + downstream usage if they are truly no longer needed.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@dahuo98 dahuo98 Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per discussed here #7009 (comment), we will stop populating these fields. I removed the validation checks on these 2 fields

flags.DurationVar(&o.ClusterMonitorPeriod.Duration, "cluster-monitor-period", 5*time.Second,
Expand Down
6 changes: 0 additions & 6 deletions cmd/controller-manager/app/options/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ func (o *Options) Validate() field.ErrorList {
if o.ClusterStatusUpdateFrequency.Duration <= 0 {
errs = append(errs, field.Invalid(newPath.Child("ClusterStatusUpdateFrequency"), o.ClusterStatusUpdateFrequency, "must be greater than 0"))
}
if o.ClusterLeaseDuration.Duration <= 0 {
errs = append(errs, field.Invalid(newPath.Child("ClusterLeaseDuration"), o.ClusterLeaseDuration, "must be greater than 0"))
}
if o.ClusterLeaseRenewIntervalFraction <= 0 || o.ClusterLeaseRenewIntervalFraction >= 1 {
errs = append(errs, field.Invalid(newPath.Child("ClusterLeaseRenewIntervalFraction"), o.ClusterLeaseRenewIntervalFraction, "must be greater than 0 and less than 1"))
}
if o.ClusterMonitorPeriod.Duration <= 0 {
errs = append(errs, field.Invalid(newPath.Child("ClusterMonitorPeriod"), o.ClusterMonitorPeriod, "must be greater than 0"))
}
Comment on lines 37 to 42
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation for ClusterLeaseDuration / ClusterLeaseRenewIntervalFraction has been removed, but those fields are still present on Options (see cmd/controller-manager/app/options/options.go). If the fields are meant to be removed along with the flags, consider deleting them from the struct; if they’re intentionally kept for internal wiring, consider keeping basic validation or setting explicit defaults to prevent invalid/zero values from silently propagating.

Copilot uses AI. Check for mistakes.
Expand Down
30 changes: 5 additions & 25 deletions cmd/controller-manager/app/options/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,11 @@ type ModifyOptions func(option *Options)
// New an Options with default parameters
func New(modifyOptions ModifyOptions) Options {
option := Options{
SkippedPropagatingAPIs: "cluster.karmada.io;policy.karmada.io;work.karmada.io",
ClusterStatusUpdateFrequency: metav1.Duration{Duration: 10 * time.Second},
ClusterLeaseDuration: metav1.Duration{Duration: 10 * time.Second},
ClusterMonitorPeriod: metav1.Duration{Duration: 10 * time.Second},
ClusterMonitorGracePeriod: metav1.Duration{Duration: 10 * time.Second},
ClusterStartupGracePeriod: metav1.Duration{Duration: 10 * time.Second},
ClusterLeaseRenewIntervalFraction: 0.25,
SkippedPropagatingAPIs: "cluster.karmada.io;policy.karmada.io;work.karmada.io",
ClusterStatusUpdateFrequency: metav1.Duration{Duration: 10 * time.Second},
ClusterMonitorPeriod: metav1.Duration{Duration: 10 * time.Second},
ClusterMonitorGracePeriod: metav1.Duration{Duration: 10 * time.Second},
ClusterStartupGracePeriod: metav1.Duration{Duration: 10 * time.Second},
FederatedResourceQuotaOptions: FederatedResourceQuotaOptions{
ResourceQuotaSyncPeriod: metav1.Duration{
Duration: 10 * time.Second,
Expand Down Expand Up @@ -78,24 +76,6 @@ func TestValidateControllerManagerConfiguration(t *testing.T) {
}),
expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterStatusUpdateFrequency"), metav1.Duration{Duration: -10 * time.Second}, "must be greater than 0")},
},
"invalid ClusterLeaseDuration": {
opt: New(func(options *Options) {
options.ClusterLeaseDuration.Duration = -40 * time.Second
}),
expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterLeaseDuration"), metav1.Duration{Duration: -40 * time.Second}, "must be greater than 0")},
},
"invalid ClusterLeaseRenewIntervalFraction": {
opt: New(func(options *Options) {
options.ClusterLeaseRenewIntervalFraction = 1.2
}),
expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterLeaseRenewIntervalFraction"), float64(1.2), "must be greater than 0 and less than 1")},
},
"invalid ClusterLeaseRenewIntervalFraction (negative)": {
opt: New(func(options *Options) {
options.ClusterLeaseRenewIntervalFraction = -0.1
}),
expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ClusterLeaseRenewIntervalFraction"), float64(-0.1), "must be greater than 0 and less than 1")},
},
"invalid ClusterMonitorPeriod": {
opt: New(func(options *Options) {
options.ClusterMonitorPeriod.Duration = -40 * time.Second
Expand Down
Loading