[content-service] incremental workspace init

[svenefftinge]

If we initialize the workspace from an outdated prebuild, we
want the regular workspace tasks (including init) to run.

Related Issue(s)

Related to #12582

How to test

Start a workspace on an an outdated prebuild and see that the init task runs.

Release Notes

NONE

Documentation

Werft options:

• /werft with-local-preview
  If enabled this will build install/preview
• /werft with-preview
• /werft with-large-vm
• /werft with-integration-tests=workspace
  Valid options are all, workspace, webapp, ide

[werft-gitpod-dev-com]

started the job as gitpod-build-se-incremental-init.1 because the annotations in the pull request description changed
(with .werft/ from main)

[werft-gitpod-dev-com]

started the job as gitpod-build-se-incremental-init.4 because the annotations in the pull request description changed
(with .werft/ from main)

[svenefftinge]

/werft run with-clean-slate-deployment

👍 started the job as gitpod-build-se-incremental-init.5
(with .werft/ from main)

[utam0k]

Do you have a plan to add the integration test about it on another PR?

[svenefftinge]

Do you have a plan to add the integration test about it on another PR?

I could add a unit test. Where do we have the other git initializer tests?

[svenefftinge]

I looked into the existing integration tests and couldn't see anything close. I think building something would be a bit too heavy for this case. Only for the positive case, what we would need to do is setup an external repo with at least two commits on a branch. Then create a prebuild on the older commit and start a workspace on the newer when the prebuild is ready. The expectation would be that init task runs in the new workspace but that there are some traces from the previous prebuild.

[utam0k]

I looked into the existing integration tests and couldn't see anything close. I think building something would be a bit too heavy for this case. Only for the positive case, what we would need to do is setup an external repo with at least two commits on a branch. Then create a prebuild on the older commit and start a workspace on the newer when the prebuild is ready. The expectation would be that init task runs in the new workspace but that there are some traces from the previous prebuild.

Sure, I agree with you. it might be a bit heavy. But this feature is complex and I'd prefer to have an integration test to avoid regression. How about creating an issue by discussing the response about testing there? I don't think it is necessary to include that test in this PR

[utam0k]

/werft run with-integration-tests=workspace

👍 started the job as gitpod-build-se-incremental-init.6
(with .werft/ from main)

[jankeromnes]

Nice! Many thanks @svenefftinge 🙏 I'll give this a spin. 👀 💿

[jankeromnes]

I tested this with:

• https://github.com/jankeromnes/gitpod-staging-prebuilds (verify which tasks are run in what sequence and what workspace ID)
• https://github.com/jankeromnes/test-gitpod-incremental-workspaces (help test incremental workspaces thanks to a 2min prebuild task & verify which commits are checked out in prebuild / in the workspace)

And this seems to work quite well! One question about the UX though:

Based on prebuild?	Terminal output?
No prebuild	Run init and command live
Fresh prebuild	Print init log and run command live
Outdated prebuild (new)	?

Currently, the behavior of "?" seems to be:

• Run init (again) and command
• (but we have no idea about what happened in the first init run in the outdated prebuild?)

Maybe it should both print the prebuild log and run init again? E.g.

• Print (old) init log + run init (again) + command

[sagor999]

@svenefftinge when this PR is ready, can you please enable integration tests for it and run it through those, just to be safe. (it might help to enable large VM option as well)

[svenefftinge]

@jankeromnes
I intentionally treated this situation as WorkspaceInitFromOther to communicate that this the regular case. I.e. I'm running my regular builds on top of the checkout I have. The state from the prebuild might help to speed things up but it is functionally irrelevant (i.e. the only thing we are interested in is potential caches) because it ran on an older state. This is really the same, regular situation where a dev would go to their dev environment, do git pull, and run the build. It's not important what the output and result of any previous build runs on older versions was. It should functionally not even be relevant if there ever was a build.

[werft-gitpod-dev-com]

started the job as gitpod-build-se-incremental-init.7 because the annotations in the pull request description changed
(with .werft/ from main)

[werft-gitpod-dev-com]

started the job as gitpod-build-se-incremental-init.8 because the annotations in the pull request description changed
(with .werft/ from main)

[utam0k]

Changed integration test to workspace only. all is too heavy.

[svenefftinge]

How about creating an issue by discussing the response about testing there?

I think we can continue discussing it here, even if the PR is merged.
All this change does is to refer to the regular initialization if we are starting from a prebuild that didn't run on the same commit. I would love to have a test for this if it was pulling its weight, but I believe the additional overhead (writing, maintaining, added complexity in test suite, additional execution time) is not worth it in this case.

[utam0k]

How about creating an issue by discussing the response about testing there?

I think we can continue discussing it here, even if the PR is merged. All this change does is to refer to the regular initialization if we are starting from a prebuild that didn't run on the same commit. I would love to have a test for this if it was pulling its weight, but I believe the additional overhead (writing, maintaining, added complexity in test suite, additional execution time) is not worth it in this case.

We have already

• prebuild testing
• git add/commit test

Can we combine these to implement this?

If it is going to take more than an hour to implement, I too will vote for no testing. But if we encounter will regressions about this feature in the future, then we spend the time to create tests. WDYT?

[svenefftinge]

Thanks for the pointers @utam0k. That helped a lot to write a test case.
The test ran through ✅

[jankeromnes]

Many thanks @svenefftinge!

