Skip to content

[ws-manager] Make cluster selection filter by application cluster #14000

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

Merged
merged 2 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion components/gitpod-protocol/src/workspace-cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export interface WorkspaceClusterDB {
*/
findFiltered(predicate: DeepPartial<WorkspaceClusterFilter>): Promise<WorkspaceClusterWoTLS[]>;
}
export interface WorkspaceClusterFilter extends Pick<WorkspaceCluster, "state" | "govern" | "url"> {
export interface WorkspaceClusterFilter
extends Pick<WorkspaceCluster, "name" | "state" | "govern" | "url" | "applicationCluster"> {
minScore: number;
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ export interface WorkspaceManagerClientProviderSource {
getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTLS[]>;
}


@injectable()
export class WorkspaceManagerClientProviderEnvSource implements WorkspaceManagerClientProviderSource {
protected _clusters: WorkspaceCluster[] | undefined = undefined;
readonly applicationCluster = process.env.GITPOD_INSTALLATION_SHORTNAME ?? "";

public async getWorkspaceCluster(name: string): Promise<WorkspaceCluster | undefined> {
return this.clusters.find(m => m.name === name);
return this.clusters.find((m) => m.name === name && m.applicationCluster === this.applicationCluster);
Copy link
Member

Choose a reason for hiding this comment

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

For debugging purposes, I'd recommend logging the following:

  1. The set of clusters available
  2. The set of clusters which match this application cluster

We can add these as log fields. This would help us identify scenarios when the filtering logic doesn't work as expected.

Copy link
Member

Choose a reason for hiding this comment

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

The extra benefit of this is if we accidentally end up in a situation where there is more than 1 cluster that matches, we'd see that in the logs when debugging.

}

public async getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTLS[]> {
return this.clusters;
return this.clusters.filter((m) => m.applicationCluster === this.applicationCluster) ?? [];
}

protected get clusters(): WorkspaceCluster[] {
Expand Down Expand Up @@ -63,13 +63,14 @@ export class WorkspaceManagerClientProviderEnvSource implements WorkspaceManager
export class WorkspaceManagerClientProviderDBSource implements WorkspaceManagerClientProviderSource {
@inject(WorkspaceClusterDB)
protected readonly db: WorkspaceClusterDB;
readonly applicationCluster = process.env.GITPOD_INSTALLATION_SHORTNAME ?? "";

public async getWorkspaceCluster(name: string): Promise<WorkspaceCluster | undefined> {
return await this.db.findByName(name);
return (await this.db.findFiltered({ name, applicationCluster: this.applicationCluster }))[0];
}

public async getAllWorkspaceClusters(): Promise<WorkspaceClusterWoTLS[]> {
return await this.db.findFiltered({});
return await this.db.findFiltered({ applicationCluster: this.applicationCluster });
}
}

Expand Down Expand Up @@ -105,4 +106,4 @@ export class WorkspaceManagerClientProviderCompositeSource implements WorkspaceM
}
return result;
}
}
}