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

[image-builder] Support imagebuild/ context prefix #4261

Merged
merged 1 commit into from
May 25, 2021
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
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