-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[server] migrate ws without usageattribution #17485
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 all commits
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,7 +4,7 @@ | |
* See License.AGPL.txt in the project root for license information. | ||
*/ | ||
|
||
import { AdditionalUserData, Team, User } from "@gitpod/gitpod-protocol"; | ||
import { AdditionalUserData, Team, User, WorkspaceInfo } from "@gitpod/gitpod-protocol"; | ||
import { ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error"; | ||
import { inject, injectable } from "inversify"; | ||
import { ProjectDB } from "./project-db"; | ||
|
@@ -15,6 +15,7 @@ import { TypeORM } from "./typeorm/typeorm"; | |
import { log, LogContext } from "@gitpod/gitpod-protocol/lib/util/logging"; | ||
import { UserDB } from "./user-db"; | ||
import { Synchronizer } from "./typeorm/synchronizer"; | ||
import { AttributionId } from "@gitpod/gitpod-protocol/lib/attribution"; | ||
|
||
@injectable() | ||
export class UserToTeamMigrationService { | ||
|
@@ -135,6 +136,13 @@ export class UserToTeamMigrationService { | |
); | ||
log.info(ctx, "Migrated workspaces.", { teamId: team.id, result }); | ||
|
||
// Ensure there are no workspaces without an organizationId. This is necessary because very old workspace instance don't have an attributionId. | ||
const workspaces = await this.workspaceDB.find({ | ||
userId: user.id, | ||
}); | ||
await this.updateWorkspacesOrganizationId(workspaces, team.id); | ||
log.info(ctx, "Updated workspaces.", { teamId: team.id }); | ||
|
||
result = await conn.query("UPDATE d_b_usage SET attributionId = ? WHERE attributionId = ?", [ | ||
newAttribution, | ||
oldAttribution, | ||
|
@@ -153,4 +161,30 @@ export class UserToTeamMigrationService { | |
const teams = await this.teamDB.findTeamsByUser(user.id); | ||
return teams.length === 0 || !user.additionalData?.isMigratedToTeamOnlyAttribution; | ||
} | ||
|
||
async updateWorkspacesOrganizationId(workspaces: WorkspaceInfo[], userOrgId: string): Promise<WorkspaceInfo[]> { | ||
return await Promise.all( | ||
workspaces.map(async (ws) => { | ||
if (!ws.workspace.organizationId) { | ||
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. It only doies something on workspaces without an organizationId. So should be fast in other cases. |
||
const attrId = | ||
ws.latestInstance?.usageAttributionId && | ||
AttributionId.parse(ws.latestInstance.usageAttributionId); | ||
if (attrId && attrId.kind === "team") { | ||
ws.workspace.organizationId = attrId.teamId; | ||
} else { | ||
ws.workspace.organizationId = userOrgId; | ||
} | ||
await this.workspaceDB.updatePartial(ws.workspace.id, { | ||
organizationId: ws.workspace.organizationId, | ||
}); | ||
} | ||
return ws; | ||
}), | ||
); | ||
} | ||
|
||
async getUserOrganization(user: User): Promise<Team> { | ||
const teams = await this.teamDB.findTeamsByUser(user.id); | ||
return teams.find((t) => t.name === user.name || t.name === user.fullName) || teams[0]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1206,6 +1206,17 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { | |
), | ||
), | ||
); | ||
|
||
// we need to update old workspaces on the fly that didn't get an orgId because we lack attribution on their instances. | ||
// this can be removed eventually. | ||
if (user.additionalData?.isMigratedToTeamOnlyAttribution) { | ||
try { | ||
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. this runs the same tested code, but just to be sure is wrapped in try-catch. |
||
const userOrg = await this.userToTeamMigrationService.getUserOrganization(user); | ||
await this.userToTeamMigrationService.updateWorkspacesOrganizationId(res, userOrg.id); | ||
} catch (error) { | ||
log.error({ userId: user.id }, "Error updating workspaces without orgId.", error); | ||
} | ||
} | ||
return res; | ||
} | ||
|
||
|
@@ -1292,11 +1303,24 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable { | |
} | ||
|
||
protected async internalGetWorkspace(id: string, db: WorkspaceDB): Promise<Workspace> { | ||
const ws = await db.findById(id); | ||
if (!ws) { | ||
const workspace = await db.findById(id); | ||
if (!workspace) { | ||
throw new ResponseError(ErrorCodes.NOT_FOUND, "Workspace not found."); | ||
} | ||
return ws; | ||
if (!workspace.organizationId && this.user?.additionalData?.isMigratedToTeamOnlyAttribution) { | ||
try { | ||
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. try-catch here as well. Also only runs for workspaces without orgId and after migration |
||
log.info({ userId: this.user.id }, "Updating workspace without orgId."); | ||
const userOrg = await this.userToTeamMigrationService.getUserOrganization(this.user); | ||
const latestInstance = await this.workspaceDb.trace({}).findCurrentInstance(workspace.id); | ||
this.userToTeamMigrationService.updateWorkspacesOrganizationId( | ||
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. Missing |
||
[{ workspace, latestInstance }], | ||
userOrg.id, | ||
); | ||
} catch (error) { | ||
log.error({ userId: this.user.id }, "Error updating workspaces without orgId.", error); | ||
} | ||
} | ||
return workspace; | ||
} | ||
|
||
private async findRunningInstancesForContext( | ||
|
Uh oh!
There was an error while loading. Please reload this page.
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.
This test fails without the addition in the migration code