assets: add creater based on dynamic client

[mfojtik]

This PR add pkg/assets/create package that contain helper to create multiple resources from disk and wait for them to be created.

The rationale here is to move this create and wait logic away from cluster-bootstrap (former bootkube) and make it available for multiple components as needed.

[mfojtik]

/cc @sttts
/cc @deads2k

[mfojtik]

TODO: need unit test

/hold

[mfojtik]

/cc @wking

[wking]

Half a second is not a big deal, but if we are worried about flooding the API server it seems like we'd want to sleep between API requests, and not just between manifests iterations. That would be dropping PollImmediateUntil with a raw loop:

while len(manifests) > 0 {
  err, refresh := create(ctx, manifests, client, mapper, options)
  ...
}

possibly with a sleep inside create's loop. If you have any manifests at all, it's hard to see the iteration over those manifests completing in less than half a second.

[mfojtik]

I would be concerned with flooding API server, it should handle creating few resources just fine (right @sttts ?)

I would like to keep wait.PollImmediateUntil because it support context and timeouts. If we wait after each create call, calculating timeout will be more complex (that we need).

[wking]

I would like to keep wait.PollImmediateUntil because it support context and timeouts. If we wait after each create call, calculating timeout will be more complex (that we need).

I've pushed git://github.com/wking/openshift-library-go.git mid-create-cancel 8e509de with a stab at dropping PollImmediateUntil and moving the wait to per-manifest instead of per-create call. If you're comfortable without a per-manifest rate-limit, I'd recommend just dropping the ticker and looping over manifests as fast as we get responses from the server. Thoughts?

[wking]

I've pushed git://github.com/wking/openshift-library-go.git mid-create-cancel 8e509de...

Rebased onto your 0069198 with 8e509de -> fd93f4f.

[mfojtik]

@wking the client itself can be configured to do QPS and rate limiting:

library-go/vendor/k8s.io/client-go/rest/config.go

Line 106 in e0f6273

// QPS indicates the maximum QPS to the master from this client.

If we are concerned about DOSing the API server, we can set this up in the *rest.Config the caller pass to this helper? I can drop the ticker to 10ms or so just to avoid hot-loop, but each individual create call can be limited by using these parameters IMHO.

@sttts @deads2k agree?

[sttts]

Does the kubeconfig have a QPS setting as well? I don't think this belongs hardcoded here, but should be settable from the outside. If the kubeconfig knows qps, there is nothing to do.

[soltysh]

I don't think kubeconfig has that option, we always default qps to 5 in https://github.com/kubernetes/kubernetes/blob/4992402fa5bee9f7ac6fe931d101d362a4295cea/staging/src/k8s.io/client-go/rest/config.go#L276-L278

[mfojtik]

@soltysh @sttts we pass rest.Config that has that option and the caller can decide how he want to throttle the qps (or even replace qps with ratelimiter if he wants).

[sttts]

/lgtm
/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mfojtik, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [sttts]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[wking]

Seems like Verbose and StdErr should be replaced with a slot for a logrus logger or generic logging interface. I can file a follow-up for that.

[wking]

I can file a follow-up for that.

Filed as #225.

[wking]

Still not clear to me why we need more sleeping on top of restConfig's limits.

[wking]

Still not clear to me why we need more sleeping on top of restConfig's limits.

I've filed #226 dropping this additional sleep.