Skip to content
This repository was archived by the owner on Jul 22, 2022. It is now read-only.

Commit

Permalink
Add per-object, one-time resync.
Browse files Browse the repository at this point in the history
This adds a `resyncAfterSeconds` field (can be a decimal) to the `sync`
hook response of CompositeController and DecoratorController.
This field allows the hook response to request a resync of one
particular object after a specified delay, for example if the hook knows
it wants to recheck some external state for just that object.
As a result, the controller-wide resyncPeriodSeconds can be kept at a
larger interval while being responsive when external state is more
likely to change quickly for certain objects.
  • Loading branch information
enisoc committed Mar 6, 2019
1 parent 2999529 commit a1d2a2f
Show file tree
Hide file tree
Showing 12 changed files with 293 additions and 61 deletions.
11 changes: 11 additions & 0 deletions controller/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,17 @@ func (m ChildMap) ReplaceChild(parent metav1.Object, child *unstructured.Unstruc
}
}

// List expands the ChildMap into a flat list of child objects, in random order.
func (m ChildMap) List() []*unstructured.Unstructured {
var list []*unstructured.Unstructured
for _, group := range m {
for _, obj := range group {
list = append(list, obj)
}
}
return list
}

// MakeChildMap builds the map of children resources that is suitable for use
// in the `children` field of a CompositeController SyncRequest or
// `attachments` field of the DecoratorControllers SyncRequest.
Expand Down
21 changes: 18 additions & 3 deletions controller/composite/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,15 @@ func (pc *parentController) enqueueParentObject(obj interface{}) {
pc.queue.Add(key)
}

func (pc *parentController) enqueueParentObjectAfter(obj interface{}, delay time.Duration) {
key, err := common.KeyFunc(obj)
if err != nil {
utilruntime.HandleError(fmt.Errorf("couldn't get key for object %+v: %v", obj, err))
return
}
pc.queue.AddAfter(key, delay)
}

