Skip to content

Add real owner to imagebuild metadata #9511

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 1 commit into from
Apr 29, 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
235 changes: 123 additions & 112 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 @@ -65,6 +65,7 @@ message BuildRequest {
BuildSource source = 1;
BuildRegistryAuth auth = 2;
bool force_rebuild = 3;
string triggered_by = 4;
}

message BuildRegistryAuth {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ export class BuildRequest extends jspb.Message {
setAuth(value?: BuildRegistryAuth): BuildRequest;
getForceRebuild(): boolean;
setForceRebuild(value: boolean): BuildRequest;
getTriggeredBy(): string;
setTriggeredBy(value: string): BuildRequest;

serializeBinary(): Uint8Array;
toObject(includeInstance?: boolean): BuildRequest.AsObject;
Expand All @@ -233,6 +235,7 @@ export namespace BuildRequest {
source?: BuildSource.AsObject,
auth?: BuildRegistryAuth.AsObject,
forceRebuild: boolean,
triggeredBy: string,
}
}

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 @@ -1758,7 +1758,8 @@ 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),
forceRebuild: jspb.Message.getBooleanFieldWithDefault(msg, 3, false)
forceRebuild: jspb.Message.getBooleanFieldWithDefault(msg, 3, false),
triggeredBy: jspb.Message.getFieldWithDefault(msg, 4, "")
};

if (includeInstance) {
Expand Down Expand Up @@ -1809,6 +1810,10 @@ proto.builder.BuildRequest.deserializeBinaryFromReader = function(msg, reader) {
var value = /** @type {boolean} */ (reader.readBool());
msg.setForceRebuild(value);
break;
case 4:
var value = /** @type {string} */ (reader.readString());
msg.setTriggeredBy(value);
break;
default:
reader.skipField();
break;
Expand Down Expand Up @@ -1861,6 +1866,13 @@ proto.builder.BuildRequest.serializeBinaryToWriter = function(message, writer) {
f
);
}
f = message.getTriggeredBy();
if (f.length > 0) {
writer.writeString(
4,
f
);
}
};


Expand Down Expand Up @@ -1956,6 +1968,24 @@ proto.builder.BuildRequest.prototype.setForceRebuild = function(value) {
};


/**
* optional string triggered_by = 4;
* @return {string}
*/
proto.builder.BuildRequest.prototype.getTriggeredBy = function() {
return /** @type {string} */ (jspb.Message.getFieldWithDefault(this, 4, ""));
};


/**
* @param {string} value
* @return {!proto.builder.BuildRequest} returns this
*/
proto.builder.BuildRequest.prototype.setTriggeredBy = function(value) {
return jspb.Message.setProto3StringField(this, 4, value);
};



/**
* Oneof group definitions for this message. Each group defines the field
Expand Down
14 changes: 10 additions & 4 deletions components/image-builder-mk3/pkg/orchestrator/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ import (
)

const (
annotationRef = "ref"
annotationBaseRef = "baseref"
annotationRef = "ref"
annotationBaseRef = "baseref"
annotationManagedBy = "managed-by"
)

type orchestrator interface {
Expand Down Expand Up @@ -70,14 +71,17 @@ func (m *buildMonitor) Run() {
for {
wss, err := m.wsman.GetWorkspaces(ctx, &wsmanapi.GetWorkspacesRequest{
MustMatch: &wsmanapi.MetadataFilter{
Owner: buildWorkspaceOwnerID,
Annotations: map[string]string{
annotationManagedBy: buildWorkspaceManagerID,
},
},
})
if err != nil {
log.WithError(err).Info("cannot get running builds from ws-manager - retrying")
time.Sleep(5 * time.Second)
continue
}

m.runningBuildsMu.Lock()
m.runningBuilds = make(map[string]*runningBuild, len(wss.Status))
m.runningBuildsMu.Unlock()
Expand All @@ -87,7 +91,9 @@ func (m *buildMonitor) Run() {

sub, err := m.wsman.Subscribe(ctx, &wsmanapi.SubscribeRequest{
MustMatch: &wsmanapi.MetadataFilter{
Owner: buildWorkspaceOwnerID,
Annotations: map[string]string{
annotationManagedBy: buildWorkspaceManagerID,
},
},
})
if err != nil {
Expand Down
13 changes: 6 additions & 7 deletions components/image-builder-mk3/pkg/orchestrator/orchestrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ import (
)

const (
// buildWorkspaceOwnerID is the owner ID we pass to ws-manager
buildWorkspaceOwnerID = "image-builder"
// buildWorkspaceManagerID identifies the manager for the workspace
buildWorkspaceManagerID = "image-builder"

// maxBuildRuntime is the maximum time a build is allowed to take
maxBuildRuntime = 60 * time.Minute
Expand Down Expand Up @@ -345,12 +345,11 @@ func (o *Orchestrator) Build(req *protocol.BuildRequest, resp protocol.ImageBuil
Metadata: &wsmanapi.WorkspaceMetadata{
MetaId: buildID,
Annotations: map[string]string{
annotationRef: wsrefstr,
annotationBaseRef: baseref,
annotationRef: wsrefstr,
annotationBaseRef: baseref,
annotationManagedBy: buildWorkspaceManagerID,
},
// TODO(cw): use the actual image build owner here and move to annotation based filter
// when retrieving running image builds.
Owner: buildWorkspaceOwnerID,
Owner: req.GetTriggeredBy(),
},
Spec: &wsmanapi.StartWorkspaceSpec{
Initializer: initializer,
Expand Down
1 change: 1 addition & 0 deletions components/server/src/workspace/workspace-starter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,7 @@ export class WorkspaceStarter {
req.setSource(src);
req.setAuth(auth);
req.setForceRebuild(forceRebuild);
req.setTriggeredBy(user.id);
Copy link
Member

Choose a reason for hiding this comment

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

Note that it's not defined what the "real owner" of an image build is, exactly. Calling it "triggered by" though makes sense.


// Make sure we persist logInfo as soon as we retrieve it
const imageBuildLogInfo = new Deferred<ImageBuildLogInfo>();
Expand Down