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

feat(proposal): runonce scripts, eqivalent to .bashrc.d but runs only once without any race conditions between several terminals #1104

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

axonasif
Copy link
Member

@axonasif axonasif commented Jun 23, 2023

Description

As IDEs start without loading the login shell for quick startup, previous assumptions about race condition between .bashrc.d startup scripts are not ideal anymore. This introduces a new directory alongside .bashrc.d called .runonce. Scripts placed inside $HOME/.runonce will be executed once in the lifecycle of a workspace, and all task terminals will wait for .runonce scripts to complete before proceeding to execute the .gitpod.yml task commands. While scripts placed inside $HOME/.bashrc.d are used for modifying the shell environment (e.g. exporting env vars) each time a terminal is created and not for starting services.

Currently Proposed
r1 r2(2)

This PR only migrates the VNC startup to .runonce. I plan to create a followup PR to migrate rust, dotnet, mysql, postgres, vnc and yugabyte startup scripts as well (see axonasif@7907f57).

Related Issue(s)

Fixes #1067, gitpod-io/gitpod#18491, slack report, frontapp ticket.

How to test

Open https://gitpod.io/#https://github.com/axonasif/test/tree/vnc_test and check if all task terminals are attempting to start the vnc service.

Or if you want to emulate the race-condition environment from your existing workspace:

# Creates five bash login shells at the same time
docker run -e GITPOD_REPO_ROOT -v /workspace:/workspace -it axonasif/workspace-base-vnc:latest sh -c 'for i in 1 2 3 4 5; do bash -lic "echo Hello from BASH process $i" & done; wait'

Documentation

/hold

@axonasif axonasif changed the title Run VNC startup script once feat: runonce scripts, eqivalent to .bashrc.d but runs only once without any race conditions between several terminals Jun 23, 2023
@axonasif axonasif changed the title feat: runonce scripts, eqivalent to .bashrc.d but runs only once without any race conditions between several terminals feat(proposal): runonce scripts, eqivalent to .bashrc.d but runs only once without any race conditions between several terminals Jun 23, 2023
@axonasif axonasif marked this pull request as ready for review June 23, 2023 12:55
@axonasif axonasif requested review from a team June 23, 2023 12:55
@iQQBot
Copy link
Contributor

iQQBot commented Jul 5, 2023

Overall it looks good, but it seems we've added too many tricks to bashrc that don't seem regular

@kylos101
Copy link
Collaborator

kylos101 commented Jul 7, 2023

Overall it looks good, but it seems we've added too many tricks to bashrc that don't seem regular

@iQQBot would it make sense to sync with @axonasif , to talk about next steps?

@axonasif I'm not sure if the IDE team planned on this work? suggestion: flip this back to draft, and plan next steps with @iQQBot in the context of a Linear issue, in the IDE team, but assigned to @axonasif. cc: @laushinka

Copy link
Collaborator

@kylos101 kylos101 left a comment

Choose a reason for hiding this comment

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

Deferring to @iQQBot for now, cc @laushinka

edit: please re-request my review, or team workspace, as needed

@laushinka
Copy link
Contributor

if the IDE team planned on this work? suggestion: flip this back to draft, and plan next steps with @iQQBot in the context of a Linear issue, in the IDE team, but assigned to @axonasif

Thanks for mentioning, Kyle!

@axonasif Thank you for the initiative 👍🏽 As per Kyle's suggestion, Asif would you mind putting the issue to the IDE team's Triage?
@iQQBot let's bring it up in team sync or teaming. I'd like to understand more, e.g. the tradeoffs of this approach.

@axonasif axonasif marked this pull request as draft July 10, 2023 11:20
@axonasif
Copy link
Member Author

axonasif commented Jul 10, 2023

Thanks for all the input 🙏

As per Kyle's suggestion, Asif would you mind putting the issue to the IDE team's Triage?

Yes, (IDE-219) 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tool-vnc (workspace-full-vnc) needs some patches for adapting with new Ubuntu Jammy
4 participants