Skip to content

upgrades: make a couple of permanent upgrades idempotent#92611

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
andreimatei:upgrades.idempotent
Dec 2, 2022
Merged

upgrades: make a couple of permanent upgrades idempotent#92611
craig[bot] merged 1 commit intocockroachdb:masterfrom
andreimatei:upgrades.idempotent

Conversation

@andreimatei
Copy link
Copy Markdown
Contributor

Some of the permanent upgrades (opting into telemetry and initializing the cluster secret) were not idempotent; this patch makes them be. In particular, this is important since these upgrades will run on 23.1 cluster that are being upgraded from 22.2 as they've been turned from startupmigrations to upgrades in #91627. For the telemetry one, there's no "natural" way to make it idempotent, so I've added a check for the old startupmigration key.

Release note: None
Epic: None

@andreimatei andreimatei requested review from a team and ajwerner November 28, 2022 20:22
@andreimatei andreimatei requested a review from a team as a code owner November 28, 2022 20:22
@andreimatei andreimatei removed the request for review from a team November 28, 2022 20:22
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@andreimatei andreimatei removed the request for review from a team November 28, 2022 20:22
Copy link
Copy Markdown
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @andreimatei)


pkg/keys/constants.go line 304 at r1 (raw file):

	// StartupMigrationPrefix specifies the key prefix to store all migration details.
	StartupMigrationPrefix = roachpb.Key(makeKey(SystemPrefix, roachpb.RKey("system-version/")))
	//

nit remove this and the weird one above it

Some of the permanent upgrades (opting into telemetry and initializing
the cluster secret) were not idempotent; this patch makes them be. In
particular, this is important since these upgrades will run on 23.1
cluster that are being upgraded from 22.2 as they've been turned from
startupmigrations to upgrades in cockroachdb#91627. For the telemetry one, there's
no "natural" way to make it idempotent, so I've added a check for the
old startupmigration key.

Release note: None
Epic: None
@andreimatei andreimatei requested a review from a team December 2, 2022 16:36
@andreimatei andreimatei requested a review from a team as a code owner December 2, 2022 16:36
Copy link
Copy Markdown
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @andreimatei)


pkg/keys/constants.go line 304 at r1 (raw file):

Previously, ajwerner wrote…

nit remove this and the weird one above it

done

Copy link
Copy Markdown
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ajwerner)

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 2, 2022

Build succeeded:

@craig craig bot merged commit f7f24fb into cockroachdb:master Dec 2, 2022
@andreimatei andreimatei deleted the upgrades.idempotent branch December 5, 2022 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants