search

karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.44k stars 880 forks source link

[Umbrella] Specification of merics and health probe port parameters #5219

Closed whitewindmills closed 1 month ago

whitewindmills commented 3 months ago

What would you like to be added:

Currently, the port parameter definitions of the metrics and health probes are different for each component, which will cause some ambiguity to users, so we plan to standardize the port definitions of each component.

This will be basically the same as controller-manager:

// MetricsBindAddress is the TCP address that the controller should bind to
// for serving prometheus metrics.
// It can be set to "0" to disable the metrics serving.
MetricsBindAddress string

// HealthProbeBindAddress is the TCP address that the controller should bind to
// for serving health probes
// It can be set to "0" to disable serving the health probe.
HealthProbeBindAddress string

Note that you need to mark the old parameters as deprecated and we will remove them in a later version.

whitewindmills commented 3 months ago

/help

karmada-bot commented 3 months ago

@whitewindmills: This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/karmada-io/karmada/issues/5219): >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
RainbowMango commented 3 months ago

Great thanks to @whitewindmills for picking this. We may need two releases to finish this, this release focuses on introducing the new one and depreciating the outdated one. And remove the deprecated flags in the next release. Are we on the same page?

liangyuanpeng commented 3 months ago

/assign for karmada-scheduler

whitewindmills commented 3 months ago

Are we on the same page?

SURE

liangyuanpeng commented 3 months ago

@whitewindmills I checked the webhook and saw that it was working as expected so there was no need to do anything, am I understanding this correctly?

whitewindmills commented 3 months ago

@liangyuanpeng yes, but it's comments need to be fixed.

// HealthProbeBindAddress is the TCP address that the controller should bind to
// for serving health probes
// It can be set to "0" or "" to disable serving the health probe.
seanlaii commented 2 months ago

Hi, I am glad to help! Can I take karmada-scheduler-estimator?

seanlaii commented 2 months ago

/assign for karmada-scheduler-estimator

whitewindmills commented 2 months ago

@seanlaii thanks

seanlaii commented 2 months ago

Hi, I can also take karmada-controller-manager.

whitewindmills commented 2 months ago

@seanlaii go for it

whitewindmills commented 2 months ago

I think we should complete those tasks before the next release. @liangyuanpeng are you free to do it?

whitewindmills commented 1 month ago

ping @liangyuanpeng if you are not available, please let me know. I'll help you with this.

liangyuanpeng commented 1 month ago

sorry for my late, let me open a PR in this week

RainbowMango commented 1 month ago

Let's try to finish this by this week, that means all components share the same deprecation cycle.

RainbowMango commented 1 month ago

@whitewindmills We have 11 componets now, I see you just listed 6 components, have you checked the others?

whitewindmills commented 1 month ago

have you checked the others?

yes

RainbowMango commented 1 month ago

OK. Thanks for the clarification.

RainbowMango commented 1 month ago

For the karmada-interpreter-webhook-example:

Generic flags:
      --bind-address string                                                                                                                                                                                                  
                The IP address on which to listen for the --secure-port port. (default "0.0.0.0")
      --secure-port int                                                                                                                                                                                                      
                The secure port on which to serve HTTPS. (default 8445)

Shall we change it to --health-probe-bind-address?

RainbowMango commented 1 month ago

Same for karmada-search, karmada-metrics-adapter, karmada-aggregated-apiserver:

      --bind-address ip                                                                                                                                                                                                      
                The IP address on which to listen for the --secure-port port. The associated interface(s) must be reachable by the rest of the cluster, and by CLI/web clients. If blank or an unspecified address (0.0.0.0 or
                ::), all interfaces and IP address families will be used. (default 0.0.0.0)
      --secure-port int                                                                                                                                                                                                      
                The port on which to serve HTTPS with authentication and authorization. If 0, don't serve HTTPS at all. (default 443)
whitewindmills commented 1 month ago

Shall we change it to --health-probe-bind-address?

but they're used to listen for the webhook server, not healthz.

RainbowMango commented 1 month ago

Get it.