-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add persistent volume support for workspaces #9242
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1221,6 +1221,12 @@ func startContentInit(ctx context.Context, cfg *Config, wg *sync.WaitGroup, cst | |
}() | ||
|
||
fn := "/workspace/.gitpod/content.json" | ||
fnReady := "/workspace/.gitpod/ready" | ||
if _, err := os.Stat("/.workspace/.gitpod/content.json"); !os.IsNotExist(err) { | ||
fn = "/.workspace/.gitpod/content.json" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To make it explicit that these paths come from a PVC mount, why not make it refer to a variable shared with the following location? This would surely help the code reader. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we will be switching default to always use |
||
fnReady = "/.workspace/.gitpod/ready" | ||
log.Info("Detected content.json in /.workspace folder, assuming PVC feature enabled") | ||
} | ||
f, err := os.Open(fn) | ||
if os.IsNotExist(err) { | ||
log.WithError(err).Info("no content init descriptor found - not trying to run it") | ||
|
@@ -1230,7 +1236,7 @@ func startContentInit(ctx context.Context, cfg *Config, wg *sync.WaitGroup, cst | |
// TODO: rewrite using fsnotify | ||
t := time.NewTicker(100 * time.Millisecond) | ||
for range t.C { | ||
b, err := os.ReadFile("/workspace/.gitpod/ready") | ||
b, err := os.ReadFile(fnReady) | ||
if err != nil { | ||
if !os.IsNotExist(err) { | ||
log.WithError(err).Error("cannot read content ready file") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,11 @@ message InitWorkspaceRequest { | |
|
||
// storage_quota_bytes enforces a storage quate for the workspace if set to a value != 0 | ||
int64 storage_quota_bytes = 8; | ||
|
||
// persistent_volume_claim means that we use PVC instead of HostPath to mount /workspace folder and content-init | ||
// happens inside workspacekit instead of in ws-daemon. We also use k8s Snapshots to store\restore workspace content | ||
// instead of GCS\tar. | ||
bool persistent_volume_claim = 9; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about using enumerations instead of using bool? e.g. VolumeType, StorageType etc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this is applicable here, since this bool flag is used as feature flag. Once PVC feature is stable and working, we will remove old code and remove this feature flag and this new code will become default code path. |
||
} | ||
|
||
// WorkspaceMetadata is data associated with a workspace that's required for other parts of the system to function | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
module github.com/gitpod-io/generated_code_dependencies | ||
|
||
go 1.18 | ||
|
||
require google.golang.org/protobuf v1.28.0 // indirect |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= | ||
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= | ||
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= | ||
google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= | ||
google.golang.org/protobuf v1.28.0 h1:w43yiav+6bVFTBQFZX0r7ipe9JQ1QsbMgHwbBziscLw= | ||
google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOTE: we could refactor these functions
GetContentLayer
andGetContentLayerPVC
into one in the future.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. Once PVC is stable and we switched over to it, old code will be removed. (so that we will have only one version of that function)