Skip to content
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

Provide the component config #55

Open
tenzen-y opened this issue Apr 24, 2023 · 18 comments · May be fixed by #609
Open

Provide the component config #55

tenzen-y opened this issue Apr 24, 2023 · 18 comments · May be fixed by #609
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@tenzen-y
Copy link
Member

I would like to provide the component config like kueue: https://github.com/kubernetes-sigs/kueue/blob/9ca57c86cf06c11a94a2d5b7badf60233a51a2f2/config/components/manager/controller_manager_config.yaml

Maybe, as a first step, we can provide the functionality for users to select whether to use the internal cert or the cert-manager.

/kind feature

ref: #52

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 24, 2023
@tenzen-y
Copy link
Member Author

/assign

@tenzen-y
Copy link
Member Author

The component-config is deprecated and will be removed. So we should hold similar schemes as component-config in this repository.

ref: kubernetes-sigs/kueue#725

@tenzen-y
Copy link
Member Author

Maybe, I don't have enough time to work on this.

/unassign

@charles-chenzz
Copy link
Member

I can take it up. do you have any idea on how to implement it or any examples that I can refer to? @tenzen-y
/assign

@tenzen-y
Copy link
Member Author

@charles-chenzz
Copy link
Member

can you be more specify about the ref and implement? I think adding this feature will require us to use code-gen to gen the file and implement the function. then there comes a the key question is: do we actually need this? and are there any more scenes that might need this api/feature?(beside internal-cert and cert manager) @tenzen-y

also /cc @ahg-g for advice

@ahg-g
Copy link
Contributor

ahg-g commented Jun 2, 2023

an you be more specify about the ref and implement? I think adding this feature will require us to use code-gen to gen the file and implement the function.

see https://book.kubebuilder.io/component-config-tutorial/tutorial.html

and are there any more scenes that might need this api/feature?(beside internal-cert and cert manager)
kube connection configuration is important to have to set qps limits, similar to https://github.com/kubernetes-sigs/kueue/blob/main/apis/config/v1beta1/configuration_types.go#L57.

@tenzen-y
Copy link
Member Author

tenzen-y commented Jun 2, 2023

do we actually need this? and are there any more scenes that might need this api/feature?

If we introduce the component config, we can set up many parameters for the controller besides the cert.

Note: ComponentConfig is deprecated. So we must implement all features of the https://github.com/kubernetes-sigs/controller-runtime/tree/30dea34848b0e0b30690dd461a0eadb74d4869f3/pkg/config in this repo.

@charles-chenzz
Copy link
Member

charles-chenzz commented Jun 3, 2023

Note: ComponentConfig is deprecated. So we must implement all features of the https://github.com/kubernetes-sigs/controller-runtime/tree/30dea34848b0e0b30690dd461a0eadb74d4869f3/pkg/config in this repo.

thanks for the heads up, the files said user need to migrate to their own. what it means is that we need to copy the struct to our own repo's codebase or we implement it in some other ways? @tenzen-y

@tenzen-y
Copy link
Member Author

tenzen-y commented Jun 5, 2023

Note: ComponentConfig is deprecated. So we must implement all features of the https://github.com/kubernetes-sigs/controller-runtime/tree/30dea34848b0e0b30690dd461a0eadb74d4869f3/pkg/config in this repo.

thanks for the heads up, the files said user need to migrate to their own. what it means is that we need to copy the struct to our own repo's codebase or we implement it in some other ways? @tenzen-y

Yes, you're right.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 21, 2024
@tenzen-y
Copy link
Member Author

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 21, 2024
@danielvegamyhre
Copy link
Contributor

@charles-chenzz I assume you aren't working on this anymore so i'm going to unassign it, but if you want to implement this please feel free to reassign.

@danielvegamyhre danielvegamyhre added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Feb 16, 2024
@rainfd
Copy link

rainfd commented Jun 22, 2024

/assign

@rainfd
Copy link

rainfd commented Jun 22, 2024

@tenzen-y only implement cert management in this issue?

@rainfd
Copy link

rainfd commented Jun 24, 2024

apiVersion: config.jobset.x-k8s.io/v1alpha1
kind: Configuration
namespace: jobset-system
health:
  healthProbeBindAddress: :8081
metrics:
  bindAddress: :8080
leaderElection:
  leaderElect: true
webhook:
  port: 9443
clientConnection:
  qps: 50
  burst: 100
internalCertManagement:
  enable: false
  webhookServiceName: ""
  webhookSecretName: ""

The basic configuration, same as kueue?

leaderElection:
  leaderElect: true
  resourceName: c1f6bfd2.jobset.x-k8s.io

Do we need to change the leaderelectionid?
@tenzen-y

@danielvegamyhre danielvegamyhre removed the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Jun 24, 2024
@rainfd rainfd linked a pull request Jun 28, 2024 that will close this issue
@tenzen-y
Copy link
Member Author

@tenzen-y only implement cert management in this issue?

I assumed that we could provide a similar functionality as Kueue.
So, it would be useful if we could provide the functionality to modify controller manager options like client qps.

@tenzen-y
Copy link
Member Author

apiVersion: config.jobset.x-k8s.io/v1alpha1
kind: Configuration
namespace: jobset-system
health:
  healthProbeBindAddress: :8081
metrics:
  bindAddress: :8080
leaderElection:
  leaderElect: true
webhook:
  port: 9443
clientConnection:
  qps: 50
  burst: 100
internalCertManagement:
  enable: false
  webhookServiceName: ""
  webhookSecretName: ""

The basic configuration, same as kueue?

leaderElection:
  leaderElect: true
  resourceName: c1f6bfd2.jobset.x-k8s.io

Do we need to change the leaderelectionid? @tenzen-y

Generally, looks great.
Thank you for proposing the design!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants