Always try "stanza-upgrade" after a failed "stanza-create"

CrunchyData / postgres-operator

Production PostgreSQL for Kubernetes, from high availability Postgres clusters to full-scale database-as-a-service.

https://access.crunchydata.com/documentation/postgres-operator/v5/

Apache License 2.0

3.96k stars 593 forks source link

Always try "stanza-upgrade" after a failed "stanza-create" #3994

Closed tjmoore4 closed 2 months ago

tjmoore4 commented 2 months ago

Checklist:

[x] Have you added an explanation of what your changes do and why you'd like them to be included?
[ ] Have you updated or added documentation for the change, as applicable?
[x] Have you tested your changes on all related environments with successful results, as applicable?
- [ ] Have you added automated tests?

Type of Changes:

[ ] New feature
[x] Bug fix
[ ] Documentation
[ ] Testing enhancement
[ ] Other

What is the new behavior (if this is a feature change)?

[ ] Breaking change (fix or feature that would cause existing functionality to change) Error messages could be on either stderr or stdout depending on logging options. Error messages and exit codes could change unexpectedly, so use a shell list to run "stanza-upgrade" any time "stanza-create" exits non-zero.

Other Information: Issue: PGO-1558

benjaminjb commented 2 months ago

Makes sense, but I wonder if it would be worthwhile to capture the errors and return that to the user (which would take some flexibility since, as you point out, an error might be on stderr or stdout)

tjmoore4 commented 2 months ago

Makes sense, but I wonder if it would be worthwhile to capture the errors and return that to the user (which would take some flexibility since, as you point out, an error might be on stderr or stdout)

This should keep our error behavior the same overall. The main impact is that we try an stanza upgrade immediately after a stanza create fails rather than depending on a particular error to trigger the upgrade. The existing code would similarly not print the initial create error without first trying an upgrade.

benjaminjb commented 2 months ago

This should keep our error behavior the same overall. The main impact is that we try an stanza upgrade immediately after a stanza create fails rather than depending on a particular error to trigger the upgrade. The existing code would similarly not print the initial create error without first trying an upgrade.

Yeah, I guess I'm wondering if we get an error on create and then get an error on upgrade, we will only ever see the upgrade error, right? Not a blocker for me, I was just wondering.

tjmoore4 commented 2 months ago

This should keep our error behavior the same overall. The main impact is that we try an stanza upgrade immediately after a stanza create fails rather than depending on a particular error to trigger the upgrade. The existing code would similarly not print the initial create error without first trying an upgrade.

Yeah, I guess I'm wondering if we get an error on create and then get an error on upgrade, we will only ever see the upgrade error, right? Not a blocker for me, I was just wondering.

Yeah, that's my understanding. The approach is effectively, if the create error is solved by calling upgrade instead, don't bother the user. Definitely an argument to be made either way, but I think this approach is reasonable.