Skip to content

[content-service] add prestop hook to extract git status #9807

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
May 10, 2022

Conversation

sagor999
Copy link
Contributor

@sagor999 sagor999 commented May 5, 2022

Description

add prestop hook to extract git status when workspace is using pvc.
This is needed when workspace is using PVC and ws-daemon cannot access those files directly because:
they are on a dedicated pvc
finalize happens when container is stopped, and at that point pvc has been detached.

So we use prestop hook to do necessary work right before container will receive signal to stop (later prebuilds will extract build log in the same way for example).

Related Issue(s)

Part of #9475

How to test

You can test this as part of #9475
But for this PR, verify that workspace starts and stops normally. This PR shouldn't affect normal workflow (without PVC) and that is what should be tested in this PR.

Release Notes

none

Documentation

@sagor999 sagor999 requested a review from a team May 5, 2022 22:26
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label May 5, 2022
@sagor999 sagor999 force-pushed the pavel/prestophook branch from 004b142 to 42f92fd Compare May 5, 2022 22:28
@roboquat roboquat added size/L and removed size/M labels May 5, 2022
layers := []fileInLayer{
{&tar.Header{Typeflag: tar.TypeDir, Name: "/.workspace", Uid: initializer.GitpodUID, Gid: initializer.GitpodGID, Mode: 0755}, nil},
{&tar.Header{Typeflag: tar.TypeDir, Name: "/.workspace/.gitpod", Uid: initializer.GitpodUID, Gid: initializer.GitpodGID, Mode: 0755}, nil},
{&tar.Header{Typeflag: tar.TypeReg, Name: "/.supervisor/prestophook.sh", Uid: 0, Gid: 0, Mode: 0775, Size: int64(len(prestophookScript))}, []byte(prestophookScript)},
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please explain why we are setting Uid and Gid to 0 but then permission to 0775?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uid/Gid to 0 to make sure no one else can edit\modify this file from inside the workspace (otherwise you can execute things from ring1).
775 to allow gitpod user to execute it as part of prestop hook.

@utam0k
Copy link
Contributor

utam0k commented May 9, 2022

Nice PR! I have a question if I use the stash command, this stash's information will disappear next time?

@sagor999 sagor999 force-pushed the pavel/prestophook branch from 42f92fd to 2cb0604 Compare May 9, 2022 17:52
@roboquat roboquat added size/XL and removed size/L labels May 9, 2022
@sagor999 sagor999 requested review from princerachit and jenting May 9, 2022 17:52
@jenting jenting requested a review from a team May 10, 2022 01:39
@princerachit
Copy link
Contributor

/hold

I am ok to merge this PR. @sagor999 please remove the hold label when you think it is ok to merge. I have added this as there were other reviewers who left comments.

@sagor999
Copy link
Contributor Author

/unhold

@roboquat roboquat merged commit 08acca8 into main May 10, 2022
@roboquat roboquat deleted the pavel/prestophook branch May 10, 2022 13:57
@sagor999 sagor999 removed the request for review from jenting May 10, 2022 13:57
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note-none size/XL team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants