Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion cmd/agent/app/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ func setupControllers(ctx context.Context, mgr controllerruntime.Manager, opts *
sharedFactory.Start(ctx.Done())
sharedFactory.WaitForCacheSync(ctx.Done())

resourceInterpreter := resourceinterpreter.NewResourceInterpreter(controlPlaneInformerManager, serviceLister)
resourceInterpreter := resourceinterpreter.NewResourceInterpreter(controlPlaneInformerManager, serviceLister, opts.ConfigurableInterpreterLuaVMPoolSize, opts.ThirdPartyInterpreterLuaVMPoolSize)
if err := resourceInterpreter.Start(ctx); err != nil {
return fmt.Errorf("failed to start resource interpreter: %w", err)
}
Expand Down
7 changes: 7 additions & 0 deletions cmd/agent/app/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ type Options struct {
// KubeAPIBurst is the burst to allow while talking with karmada-apiserver.
KubeAPIBurst int

// ConfigurableInterpreterLuaVMPoolSize is the size of the pool for the configurable interpreter lua vm.
ConfigurableInterpreterLuaVMPoolSize int
// ThirdPartyInterpreterLuaVMPoolSize is the size of the pool for the third party interpreter lua vm.
ThirdPartyInterpreterLuaVMPoolSize int

ClusterCacheSyncTimeout metav1.Duration
// ResyncPeriod is the base frequency the informers are resynced.
// Defaults to 0, which means the created informer will never do resyncs.
Expand Down Expand Up @@ -211,6 +216,8 @@ func (o *Options) AddFlags(fs *pflag.FlagSet, allControllers []string) {
fs.DurationVar(&o.CertRotationCheckingInterval, "cert-rotation-checking-interval", 5*time.Minute, "The interval of checking if the certificate need to be rotated. This is only applicable if cert rotation is enabled")
fs.Float64Var(&o.CertRotationRemainingTimeThreshold, "cert-rotation-remaining-time-threshold", 0.2, "The threshold of remaining time of the valid certificate. This is only applicable if cert rotation is enabled.")
fs.StringVar(&o.KarmadaKubeconfigNamespace, "karmada-kubeconfig-namespace", "karmada-system", "Namespace of the secret containing karmada-agent certificate. This is only applicable if cert rotation is enabled.")
fs.IntVar(&o.ConfigurableInterpreterLuaVMPoolSize, "configurable-interpreter-lua-vm-pool-size", 10, "The size of the pool for the configurable interpreter lua vm.")
fs.IntVar(&o.ThirdPartyInterpreterLuaVMPoolSize, "third-party-interpreter-lua-vm-pool-size", 10, "The size of the pool for the third party interpreter lua vm.")
o.RateLimiterOpts.AddFlags(fs)
features.FeatureGate.AddFlag(fs)
o.ProfileOpts.AddFlags(fs)
Expand Down
2 changes: 1 addition & 1 deletion cmd/controller-manager/app/controllermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ func setupControllers(ctx context.Context, mgr controllerruntime.Manager, opts *
sharedFactory.Start(ctx.Done())
sharedFactory.WaitForCacheSync(ctx.Done())

resourceInterpreter := resourceinterpreter.NewResourceInterpreter(controlPlaneInformerManager, serviceLister)
resourceInterpreter := resourceinterpreter.NewResourceInterpreter(controlPlaneInformerManager, serviceLister, opts.ConfigurableInterpreterLuaVMPoolSize, opts.ThirdPartyInterpreterLuaVMPoolSize)
if err := resourceInterpreter.Start(ctx); err != nil {
klog.Fatalf("Failed to start resource interpreter: %v", err)
}
Expand Down
7 changes: 6 additions & 1 deletion cmd/controller-manager/app/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@ type Options struct {
// GracefulEvictionTimeout is the timeout period waiting for the grace-eviction-controller performs the final
// removal since the workload(resource) has been moved to the graceful eviction tasks.
GracefulEvictionTimeout metav1.Duration
// ConfigurableInterpreterLuaVMPoolSize is the size of the pool for the configurable interpreter lua vm.
ConfigurableInterpreterLuaVMPoolSize int
// ThirdPartyInterpreterLuaVMPoolSize is the size of the pool for the third party interpreter lua vm.
ThirdPartyInterpreterLuaVMPoolSize int

RateLimiterOpts ratelimiterflag.Options
ProfileOpts profileflag.Options
Expand Down Expand Up @@ -228,7 +232,8 @@ func (o *Options) AddFlags(flags *pflag.FlagSet, allControllers, disabledByDefau
flags.BoolVar(&o.EnableClusterResourceModeling, "enable-cluster-resource-modeling", true, "Enable means controller would build resource modeling for each cluster by syncing Nodes and Pods resources.\n"+
"The resource modeling might be used by the scheduler to make scheduling decisions in scenario of dynamic replica assignment based on cluster free resources.\n"+
"Disable if it does not fit your cases for better performance.")

flags.IntVar(&o.ConfigurableInterpreterLuaVMPoolSize, "configurable-interpreter-lua-vm-pool-size", 10, "The size of the pool for the configurable interpreter lua vm.")
flags.IntVar(&o.ThirdPartyInterpreterLuaVMPoolSize, "third-party-interpreter-lua-vm-pool-size", 10, "The size of the pool for the third party interpreter lua vm.")
o.RateLimiterOpts.AddFlags(flags)
o.ProfileOpts.AddFlags(flags)
o.HPAControllerConfiguration.AddFlags(flags)
Expand Down
2 changes: 1 addition & 1 deletion pkg/karmadactl/interpret/execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (o *Options) runExecute() error {
Replica: int64(o.DesiredReplica),
}

configurableInterpreter := declarative.NewConfigurableInterpreter(nil)
configurableInterpreter := declarative.NewConfigurableInterpreter(nil, 10)
configurableInterpreter.LoadConfig(customizations)

r := o.Rules.GetByOperation(o.Operation)
Expand Down
4 changes: 2 additions & 2 deletions pkg/karmadactl/promote/promote.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,8 @@ func (o *CommandPromoteOption) promoteDeps(memberClusterFactory cmdutil.Factory,
sharedFactory.WaitForCacheSync(ctx.Done())

defaultInterpreter := native.NewDefaultInterpreter()
thirdpartyInterpreter := thirdparty.NewConfigurableInterpreter()
configurableInterpreter := declarative.NewConfigurableInterpreter(controlPlaneInformerManager)
thirdpartyInterpreter := thirdparty.NewConfigurableInterpreter(10)
configurableInterpreter := declarative.NewConfigurableInterpreter(controlPlaneInformerManager, 10)
Comment on lines +467 to +468

Choose a reason for hiding this comment

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

medium

Similar to pkg/karmadactl/interpret/execute.go, the pool size 10 is hardcoded here. To improve maintainability and avoid magic numbers, it would be better to define a constant for this default value in a shared location and use it consistently across the codebase.

customizedInterpreter, err := webhook.NewCustomizedInterpreter(controlPlaneInformerManager, serviceLister)
if err != nil {
return fmt.Errorf("failed to create customized interpreter: %v", err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ type ConfigurableInterpreter struct {

// NewConfigurableInterpreter builds a new interpreter by registering the
// event handler to the provided informer instance.
Comment on lines 43 to 44

Choose a reason for hiding this comment

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

medium

The function comment for NewConfigurableInterpreter should be updated to describe the new pool parameter, as per the repository's style guide.

Suggested change
// NewConfigurableInterpreter builds a new interpreter by registering the
// event handler to the provided informer instance.
// NewConfigurableInterpreter builds a new interpreter by registering the
// event handler to the provided informer instance. The pool parameter specifies the size of the Lua VM pool.
References
  1. All exported functions must be documented with clear and concise comments describing their purpose and behavior. (link)

func NewConfigurableInterpreter(informer genericmanager.SingleClusterInformerManager) *ConfigurableInterpreter {
func NewConfigurableInterpreter(informer genericmanager.SingleClusterInformerManager, pool int) *ConfigurableInterpreter {
return &ConfigurableInterpreter{
configManager: configmanager.NewInterpreterConfigManager(informer),
// TODO: set an appropriate pool size.

Choose a reason for hiding this comment

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

medium

This TODO comment can be removed as the pool size is now configurable via the pool parameter.

luaVM: luavm.New(false, 10),
luaVM: luavm.New(false, pool),
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/resourceinterpreter/default/thirdparty/thirdparty.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,9 @@ func (p *ConfigurableInterpreter) getCustomAccessor(kind schema.GroupVersionKind
}

// NewConfigurableInterpreter return a new ConfigurableInterpreter.
func NewConfigurableInterpreter() *ConfigurableInterpreter {
func NewConfigurableInterpreter(pool int) *ConfigurableInterpreter {

Choose a reason for hiding this comment

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

medium

The function comment for NewConfigurableInterpreter should be updated to describe the new pool parameter, as per the repository's style guide.

// NewConfigurableInterpreter return a new ConfigurableInterpreter with a specific Lua VM pool size.
References
  1. All exported functions must be documented with clear and concise comments describing their purpose and behavior. (link)

return &ConfigurableInterpreter{
configManager: NewThirdPartyConfigManager(),
luaVM: luavm.New(false, 10),
luaVM: luavm.New(false, pool),
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ type IndividualTest struct {
}

func checkInterpretationRule(t *testing.T, path string, configs []*configv1alpha1.ResourceInterpreterCustomization) {
ipt := declarative.NewConfigurableInterpreter(nil)
ipt := declarative.NewConfigurableInterpreter(nil, 10)
ipt.LoadConfig(configs)

dir := filepath.Dir(path)
Expand Down
15 changes: 10 additions & 5 deletions pkg/resourceinterpreter/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,12 @@ type ResourceInterpreter interface {
}

// NewResourceInterpreter builds a new ResourceInterpreter object.
func NewResourceInterpreter(informer genericmanager.SingleClusterInformerManager, serviceLister corev1.ServiceLister) ResourceInterpreter {
func NewResourceInterpreter(informer genericmanager.SingleClusterInformerManager, serviceLister corev1.ServiceLister, configurablePool, thirdpartyPool int) ResourceInterpreter {
return &customResourceInterpreterImpl{
informer: informer,
serviceLister: serviceLister,
informer: informer,
serviceLister: serviceLister,
configurablePool: configurablePool,
thirdpartyPool: thirdpartyPool,
}
}

Expand All @@ -96,6 +98,9 @@ type customResourceInterpreterImpl struct {
customizedInterpreter *webhook.CustomizedInterpreter
thirdpartyInterpreter *thirdparty.ConfigurableInterpreter
defaultInterpreter *native.DefaultInterpreter

configurablePool int
thirdpartyPool int
}

// Start initializes all interpreters and load all ResourceInterpreterCustomization and
Expand All @@ -109,9 +114,9 @@ func (i *customResourceInterpreterImpl) Start(_ context.Context) (err error) {
if err != nil {
return err
}
i.configurableInterpreter = declarative.NewConfigurableInterpreter(i.informer)
i.configurableInterpreter = declarative.NewConfigurableInterpreter(i.informer, i.configurablePool)

i.thirdpartyInterpreter = thirdparty.NewConfigurableInterpreter()
i.thirdpartyInterpreter = thirdparty.NewConfigurableInterpreter(i.thirdpartyPool)
i.defaultInterpreter = native.NewDefaultInterpreter()

i.informer.Start()
Expand Down