openstack-k8s-operators / dev-docs

Documentation
9 stars 25 forks source link

Document ObservedGeneration usage #102

Closed gibizer closed 5 months ago

dprince commented 6 months ago

Do we really need to document observedGeneration. It seems like most of the docs here are actually related to checking for Readyness. The pattern itself is pretty well known and the comments in the CRD should be mostly self explanatory right?

dprince commented 6 months ago

Upstream k8s only had this documented for observedGeneration: Some resources report the observedGeneration, which is the generation most recently observed by the component responsible for acting upon changes to the desired state of the resource. This can be used, for instance, to ensure that the reported status reflects the most recent desired status.

https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#spec-and-status

gibizer commented 6 months ago

Do we really need to document observedGeneration. It seems like most of the docs here are actually related to checking for Readyness. The pattern itself is pretty well known and the comments in the CRD should be mostly self explanatory right?

I think we are still not fully on the same page with @fmount so I don't think that the pattern is self explanatory.

fmount commented 6 months ago

Do we really need to document observedGeneration. It seems like most of the docs here are actually related to checking for Readyness. The pattern itself is pretty well known and the comments in the CRD should be mostly self explanatory right?

I think we are still not fully on the same page with @fmount so I don't think that the pattern is self explanatory.

I think we can avoid documenting this pattern as long as we agree that we should watch at that parameter when computing the readiness of the top-level CR. Implementation details would be out of scope I suppose here (especially because we need to go through the operators and evaluate case by case).

fmount commented 6 months ago

In general I think we're on the same page on the topic, we agree that there are cases where this pattern should be extended to the sub-level CRs and the top-level should compute its readiness according to the last version of the subCRs.

booxter commented 6 months ago

/lgtm

fmount commented 5 months ago

@gibizer @mrkisaolamb @Akrog @dprince I think we might want to use == in the Generation / ObservedGeneration comparison (and eventually explicitly handle the < case, where something wrong happened), so I think we can merge this patch if there are no additional concerns.