Skip to content

Commit

Permalink
[image-builder] Support imagebuild/ context prefix
Browse files Browse the repository at this point in the history
Forces the build of the workspace image.
  • Loading branch information
corneliusludmann authored and csweichel committed May 25, 2021
1 parent 8402607 commit cacf3ed
Show file tree
Hide file tree
Showing 10 changed files with 210 additions and 123 deletions.
1 change: 1 addition & 0 deletions components/gitpod-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,7 @@ export interface WorkspaceContext {
title: string;
normalizedContextURL?: string;
forceCreateNewWorkspace?: boolean;
forceImageBuild?: boolean;
}

export namespace WorkspaceContext {
Expand Down
194 changes: 102 additions & 92 deletions components/image-builder-api/go/imgbuilder.pb.go

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions components/image-builder-api/imgbuilder.proto
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ message ResolveWorkspaceImageResponse {
message BuildRequest {
BuildSource source = 1;
BuildRegistryAuth auth = 2;
bool forceRebuild = 3;
}

message BuildRegistryAuth {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ export class BuildRequest extends jspb.Message {
clearAuth(): void;
getAuth(): BuildRegistryAuth | undefined;
setAuth(value?: BuildRegistryAuth): BuildRequest;
getForcerebuild(): boolean;
setForcerebuild(value: boolean): BuildRequest;

serializeBinary(): Uint8Array;
toObject(includeInstance?: boolean): BuildRequest.AsObject;
Expand All @@ -230,6 +232,7 @@ export namespace BuildRequest {
export type AsObject = {
source?: BuildSource.AsObject,
auth?: BuildRegistryAuth.AsObject,
forcerebuild: boolean,
}
}

Expand Down
32 changes: 31 additions & 1 deletion components/image-builder-api/typescript/src/imgbuilder_pb.js
Original file line number Diff line number Diff line change
Expand Up @@ -1735,7 +1735,8 @@ proto.builder.BuildRequest.prototype.toObject = function(opt_includeInstance) {
proto.builder.BuildRequest.toObject = function(includeInstance, msg) {
var f, obj = {
source: (f = msg.getSource()) && proto.builder.BuildSource.toObject(includeInstance, f),
auth: (f = msg.getAuth()) && proto.builder.BuildRegistryAuth.toObject(includeInstance, f)
auth: (f = msg.getAuth()) && proto.builder.BuildRegistryAuth.toObject(includeInstance, f),
forcerebuild: jspb.Message.getBooleanFieldWithDefault(msg, 3, false)
};

if (includeInstance) {
Expand Down Expand Up @@ -1782,6 +1783,10 @@ proto.builder.BuildRequest.deserializeBinaryFromReader = function(msg, reader) {
reader.readMessage(value,proto.builder.BuildRegistryAuth.deserializeBinaryFromReader);
msg.setAuth(value);
break;
case 3:
var value = /** @type {boolean} */ (reader.readBool());
msg.setForcerebuild(value);
break;
default:
reader.skipField();
break;
Expand Down Expand Up @@ -1827,6 +1832,13 @@ proto.builder.BuildRequest.serializeBinaryToWriter = function(message, writer) {
proto.builder.BuildRegistryAuth.serializeBinaryToWriter
);
}
f = message.getForcerebuild();
if (f) {
writer.writeBool(
3,
f
);
}
};


Expand Down Expand Up @@ -1904,6 +1916,24 @@ proto.builder.BuildRequest.prototype.hasAuth = function() {
};


/**
* optional bool forceRebuild = 3;
* @return {boolean}
*/
proto.builder.BuildRequest.prototype.getForcerebuild = function() {
return /** @type {boolean} */ (jspb.Message.getBooleanFieldWithDefault(this, 3, false));
};


/**
* @param {boolean} value
* @return {!proto.builder.BuildRequest} returns this
*/
proto.builder.BuildRequest.prototype.setForcerebuild = function(value) {
return jspb.Message.setProto3BooleanField(this, 3, value);
};



/**
* Oneof group definitions for this message. Each group defines the field
Expand Down
52 changes: 32 additions & 20 deletions components/image-builder/pkg/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,29 +215,33 @@ func (b *DockerBuilder) Build(req *api.BuildRequest, resp api.ImageBuilder_Build
return status.Errorf(codes.Internal, "cannot get workspace image authentication: %v", err)
}

// check if needs build -> early return
exists, err := b.checkImageExists(ctx, wsrefstr, wsrefAuth)
if err != nil {
return dockerErrToGRPC(err, "cannot check if image is already built")
}
if exists {
// If the workspace image exists, so should the baseimage if we've built it.
// If we didn't build it and the base image doesn't exist anymore, getWorkspaceImageRef will have failed to resolve the baseref.
baserefAbsolute, err := b.getAbsoluteImageRef(ctx, baseref, allowedAuthForAll)
forceRebuid := req.GetForceRebuild()

if !forceRebuid {
// check if needs build -> early return
exists, err := b.checkImageExists(ctx, wsrefstr, wsrefAuth)
if err != nil {
return status.Errorf(codes.Internal, "cannot resolve base image ref: %v", err)
return dockerErrToGRPC(err, "cannot check if image is already built")
}
if exists {
// If the workspace image exists, so should the baseimage if we've built it.
// If we didn't build it and the base image doesn't exist anymore, getWorkspaceImageRef will have failed to resolve the baseref.
baserefAbsolute, err := b.getAbsoluteImageRef(ctx, baseref, allowedAuthForAll)
if err != nil {
return status.Errorf(codes.Internal, "cannot resolve base image ref: %v", err)
}

// image has already been built - no need for us to start building
err = resp.Send(&api.BuildResponse{
Status: api.BuildStatus_done_success,
Ref: wsrefstr,
BaseRef: baserefAbsolute,
})
if err != nil {
return err
// image has already been built - no need for us to start building
err = resp.Send(&api.BuildResponse{
Status: api.BuildStatus_done_success,
Ref: wsrefstr,
BaseRef: baserefAbsolute,
})
if err != nil {
return err
}
return nil
}
return nil
}

// Once a build is running we don't want it cancelled becuase the server disconnected i.e. during deployment.
Expand Down Expand Up @@ -372,7 +376,15 @@ func (b *DockerBuilder) Build(req *api.BuildRequest, resp api.ImageBuilder_Build
if err != nil {
return status.Errorf(codes.Internal, "cannot check base image exists: %v", err)
}
if baseExists {

var isRefSource bool
switch req.Source.From.(type) {
case *api.BuildSource_Ref:
isRefSource = true
default:
isRefSource = false
}
if baseExists && (!forceRebuid || isRefSource) {
if strings.HasPrefix(baseref, b.Config.BaseImageRepository) {
// the base image we're about to pull is one that we've built before.
// In that case we enter the workspace phase prematurely to give the censor
Expand Down
4 changes: 3 additions & 1 deletion components/server/src/container-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import { GitTokenScopeGuesser } from './workspace/git-token-scope-guesser';
import { GitTokenValidator } from './workspace/git-token-validator';
import { newAnalyticsWriterFromEnv, IAnalyticsWriter } from '@gitpod/gitpod-protocol/lib/util/analytics';
import { OAuthController } from './oauth-server/oauth-controller';
import { ImageBuildPrefixContextParser } from './workspace/imagebuild-prefix-context-parser';

export const productionContainerModule = new ContainerModule((bind, unbind, isBound, rebind) => {
bind(Env).toSelf().inSingletonScope();
Expand Down Expand Up @@ -127,13 +128,14 @@ export const productionContainerModule = new ContainerModule((bind, unbind, isBo
bind(CachingImageBuilderClientProvider).toSelf().inSingletonScope();
bind(ImageBuilderClientProvider).toService(CachingImageBuilderClientProvider);

/* The binding order of the context parser does not configure preference/a working prder. Each context parser must be able
/* The binding order of the context parser does not configure preference/a working order. Each context parser must be able
* to decide for themselves, independently and without overlap to the other parsers what to do.
*/
bind(ContextParser).toSelf().inSingletonScope();
bind(SnapshotContextParser).toSelf().inSingletonScope();
bind(IContextParser).to(SnapshotContextParser).inSingletonScope();
bind(IPrefixContextParser).to(EnvvarPrefixParser).inSingletonScope();
bind(IPrefixContextParser).to(ImageBuildPrefixContextParser).inSingletonScope();

bind(GitTokenScopeGuesser).toSelf().inSingletonScope();
bind(GitTokenValidator).toSelf().inSingletonScope();
Expand Down
2 changes: 1 addition & 1 deletion components/server/src/workspace/gitpod-server-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ export class GitpodServerImpl<Client extends GitpodClient, Server extends Gitpod
}
}

public async startWorkspace(workspaceId: string, options: { forceDefaultImage: boolean }): Promise<StartWorkspaceResult> {
public async startWorkspace(workspaceId: string, options: GitpodServer.StartWorkspaceOptions): Promise<StartWorkspaceResult> {
const span = opentracing.globalTracer().startSpan("startWorkspace");
span.setTag("workspaceId", workspaceId);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Copyright (c) 2021 Gitpod GmbH. All rights reserved.
* Licensed under the GNU Affero General Public License (AGPL).
* See License-AGPL.txt in the project root for license information.
*/

import { User, WorkspaceContext } from "@gitpod/gitpod-protocol";
import { injectable } from "inversify";
import { IPrefixContextParser } from "./context-parser";

@injectable()
export class ImageBuildPrefixContextParser implements IPrefixContextParser {
static PREFIX = 'imagebuild/';

findPrefix(user: User, context: string): string | undefined {
if (context.startsWith(ImageBuildPrefixContextParser.PREFIX)) {
return ImageBuildPrefixContextParser.PREFIX;
}
}

public async handle(user: User, prefix: string, context: WorkspaceContext): Promise<WorkspaceContext> {
context.forceImageBuild = true
return context;
}
}
19 changes: 11 additions & 8 deletions components/server/src/workspace/workspace-starter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,13 @@ export class WorkspaceStarter {
let instance = await this.workspaceDb.trace({ span }).storeInstance(await this.newInstance(workspace, user));
span.log({ "newInstance": instance.id });

const forceRebuild = !!workspace.context.forceImageBuild;

let needsImageBuild: boolean;
try {
// if we need to build the workspace image we musn't wait for actuallyStartWorkspace to return as that would block the
// frontend until the image is built.
needsImageBuild = await this.needsImageBuild({ span }, user, workspace, instance);
needsImageBuild = forceRebuild || await this.needsImageBuild({ span }, user, workspace, instance);
if (needsImageBuild) {
instance.status.conditions = {
neededImageBuild: true,
Expand All @@ -112,11 +114,11 @@ export class WorkspaceStarter {
// If the caller requested that errors be rethrown we must await the actual workspace start to be in the exception path.
// To this end we disable the needsImageBuild behaviour if rethrow is true.
if (needsImageBuild && !options.rethrow) {
this.actuallyStartWorkspace({ span }, instance, workspace, user, userEnvVars, options.rethrow);
this.actuallyStartWorkspace({ span }, instance, workspace, user, userEnvVars, options.rethrow, forceRebuild);
return { instanceID: instance.id };
}

return await this.actuallyStartWorkspace({ span }, instance, workspace, user, userEnvVars);
return await this.actuallyStartWorkspace({ span }, instance, workspace, user, userEnvVars, options.rethrow, forceRebuild);
} catch (e) {
TraceContext.logError({ span }, e);
throw e;
Expand All @@ -127,12 +129,12 @@ export class WorkspaceStarter {

// Note: this function does not expect to be awaited for by its caller. This means that it takes care of error handling itself
// and creates its tracing span as followFrom rather than the usual childOf reference.
protected async actuallyStartWorkspace(ctx: TraceContext, instance: WorkspaceInstance, workspace: Workspace, user: User, userEnvVars?: UserEnvVar[], rethrow?: boolean): Promise<StartWorkspaceResult> {
protected async actuallyStartWorkspace(ctx: TraceContext, instance: WorkspaceInstance, workspace: Workspace, user: User, userEnvVars?: UserEnvVar[], rethrow?: boolean, forceRebuild?: boolean): Promise<StartWorkspaceResult> {
const span = TraceContext.startAsyncSpan("actuallyStartWorkspace", ctx);

try {
// build workspace image
instance = await this.buildWorkspaceImage({ span }, user, workspace, instance);
instance = await this.buildWorkspaceImage({ span }, user, workspace, instance, forceRebuild, forceRebuild);

let type: WorkspaceType = WorkspaceType.REGULAR;
if (workspace.type === 'prebuild') {
Expand Down Expand Up @@ -410,17 +412,18 @@ export class WorkspaceStarter {
}
}

protected async buildWorkspaceImage(ctx: TraceContext, user: User, workspace: Workspace, instance: WorkspaceInstance, ignoreBaseImageresolvedAndRebuildBase: boolean = false): Promise<WorkspaceInstance> {
protected async buildWorkspaceImage(ctx: TraceContext, user: User, workspace: Workspace, instance: WorkspaceInstance, ignoreBaseImageresolvedAndRebuildBase: boolean = false, forceRebuild: boolean = false): Promise<WorkspaceInstance> {
const span = TraceContext.startSpan("buildWorkspaceImage", ctx);

try {
// Start build...
const client = this.imagebuilderClientProvider.getDefault();
const {src, auth, disposable} = await this.prepareBuildRequest({ span }, workspace, workspace.imageSource!, user, ignoreBaseImageresolvedAndRebuildBase);
const {src, auth, disposable} = await this.prepareBuildRequest({ span }, workspace, workspace.imageSource!, user, ignoreBaseImageresolvedAndRebuildBase || forceRebuild);

const req = new BuildRequest();
req.setSource(src);
req.setAuth(auth);
req.setForcerebuild(forceRebuild);

const result = await client.build({ span }, req);

Expand All @@ -446,7 +449,7 @@ export class WorkspaceStarter {
if (err && err.message && err.message.includes("base image does not exist") && !ignoreBaseImageresolvedAndRebuildBase) {
// we've attempted to add the base layer for a workspace whoose base image has gone missing.
// Ignore the previously built (now missing) base image and force a rebuild.
return this.buildWorkspaceImage(ctx, user, workspace, instance, true);
return this.buildWorkspaceImage(ctx, user, workspace, instance, true, forceRebuild);
} else {
throw err;
}
Expand Down

0 comments on commit cacf3ed

Please sign in to comment.