Emit K8S events to the postgresql CR as feedback to the requestor / user #896

frittentheke · 2020-04-01T21:00:17Z

Currently the only "feedback" the user or creator of a postgresql custom resource receives is the status. While the operator does produce quite a bit of log, this is not suitable to point users to in order for them to find issues with their particular cluster.

This PR shall enable to operator to (also) send events which are promoted by the Kubernetes documentation (https://kubernetes.io/docs/tasks/debug-application-cluster/#example-debugging-pending-pods) to help debugging issues with a resource and to search for messages from controllers handling this resource.

As this shall be a first cut, I might not have added enough events or even have added events that are too noisy. My intention was to report back certain aspects of the life-cycle:

Creation with the individual steps / resources being created
(Warnings in case resource limits are being adjusted)
Sync in case anything goes wrong
Deletion (might take a while or even run into problems - there seems to be not a lot of error handling / reporting there yet)
Switchover or Rolling-Restarts initiated by the Operator

FxKu · 2020-04-15T07:52:49Z

Thanks for this interesting PR. There was already an internal request to extend the status, but events would also improve the user experience, I guess.

So far it doesn't work properly for me. Getting this error:
Could not construct reference to: '&v1.Postgresql{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""},

Could you also update the clusterrole here.

frittentheke · 2020-04-16T11:41:14Z

Thanks for this interesting PR. There was already an internal request to extend the status, but events would also improve the user experience, I guess.

So far it doesn't work properly for me. Getting this error:
Could not construct reference to: '&v1.Postgresql{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""},

@FxKu In general this works for me, but with the Postgresql reference passed along the various functions quite a lot there might be a call to the event function that does not get the input it expects...

Since events are logged (by the operator) along with the other logs, can you narrow this down to a certain action ?

Could you also update the clusterrole here.

Done. I melted this into last commit.

FxKu · 2020-04-17T10:27:55Z

it's for all the events 😃

at the end of each error output I see also:
no kind is registered for the type v1.Postgresql in scheme "k8s.io/client-go/kubernetes/scheme/register.go:69"'

Btw: Unit tests in pkg/cluster/cluster_test.go and pkg/cluster/k8sres_test.go are failinng because the event recorder is missing in the constructor

frittentheke · 2020-04-17T12:17:28Z

it's for all the events smiley

at the end of each error output I see also:
no kind is registered for the type v1.Postgresql in scheme "k8s.io/client-go/kubernetes/scheme/register.go:69"'

Btw: Unit tests in pkg/cluster/cluster_test.go and pkg/cluster/k8sres_test.go are failinng because the event recorder is missing in the constructor

@FxKu Urgh sorry about that, I might have missed /messed-up something when I cherry-picked some commits from my local dev branch to this PR - I'll look into it.

…o cluster via its constructor to enable it to emit events this way

…ce the user interacts with to provide some feedback

…NewFakeRecorder)

frittentheke · 2020-04-19T19:30:49Z

@FxKu

I did push a new commit to may branch that should fix the unit tests, sorry about missing on that initially. I also moved to the current HEAD of master.
I then just built the operator locally and ran it via minikube (using the provided ./run_operator_locally.sh script) and I received events just fine:

  Normal  Update      2m24s  postgres-operator  Performing rolling update
  Normal  Switchover  118s   postgres-operator  Switching over from "acid-minimal-cluster-0" to "default/acid-minimal-cluster-1"
  Normal  Switchover  117s   postgres-operator  Successfully switched over from "acid-minimal-cluster-0" to "default/acid-minimal-cluster-1"
  Normal  Switchover  117s   postgres-operator  Switchover from "acid-minimal-cluster-0" to "default/acid-minimal-cluster-1" FAILED: <nil>
  Normal  Update      99s    postgres-operator  Rolling update done - pods have been recreated

As I said, I expect that there might be more cases that events should be generated for (to properly allow users to follow the life-cycle of their clusters and there might still be bugs). But you not seeing any events at all seems strange.

FxKu · 2020-04-20T08:37:13Z

Thanks for the update. It's interesting: When updating my operator deployment I can see events for running clusters. But when I create a new cluster I get these Could not construct reference to ... errors for everything - add, update and delete events - until the first sync I guess. Seems that new clusters are not registered correctly. Hmm...

Operator logs when it's working (maybe that helps):
level=debug msg="Event(v1.ObjectReference{Kind:\"postgresql\", Namespace:\"default\", Name:\"acid-minimal-cluster\", UID:\"2dce694f-2f83-45a4-bfc2-8409280917b2\", APIVersion:\"acid.zalan.do/v1\", ResourceVersion:\"1754\", FieldPath:\"\"}): type: 'Normal' reason: 'Update' Performing rolling update"

frittentheke · 2020-04-23T07:03:04Z

@FxKu yeah I observe the same errors now. Very strange that it complains about

no kind is registered for the type v1.Postgresql in scheme "k8s.io/client-go/kubernetes/scheme/register.go:69

for new clusters.

frittentheke · 2020-04-23T12:58:33Z

@FxKu apparently one has to get a proper reference to use when sending events.
Why and how this worked for some of those events - I don't know.