FYI, the state that I tested last time worked really well from a UX perspective, with only the UX question about the old prebuild logs (but this could definitely be a follow-up improvement, so not a blocker). I'll still reply to your comment below.

However -- what is a blocker is that now, I'm unable to get this to work again 😕 has there been a regression maybe?

1. I've added the following repository as a project https://github.com/jankeromnes/test-gitpod-incremental-workspaces (if you want to take a look, it's under this team)
2. I already had a prebuild for the latest commit ("Wob")
3. I pushed a new commit ("Wob wob") to trigger a new prebuild
4. In the Branches view, I refreshed a few times until I saw both the new commit and the "In Progress" prebuild, then I clicked on "New Workspace"
5. This showed me the "Prebuild in Progress" screen as expected, and I clicked on "Use Last Successful Prebuild"

Unfortunately, this gave me a workspace in this state (i.e. it's still the old prebuild commit "Wob" that is checked out, not the new "Wob wob" commit, and I'm not even on branch "main"):

Maybe something broke in one of your latest pushes?

(FYI, I was about to approve this PR for team WebApp because the UX was working well, but now it seems broken again. 🥲 Could you please take a look when you have time? Or maybe I'm missing something?)

[jankeromnes]

Also, regarding the UX question (but note that this is a follow-up discussion, so not a blocker here):

---

My main worry is that, by not printing the old prebuild log, we're "hiding" potentially useful debug information, i.e. we're being less transparent with the user as to what is happening. I would personally much prefer to cat the old prebuild logs + print a message like "Running an incremental build to speed up your workspace ♻️" + run the init commands again.

The state from the prebuild might help to speed things up but it is functionally irrelevant (i.e. the only thing we are interested in is potential caches) because it ran on an older state.

I think it can be relevant in some edge cases, for example if your build system prints "using version XYZ of dependency BLA", and later on your build fails due to an incompatible dependency.

If we don't re-print logs, users may get very surprised as to why sometimes their build passes (from scratch) and sometimes it fails (incremental workspace), without any logs or indicator to help them debug what's happening.

This is really the same, regular situation where a dev would go to their dev environment, do git pull, and run the build. It's not important what the output and result of any previous build runs on older versions was. It should functionally not even be relevant if there ever was a build.

That's true -- however I would argue that, when you have a local checkout, you expect to have some old state in there (always), but when you start a new Gitpod workspace, I think the expectation is reversed -- I expect to always start from a clean slate, and when that perceived clean slate fails due to some remaining older state (and the IDE does not show me any hint that an older state was even used) I can get quite surprised.

[svenefftinge]

I would personally much prefer to cat the old prebuild logs + print a message like "Running an incremental build to speed up your workspace ♻️" + run the init commands again.

As I said I'm strongly against doing this as it would create noise and make things seem more complicated than they are. The logs would end up twice in my console and the first part would be for an arbitrary past state. How could this possibly be useful?

[svenefftinge]

Maybe something broke in one of your latest pushes?

Strange, I haven't changed the production code only added a test case, which succeeds.

[svenefftinge]

and the IDE does not show me any hint that an older state was even used

If it turns out that this is indeed a source of surprise, we should think about other ways (more explicit than printing logs) to tell users they are on an incremental workspace.

[svenefftinge]

I wasn't able to reproduce this by doing the following:

• forking your repo (my fork) and setting it up as a project
• waiting for the prebuild on main to finish
• then commit a small change to the readme
• then starting a fresh workspace on branch main

I got the expected flow. That is I first end up on prebuild running screen and when I click the start of old prebuild button, I get a workspace that executed the init task and presents:

gitpod /workspace/test-gitpod-incremental-workspaces (main) $  HISTFILE=/workspace/.gitpod/cmd-0 history -r; {
> sleep 240 && echo "$(date) prebuild commit $(git rev-parse HEAD)" >> /workspace/.init
> } && {
> cat /workspace/.init && echo "$(date) current commit $(git rev-parse HEAD)"
> }
Thu 27 Oct 2022 11:10:21 AM UTC prebuild commit 9308391f1704dd7d0e64abe3601c714bad108ab7
Thu 27 Oct 2022 11:17:44 AM UTC prebuild commit 93925490053ff04bb5cef82e3fa1c98aedd2cb44
Thu 27 Oct 2022 11:17:44 AM UTC current commit 93925490053ff04bb5cef82e3fa1c98aedd2cb44
gitpod /workspace/test-gitpod-incremental-workspaces (main) $ 

Could it be that did something wrong in your manual testing? You would end up in such a state if you start on the commit context. This is not something the changes in the PR would influence. So if the button on the "prebuild in progress" sometimes starts on a commit we have this flakiness on main already.

[jankeromnes]

Could it be that did something wrong in your manual testing? You would end up in such a state if you start on the commit context.

Ah, good call -- and my apologies, I indeed missed that the New Workspace button in the Project Overview (a.k.a. Branches page) has recently changed from a branch context to the specific commit context. That's what I was using before to test this PR, but it's no longer a valid test flow.

I'll try this again. 🤞

[jankeromnes]

Super cool! 🎉 The UX works flawlessly ✨ (especially when you test it correctly 😅)

(Side-note: I haven't tested for any side effects on multi-repo projects, but I guess this is fine since most of the risk is gated by the feature flag, and any accidental fall-out could be fixed quickly in production.)

[sagor999]

LGTM

/hold in case if there is more UX discussion needed prior to merge
feel free to remove hold when ready

[svenefftinge]

/unhold