-
Notifications
You must be signed in to change notification settings - Fork 59
add support for migrating postgres pvcs to hyperdisk #3653
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,17 +4,20 @@ import * as gcp from '@pulumi/gcp'; | |
| import * as pulumi from '@pulumi/pulumi'; | ||
| import * as random from '@pulumi/random'; | ||
| import * as _ from 'lodash'; | ||
| import { CustomResource } from '@pulumi/kubernetes/apiextensions'; | ||
| import { Resource } from '@pulumi/pulumi'; | ||
|
|
||
| import { CnChartVersion } from './artifacts'; | ||
| import { clusterSmallDisk, CloudSqlConfig, config } from './config'; | ||
| import { spliceConfig } from './config/config'; | ||
| import { hyperdiskSupportConfig } from './config/hyperdiskSupportConfig'; | ||
| import { | ||
| appsAffinityAndTolerations, | ||
| infraAffinityAndTolerations, | ||
| installSpliceHelmChart, | ||
| nonHyperdiskAppsAffinityAndTolerations, | ||
| } from './helm'; | ||
| import { installPostgresPasswordSecret } from './secrets'; | ||
| import { standardStorageClassName } from './storage/storageClass'; | ||
| import { ChartValues, CLUSTER_BASENAME, ExactNamespace, GCP_ZONE } from './utils'; | ||
|
|
||
| const project = gcp.organizations.getProjectOutput({}); | ||
|
|
@@ -224,6 +227,35 @@ export class SplicePostgres extends pulumi.ComponentResource implements Postgres | |
|
|
||
| // an initial database named cantonnet is created automatically (configured in the Helm chart). | ||
| const smallDiskSize = clusterSmallDisk ? '240Gi' : undefined; | ||
| const supportsHyperdisk = | ||
| hyperdiskSupportConfig.hyperdiskSupport.enabled && !useInfraAffinityAndTolerations; | ||
| let hyperdiskMigrationValues = {}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any reason not do
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. have to create the snapshot in the if
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah I see; this sounds like a reason then... want to add a brief comment? (nit)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see the value of the comment tbh, it would comment on the syntax of typescript not add any extra details about the why
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, why can't you write this in functional style again? Would something like this work? (all of this is nit; I just stumbled about the inconsistency so figured other readers might too; we very rarely use mutable variables)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do use mutable variables in tyepscript when needed. |
||
| if (supportsHyperdisk && hyperdiskSupportConfig.hyperdiskSupport.migrating) { | ||
| const pvcSnapshot = new CustomResource( | ||
| `pg-data-${xns.logicalName}-${instanceName}-snapshot`, | ||
| { | ||
| apiVersion: 'snapshot.storage.k8s.io/v1', | ||
| kind: 'VolumeSnapshot', | ||
| metadata: { | ||
| name: `pg-data-${instanceName}-snapshot`, | ||
| namespace: xns.logicalName, | ||
| }, | ||
| spec: { | ||
| volumeSnapshotClassName: 'dev-vsc', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. offtopic, just ranting: that name...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. have no idea who created that, it's in all the clusters 🤷 not managed by us |
||
| source: { | ||
| persistentVolumeClaimName: `pg-data-${instanceName}-0`, | ||
| }, | ||
| }, | ||
| } | ||
| ); | ||
| hyperdiskMigrationValues = { | ||
| dataSource: { | ||
| kind: 'VolumeSnapshot', | ||
| name: pvcSnapshot.metadata.name, | ||
| apiGroup: 'snapshot.storage.k8s.io', | ||
| }, | ||
| }; | ||
| } | ||
| const pg = installSpliceHelmChart( | ||
| xns, | ||
| instanceName, | ||
|
|
@@ -233,6 +265,13 @@ export class SplicePostgres extends pulumi.ComponentResource implements Postgres | |
| volumeSize: overrideDbSizeFromValues | ||
| ? values?.db?.volumeSize || smallDiskSize | ||
| : smallDiskSize, | ||
| ...(supportsHyperdisk | ||
| ? { | ||
| volumeStorageClass: standardStorageClassName, | ||
| pvcTemplateName: 'pg-data-hd', | ||
| ...hyperdiskMigrationValues, | ||
| } | ||
| : {}), | ||
| }, | ||
| persistence: { | ||
| secretName: this.secretName, | ||
|
|
@@ -242,11 +281,19 @@ export class SplicePostgres extends pulumi.ComponentResource implements Postgres | |
| { | ||
| aliases: [{ name: logicalNameAlias, type: 'kubernetes:helm.sh/v3:Release' }], | ||
| dependsOn: [passwordSecret], | ||
| ...((supportsHyperdisk && | ||
| // during the migration we first delete the stateful set, which keeps the old pvcs, and the recreate with the new pvcs | ||
nicu-da marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| // the stateful sets are immutable so they need to be recreated to force the change of the pvcs | ||
| hyperdiskSupportConfig.hyperdiskSupport.migrating) || | ||
| spliceConfig.pulumiProjectConfig.replacePostgresStatefulSetOnChanges | ||
| ? { | ||
| replaceOnChanges: ['*'], | ||
| deleteBeforeReplace: true, | ||
| } | ||
| : {}), | ||
| }, | ||
| true, | ||
| useInfraAffinityAndTolerations | ||
| ? infraAffinityAndTolerations | ||
| : nonHyperdiskAppsAffinityAndTolerations | ||
| useInfraAffinityAndTolerations ? infraAffinityAndTolerations : appsAffinityAndTolerations | ||
| ); | ||
| this.pg = pg; | ||
|
|
||
|
|
||
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.
you are a brave man... I would have done a helm test here just to be sure I counted the whitespaces right (I think I'd still soft recommend it)
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 did a full manual test to migrate a scratch