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 a ComponentConfig to tweak the Manager #518

Closed
johnsonj opened this issue Jul 10, 2019 · 14 comments
Closed

Provide a ComponentConfig to tweak the Manager #518

johnsonj opened this issue Jul 10, 2019 · 14 comments
Assignees
Labels
kind/design Categorizes issue or PR as related to design. kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. wg/component-standard Categorizes an issue or PR as relevant to WG Component Standard.
Milestone

Comments

@johnsonj
Copy link

Controllers would like to tweak the settings of the manager (eg, change logging verbosity) through a ComponentConfig.

Additionally, individual controllers may want to expose settings that relate to their usecase (eg, a whitelist of git repos an operator can pull from). I'm not sure if that's a usecase worth splitting off but we would like to use this for addon-operators

/cc @stealthybox @shawn-hurley @DirectXMan12

@DirectXMan12
Copy link
Contributor

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 6, 2019
@DirectXMan12
Copy link
Contributor

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Aug 6, 2019
@stealthybox
Copy link

Just to add context, ComponentConfig implies using API Machinery for the types used in a config file. (Not just unmarshalling a Yaml file into a Go struct)

More context in the component-base KEP:
https://github.com/luxas/enhancements/blob/f3005915981cf3e590dc0744ad0256cdf76970aa/keps/sig-cluster-lifecycle/0032-create-a-k8s-io-component-repo.md#part-1-componentconfig

@stealthybox
Copy link

I would imagine we would have some fields to configure the manager's SharedInformer?

@DirectXMan12
Copy link
Contributor

Yeah, and the various other bits of the manager that can easily be configured with constant values (like metrics and webhook serving ports)

@stealthybox
Copy link

/wg component-standard

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/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 Dec 20, 2019
@DirectXMan12
Copy link
Contributor

/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 6, 2020
@christopherhein
Copy link
Member

christopherhein commented Feb 13, 2020

@DirectXMan12 @johnsonj @stealthybox I'm interested in looking into what this might take, has anyone made any progress or have thoughts about this?

Loosely looking through the codebase it seems like we could make it so that controller-runtime.Manager could support a separate New function specifically for pulling in ComponentConfig type resources. We could make this and interface for which the ComponentConfig stuct would have to adhere too.

I'm thinking something along the lines of:

// ManagerConfiguration defines what the ComponentConfig object for ControllerRuntime needs to support
type ManagerConfiguration interface {
	GetSyncPeriod() *time.Duration

	GetLeaderElection() bool
	GetLeaderElectionNamespace() string
	GetLeaderElectionID() string

	GetLeaseDuration() *time.Duration
	GetRenewDeadline() *time.Duration
	GetRetryPeriod() *time.Duration

	GetNamespace() string
	GetMetricsBindAddress() string
	GetHealthProbeBindAddress() string

	GetReadinessEndpointName() string
	GetLivenessEndpointName() string

	GetPort() int
	GetHost() string

	GetCertDir() string
}

func NewFromComponentConfig(config *rest.Config, managerconfig ManagerConfiguration) (Manager, error) {
        options := Options{}
        // loop though getters to set Options params
        if managerconfig.GetLeaderElection() {
                managerconfig.GetLeaderElection()
        }

	return New(config, options)
}

From here we could make a cli flag on init or a new subcommand of kubebuilder that would set up the initial scaffold of a ComponentConfig object that supports these getters and the parsing of the files similar to what @stealthybox is doing here - https://github.com/kubernetes-sigs/cluster-addons/blob/master/installer/cmd/installer/config.go

The idea in kubebuilder would look something like:

package config

import (
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
) 
type ControllerNameConfiguration struct {
	metav1.TypeMeta

	LeaderElection ControllerNameConfigurationLeaderElection `json:"leaderElection,omitempty"`

	LeaseDuration *time.Duration `json:"leaseDuration,omitempty"`
	RenewDeadline *time.Duration `json:"renewDeadline,omitempty"`
	RetryPeriod *time.Duration`json:"retryPeriod,omitempty"`

	Namespace string `json:"namespace,omitempty"`

	MetricsBindAddress string `json:"metricsBindAddress,omitempty"`

	Health ControllerNameConfigurationHealth

	Port int `json:"port,omitempty"`
	Host string `json:"host,omitempty"`

	CertDir string `json:"certDir,omitempty"`
}

type ControllerNameConfigurationLeaderElection struct {
	Enabled bool `json:"enabled,omitempty"`
	Namespace string `json:"namespace,omitempty"`
	ID string `json:"id,omitempty"`
}

type ControllerNameConfigurationHealth struct {
	HealthProbeBindAddress string `json:"healthProbeBindAddress,omitempty"`

	ReadinessEndpointName string `json:"readinessEndpointName,omitempty"`
	LivenessEndpointName string `json:"livenessEndpointName,omitempty"`
}

Does this make sense? Is this something controller-runtime would be interested in? Happy to put in a PR if so.

@stealthybox
Copy link

stealthybox commented Feb 18, 2020

Thanks @christopherhein

We could make this an interface for which the ComponentConfig stuct would have to adhere too.

I suppose then that the top-level ControllerNameConfiguration type would then implement this interface.
Would you then use the getters and setters everywhere you're accessing the internal Config object?

If I recall and understand you correctly, this interface is a way to prevent breaking changes with the existing config structs.
Are there other advantages of this interface?

@DirectXMan12, is backwards compatibility on the existing flags/config struct for generated controllers important to you?

@christopherhein
Copy link
Member

Yes, @stealthybox, the idea would be we could base scaffold, similar to deepcopy funcs, the entire object getters and setters using the object, they would end up being the kubebuilder implementation.

The main advantage is we would be able to have controller specific configuration structs, controller designers could implement the api of those object how ever they saw fit, but it gives a starting place. Which will natively convert to the internal Options{} type controller-runtime uses.

So for example the coredns-operator could have a CoreDNSOperatorConfiguration object, under its own GV.

@vincepri
Copy link
Member

/kind design

/assign @christopherhein
to open a PR with a design document

@k8s-ci-robot k8s-ci-robot added the kind/design Categorizes issue or PR as related to design. label Feb 20, 2020
@vincepri vincepri added this to the Next milestone Feb 20, 2020
@vincepri vincepri removed the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Feb 20, 2020
@christopherhein
Copy link
Member

christopherhein commented Apr 17, 2020

/close
track against #895

@christopherhein
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/design Categorizes issue or PR as related to design. kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. wg/component-standard Categorizes an issue or PR as relevant to WG Component Standard.
Projects
None yet
Development

No branches or pull requests

7 participants