Skip to content

add support to enable scheduler estimator in helm chart #6286

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

Conversation

mojojoji
Copy link
Contributor

@mojojoji mojojoji commented Apr 11, 2025

What type of PR is this?

/kind feature

What this PR does / why we need it:
This PR adds scheduler.enableSchedulerEstimator to helm values for karmada chart. This allows enables the scheduler to connect to scheduler estimator.

Which issue(s) this PR fixes:
Fixes #4368

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

`Helm Chart`: Added `scheduler.enableSchedulerEstimator` to helm values for karmada chart to allow the scheduler to connect with the scheduler estimator.

@Copilot Copilot AI review requested due to automatic review settings April 11, 2025 14:17
@karmada-bot karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 11, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

@karmada-bot
Copy link
Collaborator

Welcome @mojojoji! It looks like this is your first PR to karmada-io/karmada 🎉

@karmada-bot karmada-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 11, 2025
@codecov-commenter
Copy link

codecov-commenter commented Apr 11, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.14%. Comparing base (5ce1645) to head (305b3b6).
Report is 18 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6286      +/-   ##
==========================================
+ Coverage   48.05%   48.14%   +0.08%     
==========================================
  Files         678      678              
  Lines       56116    56074      -42     
==========================================
+ Hits        26966    26995      +29     
+ Misses      27374    27310      -64     
+ Partials     1776     1769       -7     
Flag Coverage Δ
unittests 48.14% <ø> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mojojoji mojojoji force-pushed the add-scheduler-estimator-enable-option-to-helm branch from 73ea3a0 to a6203e6 Compare April 11, 2025 19:48
@zhzhuang-zju
Copy link
Contributor

/assign

Copy link
Contributor

@zhzhuang-zju zhzhuang-zju left a comment

Choose a reason for hiding this comment

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

@mojojoji Thanks, others LGTM

In addition to this, the component karmada-descheduler will also use the flag --enable-scheduler-estimator. Could you please submit another pull request (PR) to complete the modification of karmada-descheduler?

@mojojoji
Copy link
Contributor Author

@mojojoji Thanks, others LGTM

In addition to this, the component karmada-descheduler will also use the flag --enable-scheduler-estimator. Could you please submit another pull request (PR) to complete the modification of karmada-descheduler?

The karamda-descheduler does not have a -enable-scheduler-estimator argument. I think descheduler assumes that the estimator is running and cannot run without it.

