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

Add PVC support to prebuilds #10689

Merged
merged 1 commit into from
Jun 24, 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
4 changes: 4 additions & 0 deletions components/content-service/pkg/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,10 @@ func (c *Client) Fetch(ctx context.Context) (err error) {
return c.Git(ctx, "fetch", "-p", "-P", "--tags", "-f")
}

func (c *Client) AddSafeDirectory(ctx context.Context, dir string) (err error) {
return c.Git(ctx, "config", "--global", "--add", "safe.directory", dir)
}

// UpdateRemote performs a git fetch on the upstream remote URI
func (c *Client) UpdateRemote(ctx context.Context) (err error) {
//nolint:staticcheck,ineffassign
Expand Down
10 changes: 7 additions & 3 deletions components/content-service/pkg/initializer/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ func (ws *GitInitializer) Run(ctx context.Context, mappings []archive.IDMapping)
b := backoff.NewExponentialBackOff()
b.MaxElapsedTime = 5 * time.Minute
if err = backoff.RetryNotify(gitClone, b, onGitCloneFailure); err != nil {
return src, xerrors.Errorf("git initializer: %w", err)
return src, xerrors.Errorf("git initializer gitClone: %w", err)
}

if err := ws.AddSafeDirectory(ctx, ws.Location); err != nil {
log.WithError(err).Warn("git initializer AddSafeDirectory")
}

if ws.Chown {
Expand All @@ -115,10 +119,10 @@ func (ws *GitInitializer) Run(ctx context.Context, mappings []archive.IDMapping)
}
}
if err := ws.realizeCloneTarget(ctx); err != nil {
return src, xerrors.Errorf("git initializer: %w", err)
return src, xerrors.Errorf("git initializer clone: %w", err)
}
if err := ws.UpdateRemote(ctx); err != nil {
return src, xerrors.Errorf("git initializer: %w", err)
return src, xerrors.Errorf("git initializer updateRemote: %w", err)
}
if err := ws.UpdateSubmodules(ctx); err != nil {
log.WithError(err).Warn("error while updating submodules - continuing")
Expand Down
7 changes: 4 additions & 3 deletions components/content-service/pkg/initializer/initializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,10 @@ func newGitInitializer(ctx context.Context, loc string, req *csapi.GitInitialize

func newSnapshotInitializer(loc string, rs storage.DirectDownloader, req *csapi.SnapshotInitializer) (*SnapshotInitializer, error) {
return &SnapshotInitializer{
Location: loc,
Snapshot: req.Snapshot,
Storage: rs,
Location: loc,
Snapshot: req.Snapshot,
Storage: rs,
FromVolumeSnapshot: req.FromVolumeSnapshot,
}, nil
}

Expand Down
13 changes: 10 additions & 3 deletions components/content-service/pkg/initializer/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/opentracing/opentracing-go"
"golang.org/x/xerrors"

"github.com/gitpod-io/gitpod/common-go/log"
"github.com/gitpod-io/gitpod/common-go/tracing"
csapi "github.com/gitpod-io/gitpod/content-service/api"
"github.com/gitpod-io/gitpod/content-service/pkg/archive"
Expand All @@ -18,9 +19,10 @@ import (

// SnapshotInitializer downloads a snapshot from a remote storage
type SnapshotInitializer struct {
Location string
Snapshot string
Storage storage.DirectDownloader
Location string
Snapshot string
Storage storage.DirectDownloader
FromVolumeSnapshot bool
}

// Run downloads a snapshot from a remote storage
Expand All @@ -32,6 +34,11 @@ func (s *SnapshotInitializer) Run(ctx context.Context, mappings []archive.IDMapp

src = csapi.WorkspaceInitFromBackup

if s.FromVolumeSnapshot {
log.Info("SnapshotInitializer detected volume snapshot, skipping")
return src, nil
}

ok, err := s.Storage.DownloadSnapshot(ctx, s.Location, s.Snapshot, mappings)
if err != nil {
return src, xerrors.Errorf("snapshot initializer: %w", err)
Expand Down
19 changes: 19 additions & 0 deletions components/content-service/pkg/layer/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,27 @@ func (s *Provider) GetContentLayerPVC(ctx context.Context, owner, workspaceID st
// At this point we've found neither a full-workspace-backup, nor a legacy backup.
// It's time to use the initializer.
if gis := initializer.GetSnapshot(); gis != nil {
jenting marked this conversation as resolved.
Show resolved Hide resolved
if gis.FromVolumeSnapshot {
layer, err = contentDescriptorToLayerPVC([]byte{})
if err != nil {
return nil, nil, err
}

l = []Layer{*layer}
return l, manifest, nil
}
return s.getSnapshotContentLayer(ctx, gis)
}
if pis := initializer.GetPrebuild(); pis != nil {
if pis.Prebuild.FromVolumeSnapshot {
layer, err = contentDescriptorToLayerPVC([]byte{})
if err != nil {
return nil, nil, err
}

l = []Layer{*layer}
return l, manifest, nil
}
l, manifest, err = s.getPrebuildContentLayer(ctx, pis)
if err != nil {
log.WithError(err).WithFields(log.OWI(owner, workspaceID, "")).Warn("cannot initialize from prebuild - falling back to Git")
Expand Down Expand Up @@ -481,6 +499,7 @@ git config --global --add safe.directory ${GITPOD_REPO_ROOT}
git status --porcelain=v2 --branch -uall > /.workspace/prestophookdata/git_status.txt
git log --pretty='%h: %s' --branches --not --remotes > /.workspace/prestophookdata/git_log_1.txt
git log --pretty=%H -n 1 > /.workspace/prestophookdata/git_log_2.txt
cp /workspace/.gitpod/prebuild-log* /.workspace/prestophookdata/
`

// version of this function for persistent volume claim feature
Expand Down
9 changes: 8 additions & 1 deletion components/content-service/pkg/logs/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,17 @@ func ListPrebuildLogFiles(ctx context.Context, location string) (filePaths []str
}
return logFiles, nil
}
filePaths, err = listLogFiles(strings.TrimPrefix(TerminalStoreLocation, "/workspace"), prebuildLogFilePrefix)
// list log files in `location` first
filePaths, err = listLogFiles("", prebuildLogFilePrefix)
if err != nil {
return nil, err
}
if len(filePaths) == 0 {
filePaths, err = listLogFiles(strings.TrimPrefix(TerminalStoreLocation, "/workspace"), prebuildLogFilePrefix)
if err != nil {
return nil, err
}
}
if len(filePaths) == 0 {
filePaths, err = listLogFiles(strings.TrimPrefix(legacyTerminalStoreLocation, "/workspace"), legacyPrebuildLogFilePrefix)
if err != nil {
Expand Down
9 changes: 9 additions & 0 deletions components/server/ee/src/prebuilds/prebuild-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,10 @@ export class PrebuildManager {
} else {
span.setTag("starting", true);
const projectEnvVars = await projectEnvVarsPromise;
const usePVC = this.shouldUsePersistentVolumeClaim(project);
await this.workspaceStarter.startWorkspace({ span }, workspace, user, [], projectEnvVars, {
excludeFeatureFlags: ["full_workspace_backup"],
forcePVC: usePVC,
});
}

Expand Down Expand Up @@ -274,6 +276,13 @@ export class PrebuildManager {
return this.config.incrementalPrebuilds.repositoryPasslist.some((url) => trimRepoUrl(url) === repoUrl);
}

protected shouldUsePersistentVolumeClaim(project?: Project): boolean {
if (project?.settings?.usePersistentVolumeClaim) {
return true;
}
return false;
}

async fetchConfig(ctx: TraceContext, user: User, context: CommitContext): Promise<WorkspaceConfig> {
const span = TraceContext.startSpan("fetchConfig", ctx);
try {
Expand Down
3 changes: 2 additions & 1 deletion components/server/ee/src/workspace/workspace-starter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ export class WorkspaceStarterEE extends WorkspaceStarter {
user: User,
excludeFeatureFlags: NamedWorkspaceFeatureFlag[],
ideConfig: IDEConfig,
forcePVC: boolean,
): Promise<WorkspaceInstance> {
const instance = await super.newInstance(ctx, workspace, user, excludeFeatureFlags, ideConfig);
const instance = await super.newInstance(ctx, workspace, user, excludeFeatureFlags, ideConfig, forcePVC);
if (await this.eligibilityService.hasFixedWorkspaceResources(user)) {
const config: WorkspaceInstanceConfiguration = instance.configuration!;
const ff = config.featureFlags || [];
Expand Down
31 changes: 26 additions & 5 deletions components/server/src/workspace/workspace-starter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export interface StartWorkspaceOptions {
rethrow?: boolean;
forceDefaultImage?: boolean;
excludeFeatureFlags?: NamedWorkspaceFeatureFlag[];
forcePVC?: boolean;
}

const MAX_INSTANCE_START_RETRIES = 2;
Expand Down Expand Up @@ -275,7 +276,14 @@ export class WorkspaceStarter {
let instance = await this.workspaceDb
.trace({ span })
.storeInstance(
await this.newInstance(ctx, workspace, user, options.excludeFeatureFlags || [], ideConfig),
await this.newInstance(
ctx,
workspace,
user,
options.excludeFeatureFlags || [],
ideConfig,
options.forcePVC || false,
),
);
span.log({ newInstance: instance.id });

Expand Down Expand Up @@ -640,6 +648,7 @@ export class WorkspaceStarter {
user: User,
excludeFeatureFlags: NamedWorkspaceFeatureFlag[],
ideConfig: IDEConfig,
forcePVC: boolean,
): Promise<WorkspaceInstance> {
//#endregion IDE resolution TODO(ak) move to IDE service
// TODO: Compatible with ide-config not deployed, need revert after ide-config deployed
Expand Down Expand Up @@ -716,6 +725,10 @@ export class WorkspaceStarter {

featureFlags = featureFlags.filter((f) => !excludeFeatureFlags.includes(f));

if (forcePVC === true) {
featureFlags = featureFlags.concat(["persistent_volume_claim"]);
}

if (!!featureFlags) {
// only set feature flags if there actually are any. Otherwise we waste the
// few bytes of JSON in the database for no good reason.
Expand Down Expand Up @@ -1283,10 +1296,13 @@ export class WorkspaceStarter {
}
}

let volumeSnapshotId = lastValidWorkspaceInstanceId;
// if this is snapshot or prebuild context, then try to find volume snapshot id in it
if (SnapshotContext.is(workspace.context) || WithPrebuild.is(workspace.context)) {
volumeSnapshotId = workspace.context.snapshotBucketId;
}
laushinka marked this conversation as resolved.
Show resolved Hide resolved
let volumeSnapshotInfo = new VolumeSnapshotInfo();
const volumeSnapshots = await this.workspaceDb
.trace(traceCtx)
.findVolumeSnapshotById(lastValidWorkspaceInstanceId);
const volumeSnapshots = await this.workspaceDb.trace(traceCtx).findVolumeSnapshotById(volumeSnapshotId);
if (volumeSnapshots !== undefined) {
volumeSnapshotInfo.setVolumeSnapshotName(volumeSnapshots.id);
volumeSnapshotInfo.setVolumeSnapshotHandle(volumeSnapshots.volumeHandle);
Expand All @@ -1302,7 +1318,10 @@ export class WorkspaceStarter {
);
const userTimeoutPromise = this.userService.getDefaultWorkspaceTimeout(user);

const featureFlags = instance.configuration!.featureFlags || [];
let featureFlags = instance.configuration!.featureFlags || [];
if (volumeSnapshots !== undefined) {
featureFlags = featureFlags.concat(["persistent_volume_claim"]);
}

let ideImage: string;
if (!!instance.configuration?.ideImage) {
Expand Down Expand Up @@ -1470,6 +1489,7 @@ export class WorkspaceStarter {
} else if (SnapshotContext.is(context)) {
const snapshot = new SnapshotInitializer();
snapshot.setSnapshot(context.snapshotBucketId);
snapshot.setFromVolumeSnapshot(hasVolumeSnapshot);
result.setSnapshot(snapshot);
} else if (WithPrebuild.is(context)) {
if (!CommitContext.is(context)) {
Expand All @@ -1478,6 +1498,7 @@ export class WorkspaceStarter {

const snapshot = new SnapshotInitializer();
snapshot.setSnapshot(context.snapshotBucketId);
snapshot.setFromVolumeSnapshot(hasVolumeSnapshot);
const { initializer } = await this.createCommitInitializer(traceCtx, workspace, context, user);
const init = new PrebuildInitializer();
init.setPrebuild(snapshot);
Expand Down
6 changes: 5 additions & 1 deletion components/ws-daemon/pkg/content/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -658,8 +658,12 @@ func (s *WorkspaceService) uploadWorkspaceLogs(ctx context.Context, sess *sessio
return xerrors.Errorf("no remote storage configured")
}

logLocation := sess.Location
if sess.PersistentVolumeClaim {
logLocation = filepath.Join(sess.ServiceLocDaemon, "prestophookdata")
}
// currently we're only uploading prebuild log files
logFiles, err := logs.ListPrebuildLogFiles(ctx, sess.Location)
logFiles, err := logs.ListPrebuildLogFiles(ctx, logLocation)
if err != nil {
return err
}
Expand Down
17 changes: 17 additions & 0 deletions components/ws-manager/pkg/manager/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1081,9 +1081,26 @@ func (m *Monitor) finalizeWorkspaceContent(ctx context.Context, wso *workspaceOb
err = m.manager.markWorkspace(context.Background(), workspaceID, addMark(pvcWorkspaceVolumeSnapshotAnnotation, string(b)))
if err != nil {
log.WithError(err).Error("cannot mark workspace with volume snapshot name - snapshot will be orphaned and backup lost")
errMark := m.manager.markWorkspace(ctx, workspaceID, addMark(workspaceExplicitFailAnnotation, xerrors.Errorf("cannot add mark to save snapshot info: %v", err).Error()))
if errMark != nil {
log.WithError(errMark).Warn("was unable to mark workspace as failed")
}
return true, nil, err
}

if tpe == api.WorkspaceType_PREBUILD {
err = m.manager.markWorkspace(context.Background(), workspaceID, addMark(workspaceSnapshotAnnotation, pvcVolumeSnapshotName))
if err != nil {
tracing.LogError(span, err)
log.WithError(err).Warn("cannot mark headless workspace with snapshot - that's one prebuild lost")
errMark := m.manager.markWorkspace(ctx, workspaceID, addMark(workspaceExplicitFailAnnotation, xerrors.Errorf("cannot add mark for prebuild snapshot info: %v", err).Error()))
if errMark != nil {
log.WithError(errMark).Warn("was unable to mark workspace as failed")
}
return true, nil, err
}
}

markVolumeSnapshotAnnotation = true
}

Expand Down