postgres-operator icon indicating copy to clipboard operation
postgres-operator copied to clipboard

No observedGeneration in Customer Resource status

Open thcdrt opened this issue 2 years ago • 7 comments

Hello community,

Some context

  • Which image of the operator are you using? registry.opensource.zalan.do/acid/postgres-operator:v1.6.3
  • Where do you run it - cloud or metal? Kubernetes or OpenShift? OVH Managed Kubernetes
  • Are you running Postgres Operator in production? no
  • Type of issue? feature request

Issue

When retrieving the Custom Resource pg with the command: kubectl get postgresqls.acid.zalan.do -o=jsonpath='{.items[0].status}'

I get : {"PostgresClusterStatus":"Running"}

I would have expected to have the field observedGeneration like in many Custom Resources / Resources.

I don't know how to check programmatically that an update on the Custom Resource has been processed without observedGeneration mechanism. I mean if for example I modify the version of the Postgres and I only check the PostgresClusterStatus value it could be Running but not on the right Postgres version. The only way I see is using ResourceVersion but it's not really user friendly.

What do you think ?

Thank you, Thomas

thcdrt avatar Nov 19 '21 17:11 thcdrt

It's true, that our status subresource is not following the schema one expects from K8s resources. Our internal K8s teams has also already complained :smile: This could and should def. be improved in the future. PRs are welcome.

FxKu avatar Nov 19 '21 17:11 FxKu

/assign

hzliangbin avatar Dec 06 '21 10:12 hzliangbin

@FxKu hi, I wanna give a try. May I take it ?

hzliangbin avatar Dec 06 '21 10:12 hzliangbin

@hzliangbin yes, please :smiley: You might want to create a PR first with your ideas before going straight into implementation. We need to make sure we stay backwards compatible by keeping the PostgresClusterStatus I think. But the PostgresStatus is a struct which can easily be extended.

FxKu avatar Dec 07 '21 15:12 FxKu

Hello @hzliangbin, did you have time to take a look at it ?

thcdrt avatar Feb 22 '22 21:02 thcdrt

@FxKu @thcdrt really sorry for being late. I think it should be carefully designed which status fields included. or just add a ObservedGeneration is fined?

hzliangbin avatar Mar 10 '22 07:03 hzliangbin

For me, ObservedGeneration field should be enough but we should set it synchronously with PostgresClusterStatus

thcdrt avatar Aug 08 '22 09:08 thcdrt