-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
migration: provide a hook to implement preconditions #68242
migration: provide a hook to implement preconditions #68242
Conversation
0df904b
to
8af77c2
Compare
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.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @knz)
pkg/migration/tenant_migration.go, line 103 at r1 (raw file):
ctx context.Context, cv clusterversion.ClusterVersion, d TenantDeps, ) error { if m.precondition == nil {
[nit] I think it would be good to require a non-nil precondition
function, to force whoever is adding a migration to consider this aspect. We can define a NoPreconditionFn
to use when it's not needed.
This change augments the migration infrastructure with an optional hook for tenant migrations to express preconditions that will prevent any attempt to push the cluster version to a version which requires this migration to run. This will be helpful to prevent scenarios where user migrations are doomed to fail based on current state in the database but that would only be determined after the cluster had locked out the older version. Release note: None
8af77c2
to
26eaca0
Compare
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.
Added testing and made the precondition required. PTAL.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @knz)
pkg/migration/tenant_migration.go, line 103 at r1 (raw file):
Previously, RaduBerinde wrote…
[nit] I think it would be good to require a non-nil
precondition
function, to force whoever is adding a migration to consider this aspect. We can define aNoPreconditionFn
to use when it's not needed.
Done.
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.
Reviewed 8 of 8 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @knz)
TFTR! bors r+ |
Build failed (retrying...): |
Build succeeded: |
This change augments the migration infrastructure with an optional
hook for tenant migrations to express preconditions that will prevent
any attempt to push the cluster version to a version which requires
this migration to run.
This will be helpful to prevent scenarios where user migrations are doomed
to fail based on current state in the database but that would only be
determined after the cluster had locked out the older version.
Release note: None