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 2 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 pkg/cvo/cvo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func New(
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
24 changes: 16 additions & 8 deletions pkg/start/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,21 +231,24 @@ type ClientBuilder struct {
config *rest.Config
}

func (cb *ClientBuilder) RestConfig() *rest.Config {
func (cb *ClientBuilder) RestConfig(fns ...func(*rest.Config)) *rest.Config {

@abhinavdahiya abhinavdahiya Jan 18, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$ godoc ./pkg/start
PACKAGE DOCUMENTATION

package start
    import "./pkg/start"


TYPES

type ClientBuilder struct {
    // contains filtered or unexported fields
}


func (cb *ClientBuilder) ClientOrDie(name string, fns ...func(*rest.Config)) clientset.Interface

func (cb *ClientBuilder) KubeClientOrDie(name string, fns ...func(*rest.Config)) kubernetes.Interface

func (cb *ClientBuilder) RestConfig(fns ...func(*rest.Config)) *rest.Config

fns ... isn't the best godoc, maybe either add comments / name the variable that it is evident that these are config modifiers...

@smarterclayton smarterclayton Jan 18, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh, sorry

c := rest.CopyConfig(cb.config)
for _, fn := range fns {
fn(c)
}
return c
}

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

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

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

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

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

type Context struct {
CVO *cvo.Operator
AutoUpdate *autoupdate.Controller
Expand Down Expand Up @@ -291,7 +299,7 @@ func (o *Options) NewControllerContext(cb *ClientBuilder) *Context {
resyncPeriod(o.ResyncInterval)(),
cvInformer.Config().V1().ClusterVersions(),
sharedInformers.Config().V1().ClusterOperators(),
cb.RestConfig(),
cb.RestConfig(increaseQPS),

@abhinavdahiya abhinavdahiya Jan 18, 2019

Copy link
Copy Markdown
Contributor

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 ?

@smarterclayton smarterclayton Jan 18, 2019

Copy link
Copy Markdown
Contributor Author

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),
Expand Down