fs.BoolVar(&o.LeaderElection.LeaderElect, "leader-elect", true, "Enable leader election, which must be true when running multi instances.")
fs.StringVar(&o.LeaderElection.ResourceNamespace, "leader-elect-resource-namespace", names.NamespaceKarmadaSystem, "The namespace of resource object that is used for locking during leader election.")
fs.DurationVar(&o.LeaderElection.LeaseDuration.Duration, "leader-elect-lease-duration", defaultElectionLeaseDuration.Duration, ""+
"The duration that non-leader candidates will wait after observing a leadership "+
"renewal until attempting to acquire leadership of a led but unrenewed leader "+
"slot. This is effectively the maximum duration that a leader can be stopped "+
"before it is replaced by another candidate. This is only applicable if leader "+
"election is enabled.")
fs.DurationVar(&o.LeaderElection.RenewDeadline.Duration, "leader-elect-renew-deadline", defaultElectionRenewDeadline.Duration, ""+
"The interval between attempts by the acting master to renew a leadership slot "+
"before it stops leading. This must be less than or equal to the lease duration. "+
"This is only applicable if leader election is enabled.")
fs.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.")
fs.StringVar(&o.KubeConfig, "kubeconfig", o.KubeConfig, "Path to karmada control plane kubeconfig file.")
fs.StringVar(&o.Master, "master", o.Master, "The address of the Kubernetes API server. Overrides any value in KubeConfig. Only required if out-of-cluster.")
fs.Float32Var(&o.KubeAPIQPS, "kube-api-qps", 40.0, "QPS to use while talking with karmada-apiserver.")
fs.IntVar(&o.KubeAPIBurst, "kube-api-burst", 60, "Burst to use while talking with karmada-apiserver.")
fs.DurationVar(&o.SchedulerEstimatorTimeout.Duration, "scheduler-estimator-timeout", 3*time.Second, "Specifies the timeout period of calling the scheduler estimator service.")
fs.IntVar(&o.SchedulerEstimatorPort, "scheduler-estimator-port", defaultEstimatorPort, "The secure port on which to connect the accurate scheduler estimator.")
fs.StringVar(&o.SchedulerEstimatorCertFile, "scheduler-estimator-cert-file", "", "SSL certification file used to secure scheduler estimator communication.")
fs.StringVar(&o.SchedulerEstimatorKeyFile, "scheduler-estimator-key-file", "", "SSL key file used to secure scheduler estimator communication.")
fs.StringVar(&o.SchedulerEstimatorCaFile, "scheduler-estimator-ca-file", "", "SSL Certificate Authority file used to secure scheduler estimator communication.")
fs.BoolVar(&o.InsecureSkipEstimatorVerify, "insecure-skip-estimator-verify", false, "Controls whether verifies the scheduler estimator's certificate chain and host name.")
fs.StringVar(&o.SchedulerEstimatorServiceNamespace, "scheduler-estimator-service-namespace", names.NamespaceKarmadaSystem, "The namespace to be used for discovering scheduler estimator services.")
fs.StringVar(&o.SchedulerEstimatorServicePrefix, "scheduler-estimator-service-prefix", names.KarmadaSchedulerEstimatorComponentName, "The prefix of scheduler estimator service name")
fs.DurationVar(&o.DeschedulingInterval.Duration, "descheduling-interval", defaultDeschedulingInterval, "Time interval between two consecutive descheduler executions. Setting this value instructs the descheduler to run in a continuous loop at the interval specified.")
fs.DurationVar(&o.UnschedulableThreshold.Duration, "unschedulable-threshold", defaultUnschedulableThreshold, "The period of pod unschedulable condition. This value is considered as a classification standard of unschedulable replicas.")
fs.StringVar(&o.MetricsBindAddress, "metrics-bind-address", ":8080", "The TCP address that the server should bind to for serving prometheus metrics(e.g. 127.0.0.1:8080, :8080). It can be set to \"0\" to disable the metrics serving. Defaults to 0.0.0.0:8080.")
fs.StringVar(&o.HealthProbeBindAddress, "health-probe-bind-address", ":10358", "The TCP address that the server should bind to for serving health probes(e.g. 127.0.0.1:10358, :10358). It can be set to \"0\" to disable serving the health probe. Defaults to 0.0.0.0:10358.")
o.ProfileOpts.AddFlags(fs)

@zhzhuang-zju
Copy link
Contributor

The karamda-descheduler does not have a -enable-scheduler-estimator argument. I think descheduler assumes that the estimator is running and cannot run without it.

@mojojoji I'm sorry for giving the incorrect advice. Indeed, the descheduler component doesn't have the "enable-scheduler-estimator" option.

The CI failed due to the DCO check. You can merge these two commits and use git commit -s -m to submit your commit.

@mojojoji mojojoji force-pushed the add-scheduler-estimator-enable-option-to-helm branch from 7af6051 to 6393758 Compare April 14, 2025 11:22
Signed-off-by: Joji Augustine <jojimail@gmail.com>

set enabled scheduler estimator arg to true if true in values

Co-authored-by: zhzhuang-zju <m17799853869@163.com>
Signed-off-by: Joji Augustine <jojimail@gmail.com>

Signed-off-by: Joji Augustine <jojimail@gmail.com>
@mojojoji mojojoji force-pushed the add-scheduler-estimator-enable-option-to-helm branch from 6393758 to 305b3b6 Compare April 14, 2025 11:27
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

I see all comments have been fixed.
Thanks @mojojoji

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 14, 2025
@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

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:

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

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 14, 2025
@karmada-bot karmada-bot merged commit a5cd153 into karmada-io:master Apr 14, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support schedulerEstimator as a list in karmada helm chart
5 participants