Skip to content
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

Migrate Prebuild settings – EXP-672 #18842

Merged
merged 24 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
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: 0 additions & 3 deletions components/dashboard/src/admin/ProjectDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ export default function ProjectDetail(props: { project: Project; owner: string |
</Property>
</div>
<div className="flex w-full mt-6">
<Property name="Incremental Prebuilds">
{props.project.settings?.useIncrementalPrebuilds ? "Yes" : "No"}
</Property>
<Property name="Marked Deleted">{props.project.markedDeleted ? "Yes" : "No"}</Property>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion components/dashboard/src/projects/ProjectListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const ProjectListItem: FunctionComponent<ProjectListItemProps> = ({ proje
const [showRemoveModal, setShowRemoveModal] = useState(false);
const { data: prebuild, isLoading } = useLatestProjectPrebuildQuery({ projectId: project.id });

const enablePrebuilds = Project.isPrebuildsEnabled(project);
const enablePrebuilds = Project.getPrebuildSettings(project).enable;

return (
<div key={`project-${project.id}`} className="h-52">
Expand Down
268 changes: 125 additions & 143 deletions components/dashboard/src/projects/ProjectSettings.tsx

Large diffs are not rendered by default.

15 changes: 7 additions & 8 deletions components/dashboard/src/service/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,16 @@ export function projectToProtocol(project: Project): ProtocolProject {
teamId: project.teamId,
appInstallationId: "undefined",
settings: {
enablePrebuilds: project.settings?.prebuild?.enablePrebuilds,
prebuildDefaultBranchOnly: project.settings?.prebuild?.prebuildDefaultBranchOnly,
prebuildBranchPattern: project.settings?.prebuild?.prebuildBranchPattern,
allowUsingPreviousPrebuilds: project.settings?.prebuild?.usePreviousPrebuilds,
keepOutdatedPrebuildsRunning: project.settings?.prebuild?.keepOutdatedPrebuildsRunning,
prebuildEveryNthCommit: project.settings?.prebuild?.prebuildEveryNth,
useIncrementalPrebuilds: project.settings?.prebuild?.enableIncrementalPrebuilds,
workspaceClasses: {
prebuild: project.settings?.workspace?.workspaceClass?.prebuild || "",
regular: project.settings?.workspace?.workspaceClass?.regular || "",
},
prebuilds: {
enable: project.settings?.prebuild?.enablePrebuilds,
branchStrategy: project.settings?.prebuild?.branchStrategy as any,
branchMatchingPattern: project.settings?.prebuild?.branchMatchingPattern,
prebuildInterval: project.settings?.prebuild?.prebuildInterval,
workspaceClass: project.settings?.prebuild?.workspaceClass,
},
},
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ export function CreateWorkspacePage() {

// we already have shown running workspaces to the user
opts.ignoreRunningWorkspaceOnSameCommit = true;
opts.ignoreRunningPrebuild = true;

// if user received an INVALID_GITPOD_YML yml for their contextURL they can choose to proceed using default configuration
if (workspaceContext.error?.code === ErrorCodes.INVALID_GITPOD_YML) {
Expand Down
2 changes: 1 addition & 1 deletion components/gitpod-db/src/typeorm/workspace-db-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ export class TypeORMWorkspaceDBImpl extends TransactionalDBImpl<WorkspaceDB> imp
const repo = await this.getPrebuiltWorkspaceRepo();

let query = repo.createQueryBuilder("pws");
query = query.where("pws.projectId != :projectId", { projectId });
query = query.where("pws.projectId = :projectId", { projectId });
Copy link
Member

Choose a reason for hiding this comment

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

πŸ™

query = query.andWhere("pws.creationTime >= :time", { time: date.toISOString() });
query = query.andWhere("pws.state != :state", { state: abortedState });
return query.getCount();
Expand Down
37 changes: 13 additions & 24 deletions components/gitpod-protocol/go/gitpod-service.go
Original file line number Diff line number Diff line change
Expand Up @@ -1678,19 +1678,9 @@ type Repository struct {

// WorkspaceCreationResult is the WorkspaceCreationResult message type
type WorkspaceCreationResult struct {
CreatedWorkspaceID string `json:"createdWorkspaceId,omitempty"`
ExistingWorkspaces []*WorkspaceInfo `json:"existingWorkspaces,omitempty"`
RunningPrebuildWorkspaceID string `json:"runningPrebuildWorkspaceID,omitempty"`
RunningWorkspacePrebuild *RunningWorkspacePrebuild `json:"runningWorkspacePrebuild,omitempty"`
WorkspaceURL string `json:"workspaceURL,omitempty"`
}

// RunningWorkspacePrebuild is the RunningWorkspacePrebuild message type
type RunningWorkspacePrebuild struct {
PrebuildID string `json:"prebuildID,omitempty"`
SameCluster bool `json:"sameCluster,omitempty"`
Starting string `json:"starting,omitempty"`
WorkspaceID string `json:"workspaceID,omitempty"`
CreatedWorkspaceID string `json:"createdWorkspaceId,omitempty"`
ExistingWorkspaces []*WorkspaceInfo `json:"existingWorkspaces,omitempty"`
WorkspaceURL string `json:"workspaceURL,omitempty"`
}

// Workspace is the Workspace message type
Expand Down Expand Up @@ -2027,8 +2017,6 @@ type CreateWorkspaceOptions struct {
ContextURL string `json:"contextUrl,omitempty"`
OrganizationId string `json:"organizationId,omitempty"`
IgnoreRunningWorkspaceOnSameCommit bool `json:"ignoreRunningWorkspaceOnSameCommit,omitemopty"`
IgnoreRunningPrebuild bool `json:"ignoreRunningPrebuild,omitemopty"`
AllowUsingPreviousPrebuilds bool `json:"allowUsingPreviousPrebuilds,omitemopty"`
ForceDefaultConfig bool `json:"forceDefaultConfig,omitemopty"`
}

Expand Down Expand Up @@ -2291,15 +2279,16 @@ type Project struct {
}

type ProjectSettings struct {
EnablePrebuilds *bool `json:"enablePrebuilds,omitempty"`
PrebuildDefaultBranchOnly *bool `json:"prebuildDefaultBranchOnly,omitempty"`
PrebuildBranchPattern *string `json:"prebuildBranchPattern,omitempty"`
UseIncrementalPrebuilds bool `json:"useIncrementalPrebuilds,omitempty"`
UsePersistentVolumeClaim bool `json:"usePersistentVolumeClaim,omitempty"`
KeepOutdatedPrebuildsRunning bool `json:"keepOutdatedPrebuildsRunning,omitempty"`
AllowUsingPreviousPrebuilds bool `json:"allowUsingPreviousPrebuilds,omitempty"`
PrebuildEveryNthCommit int `json:"prebuildEveryNthCommit,omitempty"`
WorkspaceClasses *WorkspaceClassesSettings `json:"workspaceClasses,omitempty"`
UsePersistentVolumeClaim bool `json:"usePersistentVolumeClaim,omitempty"`
WorkspaceClasses *WorkspaceClassesSettings `json:"workspaceClasses,omitempty"`
PrebuildSettings *PrebuildSettings `json:"prebuilds,omitempty"`
}
type PrebuildSettings struct {
Enable *bool `json:"enable,omitempty"`
PrebuildInterval *int32 `json:"prebuildInterval,omitempty"`
BranchStrategy *string `json:"branchStrategy,omitempty"`
BranchMatchingPattern *string `json:"branchMatchingPattern,omitempty"`
WorkspaceClass *string `json:"workspaceClass,omitempty"`
}

type WorkspaceClassesSettings struct {
Expand Down
2 changes: 0 additions & 2 deletions components/gitpod-protocol/src/gitpod-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,6 @@ export namespace GitpodServer {
// whether running workspaces on the same context should be ignored. If false (default) users will be asked.
//TODO(se) remove this option and let clients do that check if they like. The new create workspace page does it already
ignoreRunningWorkspaceOnSameCommit?: boolean;
ignoreRunningPrebuild?: boolean;
allowUsingPreviousPrebuilds?: boolean;
forceDefaultConfig?: boolean;
}

Expand Down
12 changes: 3 additions & 9 deletions components/gitpod-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,9 @@ export type IDESettings = {

export interface WorkspaceClasses {
regular?: string;
/**
* @deprecated see Project.settings.prebuilds.workspaceClass
*/
prebuild?: string;
}

Expand Down Expand Up @@ -1497,16 +1500,7 @@ export interface WorkspaceCreationResult {
createdWorkspaceId?: string;
workspaceURL?: string;
existingWorkspaces?: WorkspaceInfo[];
runningWorkspacePrebuild?: {
prebuildID: string;
workspaceID: string;
instanceID: string;
starting: RunningWorkspacePrebuildStarting;
sameCluster: boolean;
};
runningPrebuildWorkspaceID?: string;
}
export type RunningWorkspacePrebuildStarting = "queued" | "starting" | "running";

export namespace WorkspaceCreationResult {
export function is(data: any): data is WorkspaceCreationResult {
Expand Down
113 changes: 75 additions & 38 deletions components/gitpod-protocol/src/teams-projects-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,79 @@ export interface ProjectConfig {
}

export interface ProjectSettings {
/**
* Controls settings of prebuilds for this project.
*/
prebuilds?: PrebuildSettings;

/** @deprecated see `Project.settings.prebuilds.enabled` instead. */
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI, not removing here, as these are required for the migration phase

enablePrebuilds?: boolean;
/**
* Wether prebuilds (if enabled) should only be started on the default branch.
* Defaults to `true` on project creation.
*
* @deprecated see `Project.settings.prebuilds.branchStrategy` instead.
*/
prebuildDefaultBranchOnly?: boolean;
/**
* Use this pattern to match branch names to run prebuilds on.
* The pattern matching will only be applied if prebuilds are enabled and
* they are not limited to the default branch.
*
* @deprecated see `Project.settings.prebuilds.branchMatchingPattern` instead.
*/
prebuildBranchPattern?: string;
/**
* how many commits in the commit history a prebuild is good (undefined and 0 means every commit is prebuilt)
*
* @deprecated see `Project.settings.prebuilds.intervall` instead.
*/
prebuildEveryNthCommit?: number;

/**
* @deprecated always false
*/
useIncrementalPrebuilds?: boolean;

/**
* @deprecated always true (we should kill dangling prebuilds)
*/
keepOutdatedPrebuildsRunning?: boolean;
// whether new workspaces can start on older prebuilds and incrementally update
/**
* @deprecated always true
*/
allowUsingPreviousPrebuilds?: boolean;
// how many commits in the commit history a prebuild is good (undefined and 0 means every commit is prebuilt)
prebuildEveryNthCommit?: number;

// preferred workspace classes
workspaceClasses?: WorkspaceClasses;
}
export namespace ProjectSettings {
export type PrebuildBranchStrategy = "defaultBranch" | "allBranches" | "selectedBranches";
export namespace PrebuildSettings {
export type BranchStrategy = "default-branch" | "all-branches" | "matched-branches";
}

export interface PrebuildSettings {
enable?: boolean;

/**
* Defines an interval of commits to run new prebuilds for. Defaults to 20
*/
prebuildInterval?: number;

/**
* Which branches to consider to run new prebuilds on. Default to "all-branches"
*/
branchStrategy?: PrebuildSettings.BranchStrategy;
/**
* If `branchStrategy` s set to "matched-branches", this should define a glob-pattern to be used
* to match the branch to run new prebuilds on. Defaults to "**"
*/
branchMatchingPattern?: string;

/**
* Preferred workspace class for prebuilds.
*/
workspaceClass?: string;
}

export interface Project {
Expand Down Expand Up @@ -69,46 +119,33 @@ export namespace Project {
return p.name + "-" + p.id;
}

export type PrebuildSettingsWithDefaults = Required<Pick<PrebuildSettings, "prebuildInterval">> & PrebuildSettings;

export const PREBUILD_SETTINGS_DEFAULTS: PrebuildSettingsWithDefaults = {
enable: false,
branchMatchingPattern: "**",
prebuildInterval: 20,
branchStrategy: "all-branches",
};

/**
* If *no settings* are present on pre-existing projects, this defaults to `true` (enabled) for
* backwards compatibility. This allows to do any explicit migration of data or adjustment of
* the default behavior at a later point in time.
*
* Otherwise this returns the value of the `enablePrebuilds` settings persisted in the given
* project.
* Returns effective prebuild settings for the given project. The resulting settings
* contain default values for properties which are not set explicitly for this project.
*/
export function isPrebuildsEnabled(project: Project): boolean {
// Defaulting to `true` for backwards compatibility. Ignoring non-boolean for `enablePrebuilds`
// for evaluation here allows to do any explicit migration of data or adjustment of the default
// behavior at a later point in time.
if (!hasPrebuildSettings(project)) {
return true;
}
export function getPrebuildSettings(project: Project): PrebuildSettingsWithDefaults {
// ignoring persisted properties with `undefined` values to exclude them from the override.
const overrides = Object.fromEntries(
Object.entries(project.settings?.prebuilds ?? {}).filter(([_, value]) => value !== undefined),
);

return !!project.settings?.enablePrebuilds;
return {
...PREBUILD_SETTINGS_DEFAULTS,
...overrides,
};
}

export function hasPrebuildSettings(project: Project) {
return !(typeof project.settings?.enablePrebuilds === "undefined");
}

export function getPrebuildBranchStrategy(project: Project): ProjectSettings.PrebuildBranchStrategy {
if (!hasPrebuildSettings(project)) {
// returning "all branches" to mimic the default value of projects which were added
// before introduction of persisted settings for prebuilds.
return "allBranches";
}
if (typeof project.settings?.prebuildDefaultBranchOnly === "undefined") {
return "defaultBranch"; // default value for `settings.prebuildDefaultBranchOnly`
}
if (project.settings?.prebuildDefaultBranchOnly) {
return "defaultBranch";
}
const prebuildBranchPattern = project.settings?.prebuildBranchPattern?.trim();
if (typeof prebuildBranchPattern === "string" && prebuildBranchPattern.length > 1) {
return "selectedBranches";
}
return "allBranches";
return !(typeof project.settings?.prebuilds === "undefined");
}

export interface Overview {
Expand Down
24 changes: 12 additions & 12 deletions components/public-api-server/pkg/apiv1/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,21 +167,21 @@ func projectSettingsToAPIResponse(s *protocol.ProjectSettings) *v1.ProjectSettin
return &v1.ProjectSettings{}
}

return &v1.ProjectSettings{
Prebuild: &v1.PrebuildSettings{
EnablePrebuilds: s.EnablePrebuilds,
PrebuildDefaultBranchOnly: s.PrebuildDefaultBranchOnly,
PrebuildBranchPattern: s.PrebuildBranchPattern,
EnableIncrementalPrebuilds: s.UseIncrementalPrebuilds,
KeepOutdatedPrebuildsRunning: s.KeepOutdatedPrebuildsRunning,
UsePreviousPrebuilds: s.AllowUsingPreviousPrebuilds,
PrebuildEveryNth: int32(s.PrebuildEveryNthCommit),
},
settings := &v1.ProjectSettings{
Prebuild: &v1.PrebuildSettings{},
Workspace: &v1.WorkspaceSettings{
EnablePersistentVolumeClaim: s.UsePersistentVolumeClaim,
WorkspaceClass: workspaceClassesToAPIResponse(s.WorkspaceClasses),
WorkspaceClass: workspaceClassesToAPIResponse(s.WorkspaceClasses),
},
}
if s.PrebuildSettings != nil {
settings.Prebuild.EnablePrebuilds = s.PrebuildSettings.Enable
settings.Prebuild.BranchStrategy = s.PrebuildSettings.BranchStrategy
settings.Prebuild.BranchMatchingPattern = s.PrebuildSettings.BranchMatchingPattern
settings.Prebuild.PrebuildInterval = s.PrebuildSettings.PrebuildInterval
settings.Prebuild.WorkspaceClass = s.PrebuildSettings.WorkspaceClass
}

return settings
}

func workspaceClassesToAPIResponse(s *protocol.WorkspaceClassesSettings) *v1.WorkspaceClassSettings {
Expand Down
10 changes: 5 additions & 5 deletions components/public-api-server/pkg/apiv1/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,22 +328,22 @@ func setupProjectsService(t *testing.T) (*protocol.MockAPIInterface, v1connect.P

func newProject(p *protocol.Project) *protocol.Project {
r := rand.Int()
b_false := false
Copy link
Member

Choose a reason for hiding this comment

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

[nit] Underscore is an anti-pattern in go. camelCase is the expected

result := &protocol.Project{
ID: uuid.New().String(),
Name: fmt.Sprintf("team-%d", r),
TeamID: uuid.New().String(),
CloneURL: "https://github.com/easyCZ/foobar",
AppInstallationID: "1337",
Settings: &protocol.ProjectSettings{
UseIncrementalPrebuilds: true,
UsePersistentVolumeClaim: true,
KeepOutdatedPrebuildsRunning: true,
AllowUsingPreviousPrebuilds: true,
PrebuildEveryNthCommit: 5,
UsePersistentVolumeClaim: true,
WorkspaceClasses: &protocol.WorkspaceClassesSettings{
Regular: "default",
Prebuild: "default",
},
PrebuildSettings: &protocol.PrebuildSettings{
Enable: &b_false,
},
},
CreationTime: "2022-09-09T09:09:09.000Z",
}
Expand Down
15 changes: 9 additions & 6 deletions components/public-api/gitpod/experimental/v1/projects.proto
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,16 @@ message ProjectSettings {
}

message PrebuildSettings {
bool enable_incremental_prebuilds = 1;
bool keep_outdated_prebuilds_running = 2;
bool use_previous_prebuilds = 3;
int32 prebuild_every_nth = 4;
reserved 1; // enable_incremental_prebuilds
Copy link
Member

Choose a reason for hiding this comment

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

πŸ‘

reserved 2; // keep_outdated_prebuilds_running
reserved 3; // use_previous_prebuilds
reserved 4; // was prebuild_every_nth
optional bool enable_prebuilds = 5;
optional bool prebuild_default_branch_only = 6;
optional string prebuild_branch_pattern = 7;
reserved 6; // was prebuild_default_branch_only
optional string branch_matching_pattern = 7;
optional string branch_strategy = 8;
optional int32 prebuild_interval = 9;
optional string workspace_class = 10;
}

message WorkspaceSettings {
Expand Down
Loading
Loading