I added this with commit 633e001 now ... and heureka, we have many many more events:

[...]
      superuser
      createdb
  Volume:
    Size:  1Gi
Status:
  Postgres Cluster Status:  Running
Events:
  Type    Reason       Age    From               Message
  ----    ------       ----   ----               -------
  Normal  Create       4m16s  postgres-operator  Started creation of new cluster resources
  Normal  Endpoints    4m16s  postgres-operator  Endpoint "default/acid-minimal-cluster" has been successfully created
  Normal  Services     4m16s  postgres-operator  The service "default/acid-minimal-cluster" for role master has been successfully created
  Normal  Services     4m16s  postgres-operator  The service "default/acid-minimal-cluster-repl" for role replica has been successfully created
  Normal  Secrets      4m14s  postgres-operator  The secrets have been successfully created
  Normal  StatefulSet  4m14s  postgres-operator  Statefulset "default/acid-minimal-cluster" has been successfully created
  Normal  StatefulSet  3m26s  postgres-operator  Pods are ready

FxKu · 2020-04-24T13:01:24Z

Tested it and it works now. Very nice new feature. Thanks a lot @frittentheke

FxKu · 2020-04-24T13:01:42Z

👍

sdudoladov · 2020-04-24T14:36:34Z

👍

mwpeterson · 2020-09-09T13:31:07Z

pkg/cluster/cluster.go

@@ -1136,6 +1163,7 @@ func (c *Cluster) Switchover(curMaster *v1.Pod, candidate spec.NamespacedName) e
 	// close the label waiting channel no sooner than the waiting goroutine terminates.
 	close(podLabelErr)

+	c.eventRecorder.Eventf(c.GetReference(), v1.EventTypeNormal, "Switchover", "Switchover from %q to %q FAILED: %v", curMaster.Name, candidate, err)


It seems this should be wrapped in if err != nil {}
FAILED: <nil> is technically correct, but unexpected when the switchover was successful

Opened issue #1134

frittentheke requested review from avaczi, CyberDem0n, erthalion, FxKu, Jan-M, RafiaSabih and sdudoladov as code owners April 1, 2020 21:00

frittentheke changed the title ~~Start sending K8S events as feedback to the postgresql CR~~ Emit K8S events to the postgresql CR as feedback to the requestor / user Apr 2, 2020

frittentheke force-pushed the events branch from 69c46ba to 3159c97 Compare April 16, 2020 11:37

frittentheke added 5 commits April 19, 2020 21:05

Add EventsGetter to KubeClient to enable to sending K8S events

c492042

Add eventRecorder to the controller, initialize it and hand it down t…

5860185

…o cluster via its constructor to enable it to emit events this way

Add first set of events which then go to the postgresql custom resour…

7df13f7

…ce the user interacts with to provide some feedback

Add right to "create" events to operator cluster role

97c20de

Adapt cluster tests to new function sigurature with eventRecord (via …

154a58f

…NewFakeRecorder)

frittentheke force-pushed the events branch from 3159c97 to 154a58f Compare April 19, 2020 19:06

FxKu added this to the 1.5 milestone Apr 20, 2020

Get a proper reference before sending events to a resource

633e001

FxKu merged commit 21b9b6f into zalando:master Apr 27, 2020

mwpeterson reviewed Sep 9, 2020

View reviewed changes

frittentheke deleted the events branch October 1, 2020 23:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emit K8S events to the postgresql CR as feedback to the requestor / user #896

Emit K8S events to the postgresql CR as feedback to the requestor / user #896

frittentheke commented Apr 1, 2020

FxKu commented Apr 15, 2020

frittentheke commented Apr 16, 2020

FxKu commented Apr 17, 2020 •

edited

frittentheke commented Apr 17, 2020

frittentheke commented Apr 19, 2020

FxKu commented Apr 20, 2020 •

edited

frittentheke commented Apr 23, 2020

frittentheke commented Apr 23, 2020

FxKu commented Apr 24, 2020 •

edited

FxKu commented Apr 24, 2020 •

edited

sdudoladov commented Apr 24, 2020

mwpeterson Sep 9, 2020

mwpeterson Sep 9, 2020

Emit K8S events to the postgresql CR as feedback to the requestor / user #896

Emit K8S events to the postgresql CR as feedback to the requestor / user #896

Conversation

frittentheke commented Apr 1, 2020

FxKu commented Apr 15, 2020

frittentheke commented Apr 16, 2020

FxKu commented Apr 17, 2020 • edited

frittentheke commented Apr 17, 2020

frittentheke commented Apr 19, 2020

FxKu commented Apr 20, 2020 • edited

frittentheke commented Apr 23, 2020

frittentheke commented Apr 23, 2020

FxKu commented Apr 24, 2020 • edited

FxKu commented Apr 24, 2020 • edited

sdudoladov commented Apr 24, 2020

mwpeterson Sep 9, 2020

Choose a reason for hiding this comment

mwpeterson Sep 9, 2020

Choose a reason for hiding this comment

FxKu commented Apr 17, 2020 •

edited

FxKu commented Apr 20, 2020 •

edited

FxKu commented Apr 24, 2020 •

edited

FxKu commented Apr 24, 2020 •

edited