Giter Club home page Giter Club logo

Comments (13)

marun avatar marun commented on August 25, 2024

The current set of common parameters required by controllers is captured in ControllerConfig struct. Sourcing this configuration from a configmap or other api resource would allow out-of-tree components to discover this configuration rather than having to duplicate it, and allow kubefed2 to discover the configuration of the cotnrol plane it was targeting (e.g. so that join would know whether it was joining a cluster to a namespaced or cluster-scoped control plane).

from kubefed.

xunpan avatar xunpan commented on August 25, 2024

I think it can be organized as a new FederationConfig API after read https://docs.google.com/document/d/1arP4T9Qkp2SovlJZ_y790sBeiWXDO6SG10pZ_UUU-Lc/edit and based on shashidharatd's proposal.

kind: FederationConfig 
apiVersion: v1 
metadata:
  namespace: federation-system
  name: federation-v2
data:
  install-crds: "false"
  limited-scope: "false"
  registry-namespace: kube-multicluster-public
  controller-duration:
    available-delay: "20s"
    unavailable-delay: "60s"
    user-monitor-period: "40s"
  leader-elect:
    enable: "true"
    lease-duration: "15s"
    renew-deadline: "10s"
    retry-period: "5s"
    resource-lock: configmaps
  feature-gates:
    push-reconciler: true
    scheduler-preferences: true
    cross-cluster-service-discovery: true
    federated-ingress: true

The behavior should be:

  • This resource should be created after federation namespace is created.
  • This resource only take effect in controller start up. Controller will not monitor it and reconcile variations of the resource (at least in current implementation)
  • This configuration will override command line options. Command line options is kept for current users.

The advantages from my point of view is the validation and better extendibility.

@marun @shashidharatd @pmorie

from kubefed.

marun avatar marun commented on August 25, 2024

@xunpan I have a couple of comments:

  • I don't think install-crds should be exposed. It's more a vestige of our kubebuilder heritage than anything, and I think its use should be deprecated.
  • I'm not sure feature-gates should be a hard-coded list. That would complicate a feature gaining sufficient maturity to be enabled without a feature gate, or a feature being deprecated and removed. Is there any prior art in the document to refer to? If not, consider using a list of objects with name/enabled properties, e.g.
  feature-gates:
   - name: push-reconciler
     enabled: true
  • I think the command-line options should be deprecated (and perhaps removed before beta) in favor of accepting the yaml of the configuration type (e.g. via --config=<config.yaml>). This strategy supports command-line users without imposing the burden of coordinating maintenance across the config type and the command line options.

from kubefed.

xunpan avatar xunpan commented on August 25, 2024

@marun

  1. I agree to deprecate --install-crds. I never see it is used in deployment ...
  2. I did not see any best practise for feature-gates settings. Your proposal makes sense.
  3. FederationConfig should be a resource that we created for controller manager configuration. So, if we use --config, does that means controller manager will create/update the FederationConfig based on this yaml file? This seems not good to such changes. Our use case is not same as kubeconfig case.

I also have question:

  1. Do we still need to make registry-namespace independant? What about make it same as federation-system namespace?
    For namespaced federation, we already do it. I guess it is only used by federation v2 in our use case even cluster scoped. An independent registry-namespace is a little confused.
  2. what is our deprecate policy? Delete from code? Or just leave it as is ...
    I think a clean code is better if configuration is the right thing.

from kubefed.

marun avatar marun commented on August 25, 2024

@xunpan For your 3.

  1. FederationConfig should be a resource that we created for controller manager configuration. So, if we use --config, does that means controller manager will create/update the FederationConfig based on this yaml file? This seems not good to such changes. Our use case is not same as kubeconfig case.

My thinking is that provision of yaml via -f would be to support running the controller manager outside of a kube cluster (e.g. for testing). As you say, the use cases within the cluster are both exposing configuration to agents other than the controller manager (e.g. kubefed2 discovering configuration) as well as configuring the controller manager.

  1. Do we still need to make registry-namespace independant? What about make it same as federation-system namespace?
    For namespaced federation, we already do it. I guess it is only used by federation v2 in our use case even cluster scoped. An independent registry-namespace is a little confused.

In a world with a single federation control plane and no other consumers of clusters, it might make sense to have clusters only in the same namespace. But if more than one federation control plane (namespaced or otherwise) exists in a cluster, it may make sense to share a namespace containing available clusters. And if there are consumers of clusters other than federation - and its been suggested that this is the case - there is an added need to support a configurable cluster namespace.

That said, I think it might make sense to default the registry namespace to the federation namespace rather than kube-multicluster-public, so long as any given deployment can choose to override that decision.

cc: @pmorie

  1. what is our deprecate policy? Delete from code? Or just leave it as is ...
    I think a clean code is better if configuration is the right thing.

As per recent breaking API changes, I think deprecation before we go beta is simply removal. That means we have a limited window left to do this kind of invasive work.

from kubefed.

xunpan avatar xunpan commented on August 25, 2024

Follow up actions:

  • make installer script uses federationconfig for configuration
  • make helm chart uses federationconfig for configuration
    - [ ] make kubectl disable reject in namespaced federation.
    does not need anymore because we do not make nanespaced kubectl delete CRD.

from kubefed.

pmorie avatar pmorie commented on August 25, 2024

A couple notes here while they're in my head:

I think one input to determining what the shape of the eventual config API is knowing the outcome of #688.

The right granularity for configuration APIs seems like one per functional area. That would yield:

  • workloads/sync
  • dns
  • replica scheduling

Having distinct APIs for different functional areas would make it easier to evolve and version config APIs independently, which is desireable.

from kubefed.

marun avatar marun commented on August 25, 2024

The only options we set today are for all controllers - there are no options specific to the functional areas you mention.

from kubefed.

pmorie avatar pmorie commented on August 25, 2024

A couple notes here while they're in my head:

I think one input to determining what the shape of the eventual config API is knowing the outcome of #688.

The right granularity for configuration APIs seems like one per functional area. That would yield:

  • workloads/sync
  • dns
  • replica scheduling

Having distinct APIs for different functional areas would make it easier to evolve and version config APIs independently, which is desireable.

from kubefed.

pmorie avatar pmorie commented on August 25, 2024

Whoops - did not mean to close this.

from kubefed.

marun avatar marun commented on August 25, 2024

Work is ongoing to implement support for FederationConfig via #729.

from kubefed.

xunpan avatar xunpan commented on August 25, 2024

@pmorie @marun
#729 is closed and FederationConfig is merged. Is that OK to close this ticket?

from kubefed.

marun avatar marun commented on August 25, 2024

Yes, I think this can be closed. Thank you!

from kubefed.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.