Support status subresource and ObservedGeneration.

[enisoc]

Now that we have support for /status in both CRD and the dynamic client, we should use /status whenever it's available.

As of k8s 1.10, /status for CRD is behind an alpha feature gate, and is also optional on a per-CRD basis. If we neglect to use /status when it's available, our status update will be silently ignored. If we try to use /status when it's not available, our status update will fail. We should be able to tell whether /status is available for a given resource by looking at discovery, without requiring the user to configure anything on the Metacontroller side (they'll just enable /status in the CRD itself).

We should also set status.observedGeneration now that CRD is capable of incrementing metadata.generation (if the /status subresource is enabled for that CRD).

https://github.com/kstmp/metacontroller/blob/8c2b6730d8db89c5eba31bf180ebd9fbe93b3a5c/controller/composite/controller.go#L256-L271

Lastly, for controllers that support ObservedGeneration, we can consider an optimization to avoid syncing if the controller object is updated, but the spec hasn't changed (ObservedGeneration = Generation). For example, we currently might trigger a sync as a result of updating our own status.

[liyinan926]

Lastly, for controllers that support ObservedGeneration, we can consider an optimization to avoid syncing if the controller object is updated, but the spec hasn't changed (ObservedGeneration = Generation). For example, we currently might trigger a sync as a result of updating our own status.

What about comparing the ResourceVersion of the old and new objects to detect spec changes?

[enisoc]

The ResourceVersion changes if anything in the object is updated, including metadata and status. Generation only changes if spec is updated; it's essentially a ResourceVersion scoped just to spec.

[enisoc]

Another requirement for the design of this feature came up while talking to @danisla, and it applies both to CRDs and built-in resources that have /status subresources (like ReplicaSet).

If the hook response tries to set something in .status of a child object, we need to do one of two things if we know from API discovery info that the child resource supports /status:

1. If we choose to ignore the changes in .status, since updates there will be ignored on the main resource path anyway, we need to make sure ApplyUpdate() also ignores everything under .status (by reverting it after merging in the latest update). Otherwise, we will think we need to do an update, but the update will have no effect.

2. Or, if we choose to try to make the changes in .status actually work, we need to update them separately with UpdateStatus() instead of Update().

Historically, there was not much point to choosing option 2 because updating status on your child objects was considered an anti-pattern, as your changes would generally be clobbered by the child object's own controller anyway. However, there are now new patterns developing, such as Pod Ready++, where it does make sense to update the status of a child object.

[crimsonfaith91]

@enisoc Thanks for the summary. For resolving this issue, I prefer option 1 (Pod Ready++ is not GA yet).