-
Notifications
You must be signed in to change notification settings - Fork 634
Look in more directories for restore binaries #4310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
// Patroni config when bootstrapping a cluster using an existing data directory. | ||
func RestoreCommand(pgdata, hugePagesSetting, fetchKeyCommand string, _ []*corev1.PersistentVolumeClaim, args ...string) []string { | ||
ps := postgres.NewParameterSet() | ||
func RestoreCommand(postgresVersion int32, pgdata string, params *postgres.ParameterSet, args ...string) []string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
// ensure options are properly set | ||
// TODO (andrewlecuyer): move validation logic to a webhook | ||
for _, opt := range options { | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to check all the flags and emit all the errors at once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to change any more of this behavior right now. 🌱 The comment above suggests CRD validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be ranging over hasOptions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that I know of. What would a range do here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see where I was going wrong: in the original code, we loop over the options, and if one of them is wrong for some reason, we emit an event and return.
(As opposed to, say, telling the user every things that's wrong with their input in one pass.)
Input options were being scanned multiple times in different ways.
This makes restores compatible with images from other distros. Issue: PGO-864
This is similar to #4304, but for restore jobs in PostgresCluster using Postgres images.
Checklist:
Type of Changes:
Other Information:
📝 Might be easier to review as separate commits.
Issue: PGO-864