Kibana: Set status.ObservedGeneration from metadata.Generation #5409
Conversation
naemono
force-pushed
the
3392-kibana-observedGeneration
branch
from
caf4843
to
783a536
Compare
|
run full pr build |
1 similar comment
|
run full pr build |
pebrc
changed the title
| } | ||
|
|
||
| // RetrieveAgentGenerationsStep stores the current metadata.Generation, and status.ObservedGeneration into the given fields. | ||
| func RetrieveAgentGenerationsStep(obj commonv1.HasObservedGeneration, k *test.K8sClient, generation, observedGeneration *int64) test.Step { |
There was a problem hiding this comment.
I would expect these also to be used for the Elasticsearch tests but I think you based this branch of a version of main where the observedGeneration code for Elasticsearch does not exist yet. Worth merging main and using the same code for both?
There was a problem hiding this comment.
I removed agent from this in this PR. Let me see about merging rebasing/merging main and fixing this in ES e2e tests also.
There was a problem hiding this comment.
Rebased main, and updated Elasticsearch e2e tests to use shared/common generation package.
Update observedGeneration description. Defer status updates to kibana to ensure status gets updated regardless of reconcilation outcome. Add common generation steps for e2e tests to use.
add nolint:thelper to test because of validation func
Use named errors in doReconcile allowing deferred function to modify return values. Update logic in NewState to not double copy original Kibana object. Modify tests to check the error message returned from Reconcile. Add a failing k8s client to unit test the deferred functional modifying the return values.
Remove agent from generic generation package in e2e tests.
naemono
force-pushed
the
3392-kibana-observedGeneration
branch
from
101f741
to
7836cc1
Compare
|
run full pr build |
| // or more contributor license agreements. Licensed under the Elastic License 2.0; | ||
| // you may not use this file except in compliance with the Elastic License 2.0. | ||
|
|
||
| package generation |
There was a problem hiding this comment.
Nit I would probably have put this into common_steps.go
There was a problem hiding this comment.
looking at the surrounding files, and their names, I went with steps_common.go. lmk if that's acceptable.
There was a problem hiding this comment.
I my comment was not explicit enough. I meant keeping this in the test package in the already existing file by that name. But either way is OK.
|
run full pr build |
…ic#5409) * Add Kibana observedGeneration. Update observedGeneration description. Defer status updates to kibana to ensure status gets updated regardless of reconcilation outcome. Add common generation steps for e2e tests to use. * remove unnecessary args struct add nolint:thelper to test because of validation func * Add new interface for retrieving of observed generation. Use named errors in doReconcile allowing deferred function to modify return values. Update logic in NewState to not double copy original Kibana object. Modify tests to check the error message returned from Reconcile. Add a failing k8s client to unit test the deferred functional modifying the return values. * Simplify Kibana new state logic. Remove agent from generic generation package in e2e tests. * Make Elasticsearch Generation e2e tests use common generation package. * make receiver name consistent. * Adjust wording in test name * Add spacing to api docs * rename common generation filename
This PR is Kibana specific.
related to #3392
related to #5331
This change allows status.ObservedGeneration to be kept in sync with metadata.Generation according to the rules described in the above issue. This change adds a set of unit tests which show more details into when observedGeneration will, and will not be updated according to any errors that may occur during reconciliation. Steps were added to any kibana mutating e2e tests to show how the change is intended to function in a true cluster.