func (pc *parentController) updateParentObject(old, cur interface{}) {
// We used to ignore our own status updates, but we don't anymore.
// It's sometimes necessary for a hook to see its own status updates
Expand Down Expand Up @@ -427,14 +436,20 @@ func (pc *parentController) syncParentObject(parent *unstructured.Unstructured)
// Reconcile ControllerRevisions belonging to this parent.
// Call the sync hook for each revision, then compute the overall status and
// desired children, accounting for any rollout in progress.
parentStatus, desiredChildren, finalized, err := pc.syncRevisions(parent, observedChildren)
syncResult, err := pc.syncRevisions(parent, observedChildren)
if err != nil {
return err
}
desiredChildren := common.MakeChildMap(parent, syncResult.Children)

// Enqueue a delayed resync, if requested.
if syncResult.ResyncAfterSeconds > 0 {
pc.enqueueParentObjectAfter(parent, time.Duration(syncResult.ResyncAfterSeconds*float64(time.Second)))
}

// If all revisions agree that they've finished finalizing,
// remove our finalizer.
if finalized {
if syncResult.Finalized {
updatedParent, err := pc.parentClient.Namespace(parent.GetNamespace()).RemoveFinalizer(parent, pc.finalizer.Name)
if err != nil {
return fmt.Errorf("can't remove finalizer for %v %v/%v: %v", parent.GetKind(), parent.GetNamespace(), parent.GetName(), err)
Expand Down Expand Up @@ -489,7 +504,7 @@ func (pc *parentController) syncParentObject(parent *unstructured.Unstructured)

// Update parent status.
// We'll want to make sure this happens after manageChildren once we support observedGeneration.
if _, err := pc.updateParentStatus(parent, parentStatus); err != nil {
if _, err := pc.updateParentStatus(parent, syncResult.Status); err != nil {
return fmt.Errorf("can't update status for %v %v/%v: %v", pc.parentResource.Kind, parent.GetNamespace(), parent.GetName(), err)
}

Expand Down
58 changes: 35 additions & 23 deletions controller/composite/controller_revision.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (pc *parentController) claimRevisions(parent *unstructured.Unstructured) ([
return revisions, nil
}

func (pc *parentController) syncRevisions(parent *unstructured.Unstructured, observedChildren common.ChildMap) (parentStatus map[string]interface{}, desiredChildren common.ChildMap, finalized bool, err error) {
func (pc *parentController) syncRevisions(parent *unstructured.Unstructured, observedChildren common.ChildMap) (*SyncHookResponse, error) {
// If no child resources use rolling updates, just sync the latest parent.
// Also, if the parent object is being deleted and we don't have a finalizer,
// just sync the latest parent to get the status since we won't manage
Expand All @@ -87,15 +87,15 @@ func (pc *parentController) syncRevisions(parent *unstructured.Unstructured, obs
}
syncResult, err := callSyncHook(pc.cc, syncRequest)
if err != nil {
return nil, nil, false, fmt.Errorf("sync hook failed for %v %v/%v: %v", pc.parentResource.Kind, parent.GetNamespace(), parent.GetName(), err)
return nil, fmt.Errorf("sync hook failed for %v %v/%v: %v", pc.parentResource.Kind, parent.GetNamespace(), parent.GetName(), err)
}
return syncResult.Status, common.MakeChildMap(parent, syncResult.Children), syncResult.Finalized, nil
return syncResult, nil
}

// Claim all matching ControllerRevisions for the parent.
observedRevisions, err := pc.claimRevisions(parent)
if err != nil {
return nil, nil, false, err
return nil, err
}

// Extract the fields from parent that the controller author
Expand All @@ -121,7 +121,7 @@ func (pc *parentController) syncRevisions(parent *unstructured.Unstructured, obs
for _, revision := range observedRevisions {
patch := make(map[string]interface{})
if err := json.Unmarshal(revision.ParentPatch.Raw, &patch); err != nil {
return nil, nil, false, fmt.Errorf("can't unmarshal ControllerRevision parentPatch: %v", err)
return nil, fmt.Errorf("can't unmarshal ControllerRevision parentPatch: %v", err)
}
if reflect.DeepEqual(patch, latestPatch) {
// This ControllerRevision matches the latest parent state.
Expand All @@ -138,7 +138,7 @@ func (pc *parentController) syncRevisions(parent *unstructured.Unstructured, obs
if latest.revision == nil {
revision, err := newControllerRevision(&pc.parentResource.APIResource, latest.parent, latestPatch)
if err != nil {
return nil, nil, false, err
return nil, err
}
latest.revision = revision
}
Expand All @@ -160,24 +160,22 @@ func (pc *parentController) syncRevisions(parent *unstructured.Unstructured, obs
pr.syncError = err
return
}
pr.status = syncResult.Status
pr.desiredChildList = syncResult.Children
pr.syncResult = syncResult
pr.desiredChildMap = common.MakeChildMap(parent, syncResult.Children)
pr.finalized = syncResult.Finalized
}(pr)
}
wg.Wait()

// If any of the sync calls failed, abort.
for _, pr := range parentRevisions {
if pr.syncError != nil {
return nil, nil, false, fmt.Errorf("sync hook failed for %v %v/%v: %v", pc.parentResource.Kind, parent.GetNamespace(), parent.GetName(), pr.syncError)
return nil, fmt.Errorf("sync hook failed for %v %v/%v: %v", pc.parentResource.Kind, parent.GetNamespace(), parent.GetName(), pr.syncError)
}
}

// Manipulate revisions to proceed with any ongoing rollout, if possible.
if err := pc.syncRollingUpdate(parentRevisions, observedChildren); err != nil {
return nil, nil, false, err
return nil, err
}

// Remove any ControllerRevisions that no longer have any children.
Expand All @@ -197,13 +195,13 @@ func (pc *parentController) syncRevisions(parent *unstructured.Unstructured, obs
}
}
if err := pc.manageRevisions(parent, observedRevisions, desiredRevisions); err != nil {
return nil, nil, false, fmt.Errorf("%v %v/%v: can't reconcile ControllerRevisions: %v", pc.parentResource.Kind, parent.GetNamespace(), parent.GetName(), err)
return nil, fmt.Errorf("%v %v/%v: can't reconcile ControllerRevisions: %v", pc.parentResource.Kind, parent.GetNamespace(), parent.GetName(), err)
}

// We now know which revision ought to be responsible for which children.
// Start with the latest revision's desired children.
// Then overwrite any children that are still claimed by other revisions.
desiredChildren = latest.desiredChildMap
desiredChildren := latest.desiredChildMap
for _, pr := range parentRevisions[1:] {
for _, ck := range pr.revision.Children {
for _, name := range ck.Names {
Expand All @@ -215,17 +213,32 @@ func (pc *parentController) syncRevisions(parent *unstructured.Unstructured, obs
}
}

// Build a single, aggregated syncResult.
// We only take parent status from the latest revision.
syncResult := &SyncHookResponse{
Status: latest.syncResult.Status,
Children: desiredChildren.List(),
}

// Aggregate `resyncAfterSeconds` from all revisions.
// The smallest positive value wins.
for _, pr := range parentRevisions {
if resync := pr.syncResult.ResyncAfterSeconds; resync > 0 &&
(syncResult.ResyncAfterSeconds == 0 || resync < syncResult.ResyncAfterSeconds) {
syncResult.ResyncAfterSeconds = resync
}
}

// Aggregate `finalized` from all revisions. We're finalized if all agree.
finalized = true
syncResult.Finalized = true
for _, pr := range parentRevisions {
if !pr.finalized {
finalized = false
if !pr.syncResult.Finalized {
syncResult.Finalized = false
break
}
}

// We only take parent status from the latest revision.
return latest.status, desiredChildren, finalized, nil
return syncResult, nil
}

func (pc *parentController) manageRevisions(parent *unstructured.Unstructured, observedRevisions, desiredRevisions []*v1alpha1.ControllerRevision) error {
Expand Down Expand Up @@ -365,11 +378,10 @@ type parentRevision struct {
parent *unstructured.Unstructured
revision *v1alpha1.ControllerRevision

status map[string]interface{}
desiredChildList []*unstructured.Unstructured
desiredChildMap common.ChildMap
syncError error
finalized bool
syncResult *SyncHookResponse
syncError error

desiredChildMap common.ChildMap
}

func (pr *parentRevision) countChildren() int {
Expand Down
2 changes: 2 additions & 0 deletions controller/composite/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ type SyncHookResponse struct {
Status map[string]interface{} `json:"status"`
Children []*unstructured.Unstructured `json:"children"`

ResyncAfterSeconds float64 `json:"resyncAfterSeconds"`

// Finalized is only used by the finalize hook.
Finalized bool `json:"finalized"`
}
Expand Down
8 changes: 4 additions & 4 deletions controller/composite/rolling_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (pc *parentController) syncRollingUpdate(parentRevisions []*parentRevision,
// Look for the next child to update, if any.
// We go one by one, in the order in which the controller returned them
// in the latest sync hook result.
for _, child := range latest.desiredChildList {
for _, child := range latest.syncResult.Children {
apiGroup, _ := common.ParseAPIVersion(child.GetAPIVersion())
kind := child.GetKind()
name := child.GetName()
Expand Down Expand Up @@ -115,7 +115,7 @@ func (pc *parentController) syncRollingUpdate(parentRevisions []*parentRevision,
Reason: "RolloutWaiting",
Message: err.Error(),
}
dynamicobject.SetCondition(latest.status, updatedCondition)
dynamicobject.SetCondition(latest.syncResult.Status, updatedCondition)
return nil
}

Expand All @@ -133,7 +133,7 @@ func (pc *parentController) syncRollingUpdate(parentRevisions []*parentRevision,
Reason: "RolloutProgressing",
Message: fmt.Sprintf("updating %v %v", kind, name),
}
dynamicobject.SetCondition(latest.status, updatedCondition)
dynamicobject.SetCondition(latest.syncResult.Status, updatedCondition)
return nil
}
}
Expand All @@ -145,7 +145,7 @@ func (pc *parentController) syncRollingUpdate(parentRevisions []*parentRevision,
Reason: "OnLatestRevision",
Message: fmt.Sprintf("latest ControllerRevision: %v", latest.revision.Name),
}
dynamicobject.SetCondition(latest.status, updatedCondition)
dynamicobject.SetCondition(latest.syncResult.Status, updatedCondition)
return nil
}

Expand Down
18 changes: 15 additions & 3 deletions controller/decorator/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,15 @@ func (c *decoratorController) enqueueParentObject(obj interface{}) {
c.queue.Add(key)
}

func (c *decoratorController) enqueueParentObjectAfter(obj interface{}, delay time.Duration) {
key, err := parentQueueKey(obj)
if err != nil {
utilruntime.HandleError(fmt.Errorf("couldn't get key for object %+v: %v", obj, err))
return
}
c.queue.AddAfter(key, delay)
}

func (c *decoratorController) updateParentObject(old, cur interface{}) {
// TODO(enisoc): Is there any way to avoid resyncing after our own updates?
c.enqueueParentObject(cur)
Expand Down Expand Up @@ -448,8 +457,6 @@ func (c *decoratorController) syncParentObject(parent *unstructured.Unstructured
return err
}

var desiredChildren common.ChildMap

// Call the sync hook to get the desired annotations and children.
syncRequest := &SyncHookRequest{
Controller: c.dc,
Expand All @@ -460,7 +467,12 @@ func (c *decoratorController) syncParentObject(parent *unstructured.Unstructured
if err != nil {
return err
}
desiredChildren = common.MakeChildMap(parent, syncResult.Attachments)
desiredChildren := common.MakeChildMap(parent, syncResult.Attachments)

// Enqueue a delayed resync, if requested.
if syncResult.ResyncAfterSeconds > 0 {
c.enqueueParentObjectAfter(parent, time.Duration(syncResult.ResyncAfterSeconds*float64(time.Second)))
}

// Set desired labels and annotations on parent.
// Also remove finalizer if requested.
Expand Down
2 changes: 2 additions & 0 deletions controller/decorator/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ type SyncHookResponse struct {
Status map[string]interface{} `json:"status"`
Attachments []*unstructured.Unstructured `json:"attachments"`

ResyncAfterSeconds float64 `json:"resyncAfterSeconds"`

// Finalized is only used by the finalize hook.
Finalized bool `json:"finalized"`
}
Expand Down
19 changes: 15 additions & 4 deletions docs/_api/compositecontroller.md
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ The body of your response should be a JSON object with the following fields:
| ----- | ----------- |
| `status` | A JSON object that will completely replace the `status` field within the parent object. |
| `children` | A list of JSON objects representing all the desired children for this parent object. |
| `resyncAfterSeconds` | Set the delay (in seconds, as a float) before an optional, one-time, per-object resync. |

What you put in `status` is up to you, but usually it's best to follow
conventions established by controllers like Deployment.
Expand Down Expand Up @@ -407,6 +408,16 @@ Instead, you should think of each entry in the list of `children` as being
sent to [`kubectl apply`][kubectl apply].
That is, you should [set only the fields that you care about](/api/apply/).

You can optionally set `resyncAfterSeconds` to a value greater than 0 to request
that the `sync` hook be called again with this particular parent object after
some delay (specified in seconds, with decimal fractions allowed).
Unlike the controller-wide [`resyncPeriodSeconds`](#resync-period), this is a
one-time request (not a request to start periodic resyncs), although you can
always return another `resyncAfterSeconds` value from subsequent `sync` calls.
Also unlike the controller-wide setting, this request only applies to the
particular parent object that this `sync` call sent, so you can request
different delays (or omit the request) depending on the state of each object.

Note that your webhook handler must return a response with a status code of `200`
to be considered successful. Metacontroller will wait for a response for up to the
amount defined in the [Webhook spec](/api/hook/#webhook).
Expand Down Expand Up @@ -491,9 +502,9 @@ in the observed state.

If the only thing you're still waiting for is a state change in an external
system, and you don't need to assert any new desired state for your children,
consider returning an error from your `finalize` hook instead of success.
This will cause Metacontroller to requeue and retry the hook later, giving you
returning success from the `finalize` hook may mean that Metacontroller doesn't
call your hook again until the next [periodic resync](#resync-period).
To reduce the delay, you can request a one-time, per-object resync by setting
`resyncAfterSeconds` in your [hook response](#sync-hook-response), giving you
a chance to recheck the external state without holding up a slot in the work
queue.
If you return success, Metacontroller won't call your hook again until the next
[periodic resync](#resync-period).
21 changes: 16 additions & 5 deletions docs/_api/decoratorcontroller.md
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ The body of your response should be a JSON object with the following fields:
| `annotations` | A map of key-value pairs for annotations to set on the target object. |
| `status` | A JSON object that will completely replace the `status` field within the target object. Leave unspecified or `null` to avoid changing `status`. |
| `attachments` | A list of JSON objects representing all the desired attachments for this target object. |
| `resyncAfterSeconds` | Set the delay (in seconds, as a float) before an optional, one-time, per-object resync. |

By convention, the controller for a given resource should not
modify its own spec, so your decorator can't mutate the target's spec.
Expand Down Expand Up @@ -324,6 +325,16 @@ Instead, you should think of each entry in the list of `attachments` as being
sent to [`kubectl apply`][kubectl apply].
That is, you should [set only the fields that you care about](/api/apply/).

You can optionally set `resyncAfterSeconds` to a value greater than 0 to request
that the `sync` hook be called again with this particular parent object after
some delay (specified in seconds, with decimal fractions allowed).
Unlike the controller-wide [`resyncPeriodSeconds`](#resync-period), this is a
one-time request (not a request to start periodic resyncs), although you can
always return another `resyncAfterSeconds` value from subsequent `sync` calls.
Also unlike the controller-wide setting, this request only applies to the
particular parent object that this `sync` call sent, so you can request
different delays (or omit the request) depending on the state of each object.

Note that your webhook handler must return a response with a status code of `200`
to be considered successful. Metacontroller will wait for a response for up to the
amount defined in the [Webhook spec](/api/hook/#webhook).
Expand Down Expand Up @@ -417,10 +428,10 @@ and Metacontroller will call your hook again automatically if anything changes
in the observed state.

If the only thing you're still waiting for is a state change in an external
system, and you don't need to assert any new desired state for your attachments,
consider returning an error from your `finalize` hook instead of success.
This will cause Metacontroller to requeue and retry the hook later, giving you
system, and you don't need to assert any new desired state for your children,
returning success from the `finalize` hook may mean that Metacontroller doesn't
call your hook again until the next [periodic resync](#resync-period).
To reduce the delay, you can request a one-time, per-object resync by setting
`resyncAfterSeconds` in your [hook response](#sync-hook-response), giving you
a chance to recheck the external state without holding up a slot in the work
queue.
If you return success, Metacontroller won't call your hook again until the next
[periodic resync](#resync-period).
Loading

0 comments on commit a1d2a2f

Please sign in to comment.