Skip to content

Increase the QPS limits on clients slightly and use protobuf for sync #90

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 4 commits into from
Jan 18, 2019
Merged

Increase the QPS limits on clients slightly and use protobuf for sync #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
95ed276
cvo: Update the component event source to match the namespace name
smarterclayton Jan 18, 2019
2376468
sync: Increase QPS for the sync client so we can deploy faster
smarterclayton Jan 18, 2019
0a255ab
cvo: Use protobuf for sending events and other basic API commands
smarterclayton Jan 18, 2019
f7fbfb2
doc: Improve godoc on the start package
smarterclayton Jan 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/resourcebuilder/apiext.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type crdBuilder struct {

func newCRDBuilder(config *rest.Config, m lib.Manifest) Interface {
return &crdBuilder{
client: apiextclientv1beta1.NewForConfigOrDie(config),
client: apiextclientv1beta1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/resourcebuilder/apps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type deploymentBuilder struct {

func newDeploymentBuilder(config *rest.Config, m lib.Manifest) Interface {
return &deploymentBuilder{
client: appsclientv1.NewForConfigOrDie(config),
client: appsclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand Down Expand Up @@ -89,7 +89,7 @@ type daemonsetBuilder struct {

func newDaemonsetBuilder(config *rest.Config, m lib.Manifest) Interface {
return &daemonsetBuilder{
client: appsclientv1.NewForConfigOrDie(config),
client: appsclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/resourcebuilder/batch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type jobBuilder struct {

func newJobBuilder(config *rest.Config, m lib.Manifest) Interface {
return &jobBuilder{
client: batchclientv1.NewForConfigOrDie(config),
client: batchclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand Down
8 changes: 4 additions & 4 deletions lib/resourcebuilder/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type serviceAccountBuilder struct {

func newServiceAccountBuilder(config *rest.Config, m lib.Manifest) Interface {
return &serviceAccountBuilder{
client: coreclientv1.NewForConfigOrDie(config),
client: coreclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand All @@ -43,7 +43,7 @@ type configMapBuilder struct {

func newConfigMapBuilder(config *rest.Config, m lib.Manifest) Interface {
return &configMapBuilder{
client: coreclientv1.NewForConfigOrDie(config),
client: coreclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand All @@ -70,7 +70,7 @@ type namespaceBuilder struct {

func newNamespaceBuilder(config *rest.Config, m lib.Manifest) Interface {
return &namespaceBuilder{
client: coreclientv1.NewForConfigOrDie(config),
client: coreclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand All @@ -97,7 +97,7 @@ type serviceBuilder struct {

func newServiceBuilder(config *rest.Config, m lib.Manifest) Interface {
return &serviceBuilder{
client: coreclientv1.NewForConfigOrDie(config),
client: coreclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand Down
11 changes: 11 additions & 0 deletions lib/resourcebuilder/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package resourcebuilder

import "k8s.io/client-go/rest"

// withProtobuf makes a client use protobuf.
func withProtobuf(config *rest.Config) *rest.Config {
config = rest.CopyConfig(config)
config.AcceptContentTypes = "application/vnd.kubernetes.protobuf,application/json"
config.ContentType = "application/vnd.kubernetes.protobuf"
return config
}
8 changes: 4 additions & 4 deletions lib/resourcebuilder/rbac.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type clusterRoleBuilder struct {

func newClusterRoleBuilder(config *rest.Config, m lib.Manifest) Interface {
return &clusterRoleBuilder{
client: rbacclientv1.NewForConfigOrDie(config),
client: rbacclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand All @@ -43,7 +43,7 @@ type clusterRoleBindingBuilder struct {

func newClusterRoleBindingBuilder(config *rest.Config, m lib.Manifest) Interface {
return &clusterRoleBindingBuilder{
client: rbacclientv1.NewForConfigOrDie(config),
client: rbacclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand All @@ -70,7 +70,7 @@ type roleBuilder struct {

func newRoleBuilder(config *rest.Config, m lib.Manifest) Interface {
return &roleBuilder{
client: rbacclientv1.NewForConfigOrDie(config),
client: rbacclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand All @@ -97,7 +97,7 @@ type roleBindingBuilder struct {

func newRoleBindingBuilder(config *rest.Config, m lib.Manifest) Interface {
return &roleBindingBuilder{
client: rbacclientv1.NewForConfigOrDie(config),
client: rbacclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/resourcebuilder/security.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type securityBuilder struct {

func newSecurityBuilder(config *rest.Config, m lib.Manifest) Interface {
return &securityBuilder{
client: securityclientv1.NewForConfigOrDie(config),
client: securityclientv1.NewForConfigOrDie(withProtobuf(config)),
raw: m.Raw,
}
}
Expand Down
6 changes: 1 addition & 5 deletions pkg/cvo/cvo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/golang/glog"
"github.com/google/uuid"
corev1 "k8s.io/api/core/v1"
apiextclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand Down Expand Up @@ -83,7 +82,6 @@ type Operator struct {

client clientset.Interface
kubeClient kubernetes.Interface
apiExtClient apiextclientset.Interface
eventRecorder record.EventRecorder

// minimumUpdateCheckInterval is the minimum duration to check for updates from
Expand Down Expand Up @@ -132,7 +130,6 @@ func New(
restConfig *rest.Config,
client clientset.Interface,
kubeClient kubernetes.Interface,
apiExtClient apiextclientset.Interface,
enableMetrics bool,
) *Operator {
eventBroadcaster := record.NewBroadcaster()
Expand All @@ -153,8 +150,7 @@ func New(
restConfig: restConfig,
client: client,
kubeClient: kubeClient,
apiExtClient: apiExtClient,
eventRecorder: eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "clusterversionoperator"}),
eventRecorder: eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: namespace}),

queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "clusterversion"),
availableUpdatesQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "availableupdates"),
Expand Down
47 changes: 34 additions & 13 deletions pkg/start/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// package start initializes and launches the core cluster version operator
// loops.
package start

import (
Expand All @@ -19,7 +21,6 @@ import (
"github.com/openshift/cluster-version-operator/pkg/cvo"
"github.com/prometheus/client_golang/prometheus/promhttp"
v1 "k8s.io/api/core/v1"
apiext "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/scheme"
Expand All @@ -42,6 +43,7 @@ const (
retryPeriod = 30 * time.Second
)

// Options are the valid inputs to starting the CVO.
type Options struct {
ReleaseImage string

Expand All @@ -67,6 +69,8 @@ func defaultEnv(name, defaultValue string) string {
return env
}

// NewOptions creates the default options for the CVO and loads any environment
// variable overrides.
func NewOptions() *Options {
return &Options{
ListenAddr: "0.0.0.0:9099",
Expand Down Expand Up @@ -227,25 +231,28 @@ func resyncPeriod(minResyncPeriod time.Duration) func() time.Duration {
}
}

// ClientBuilder simplifies returning Kubernetes client and client configs with
// an appropriate user agent.
type ClientBuilder struct {
config *rest.Config
}

func (cb *ClientBuilder) RestConfig() *rest.Config {
// RestConfig returns a copy of the ClientBuilder's rest.Config with any overrides
// from the provided configFns applied.
func (cb *ClientBuilder) RestConfig(configFns ...func(*rest.Config)) *rest.Config {
c := rest.CopyConfig(cb.config)
for _, fn := range configFns {
fn(c)
}
return c
}

func (cb *ClientBuilder) ClientOrDie(name string) clientset.Interface {
return clientset.NewForConfigOrDie(rest.AddUserAgent(cb.config, name))
}

func (cb *ClientBuilder) KubeClientOrDie(name string) kubernetes.Interface {
return kubernetes.NewForConfigOrDie(rest.AddUserAgent(cb.config, name))
func (cb *ClientBuilder) ClientOrDie(name string, configFns ...func(*rest.Config)) clientset.Interface {
return clientset.NewForConfigOrDie(rest.AddUserAgent(cb.RestConfig(configFns...), name))
}

func (cb *ClientBuilder) APIExtClientOrDie(name string) apiext.Interface {
return apiext.NewForConfigOrDie(rest.AddUserAgent(cb.config, name))
func (cb *ClientBuilder) KubeClientOrDie(name string, configFns ...func(*rest.Config)) kubernetes.Interface {
return kubernetes.NewForConfigOrDie(rest.AddUserAgent(cb.RestConfig(configFns...), name))
}

func newClientBuilder(kubeconfig string) (*ClientBuilder, error) {
Expand All @@ -263,6 +270,17 @@ func newClientBuilder(kubeconfig string) (*ClientBuilder, error) {
}, nil
}

func increaseQPS(config *rest.Config) {
config.QPS = 20
config.Burst = 40
}

func useProtobuf(config *rest.Config) {
config.AcceptContentTypes = "application/vnd.kubernetes.protobuf,application/json"
config.ContentType = "application/vnd.kubernetes.protobuf"
}

// Context holds the controllers for this operator and exposes a unified start command.
type Context struct {
CVO *cvo.Operator
AutoUpdate *autoupdate.Controller
Expand All @@ -271,6 +289,8 @@ type Context struct {
InformerFactory informers.SharedInformerFactory
}

// NewControllerContext initializes the default Context for the current Options. It does
// not start any background processes.
func (o *Options) NewControllerContext(cb *ClientBuilder) *Context {
client := cb.ClientOrDie("shared-informer")

Expand All @@ -291,10 +311,9 @@ func (o *Options) NewControllerContext(cb *ClientBuilder) *Context {
resyncPeriod(o.ResyncInterval)(),
cvInformer.Config().V1().ClusterVersions(),
sharedInformers.Config().V1().ClusterOperators(),
cb.RestConfig(),
cb.RestConfig(increaseQPS),
Copy link
Contributor

@abhinavdahiya abhinavdahiya Jan 18, 2019

Choose a reason for hiding this comment

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

this will only affect the generic resource builders. what about the ClientOrDie, KubeClientOrDie atleast KubeClientOrDie ?

Copy link
Contributor Author

@smarterclayton smarterclayton Jan 18, 2019

Choose a reason for hiding this comment

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

I left those at the defaults, those have low qps and low actions (rest config is what is used for sync loop). I'd like to do a shared qps pool eventually though for everything that isn't status updates to CV (which is effectively what this PR is).

cb.ClientOrDie(o.Namespace),
cb.KubeClientOrDie(o.Namespace),
cb.APIExtClientOrDie(o.Namespace),
cb.KubeClientOrDie(o.Namespace, useProtobuf),
o.EnableMetrics,
),
}
Expand All @@ -310,6 +329,8 @@ func (o *Options) NewControllerContext(cb *ClientBuilder) *Context {
return ctx
}

// Start launches the controllers in the provided context and any supporting
// infrastructure. When ch is closed the controllers will be shut down.
func (ctx *Context) Start(ch <-chan struct{}) {
go ctx.CVO.Run(2, ch)
if ctx.AutoUpdate != nil {
Expand Down