[go: up one dir, main page]
Skip to content

Improve handling of workspace persistent volume mount

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

MR: Pending

Description

This issue is based on this comment

We need to step back and think about how we are handling these persistent volumes, project mounts, and directories.

We need to get this correct, and there is an urgency, because the longer we go the more people (and ourselves) will hardcode dependencies on these paths and locations.

Here is an example of us doing this ourselves, hardcoding the path in a way which would break if we renamed the persistent volume mount from /projects to /workspace: https://gitlab.com/gitlab-com/dev-on-call/-/merge_requests/46#note_2643313816

It's very likely there's lots of other existing scripts which would break also.

Furthermore, the /projects path is actually part of the devfile standard, as a default directory: https://devfile.io/docs/2.0.0/adding-container-component

"The sources is mounted on a location stored in the PROJECTS_ROOT environment variable that is made available in the running container of the image. This location defaults to /projects. If sourceMapping is defined in the container, it overrides the PROJECT_ROOT value if present and mounts the source to the path defined by sourceMapping."

And here's the docs on PROJECTS_ROOT and PROJECTS_SOURCE (https://devfile.io/docs/2.3.0/devfile-schema#commands-exec):

Special variables that can be used:

  • $PROJECTS_ROOT: A path where projects sources are mounted as defined by container component's sourceMapping.
  • $PROJECT_SOURCE: A path to a project source ($PROJECTS_ROOT/). If there are multiple projects, this will point to the directory of the first one.

See also:

Also note that this is also causing related confusion where we are not changing to the project directory by default - see this comment on postStart commands should run in project directory (#555948 - closed)

We should change the default directory. BUT...

However, we don't seem to implement this correctly according to the standard currently.

This is because we don't provide any provided ENV var pointing to the actual clone directory, which users can properly use, even if they wanted to make their scripts path-independent.

Specifically, we seem to be incorrectly setting PROJECT_SOURCE. We currently just point this to the same location as PROJECT_SOURCE, which is the /projects root. See https://gitlab.com/gitlab-org/gitlab/blob/23379a69f2e3a527afd12c1eae549d381b28393c/ee/spec/support/shared_contexts/remote_development/remote_development_shared_contexts.rb#L677-684:

                  {
                    name: "PROJECTS_ROOT",
                    value: workspace_operations_constants_module::WORKSPACE_DATA_VOLUME_PATH
                  },
                  {
                    name: "PROJECT_SOURCE",
                    value: workspace_operations_constants_module::WORKSPACE_DATA_VOLUME_PATH
                  }

But based on the above devfile standard docs, since we are cloning to /projects/<clone directory>, PROJECT_SOURCE should really be set to /projects/<cloned directory>

And we should have documentation to indicate users should use these ENV vars

Furthermore, we don't seem to be handling at all the devfile standards for mountSources and sourceMapping.

Either we should add proper support for these, or explicitly disable them in the devfile validation.

So, this leads back to the question of where we stick all the "other" stuff that we want to be on a persistent volume, such as user-specific content in XDG_* vars, as referred to in gitlab-build-images!983 (comment 2618725341) on Add workspace base image (gitlab-build-images!983 - merged) • zli • 18.3

One thing that would help is to have the HOME directory persistent by now to put some of this stuff such as user data. But we have deprioritized the support for this (Workspace changes (user level settings) are los... (&15769)). So we continue to have to work around it with approaches such as explicitly setting XDG_* and pointing them to some directory. Even though our metrics say that people don't usually restart workspaces, we still get some complaints/bug reports that data is lost upon workspace restarts (e.g. installed extensions).

But even a persistent HOME wouldn't address all of the above concerns, because we currently dump other stuff in /projects like our log files, that isn't a "project" at all.

So we need to deal with this more holistically, and ideally in a way which is compatible with both devfile and devcontainer standards.

As another data point, see how the devcontainer standard handles this: https://containers.dev/implementors/json_reference/. Search that page for references to WorkspaceFolder, and you'll see it has the assumption that there's a single folder that all stuff related to the "workspace" is stored.

On CodeSpaces, this dir is /workspaces.

PROPOSAL

SO, given all the above, here's what I propose to fix this, with minimal effort, and hopefully no chance of breaking people with scripts that currently depend on the hardcoded /projects path.

  1. Get the persistent DesiredConfig done and merged, so we can change the kubernetes resources without causing restarts: Persist desired kubernetes configuration on wor... (#541907 - closed) (cc @ashvins )
  2. Make an MR which:
    1. Changes WORKSPACE_DATA_VOLUME_PATH from projects to workspace - to reflect the fact that it's not just projects, but everything related to the workspace, like the devcontainer standard encourages
    2. Clones the project to /workspace/<cloned directory>. This matches what Codespaces does, so it should be familiar and help make things smooth with customers who may be migrating.
    3. Sets PROJECTS_ROOT to /workspace - this should be backward compatible, if anyone was referring to it.
    4. Sets PROJECT_SOURCE to point to /workspace/<cloned directory>.
    5. Add a new internal poststart script, as the first one, before INTERNAL_POSTSTART_COMMAND_CLONE_PROJECT_SCRIPT, which _adds a symlink of /projects which points to /workspace. This should prevent breakage of any scripts which hardcode a path to point to /projects. This symlink will eventually be deprecated.
    6. Add devfile validation to explicitly disallow usage of mountSources and sourceMapping until we support them. (this has been pulled out to a separate issue: Add devfile validation to explicitly disallow u... (#557221) • Omar Nasser • Backlog)
    7. Add logic to the postStart commands to change the default PWD to PROJECT_SOURCE. This is what CodeSpaces does, and addresses the concerns which are currently being raised. This would be somewhere in ee/lib/remote_development/workspace_operations/create/desired_config/scripts_configmap_appender.rb or one of its scripts. This could potentially be a breaking change if anyone depends on default directory, but that's why we should feature flag this.
  3. Put all of this behind a group-level actor feature flag, just in case it causes some unexpected breakages, we can disable it per group on .com and on-prem admins can disable it completely until they fix their references.
  4. Document all of the above clearly in user-facing docs, especially, that PROJECT_SOURCE can be used to refer the location the project is cloned instead of hardcoding paths

References

Acceptance criteria

  • Changes WORKSPACE_DATA_VOLUME_PATH from projects to workspace - to reflect the fact that it's not just projects, but everything related to the workspace, like the devcontainer standard encourages
  • Clones the project to /workspace/<cloned directory>. This matches what Codespaces does, so it should be familiar and help make things smooth with customers who may be migrating.
  • Sets PROJECTS_ROOT to /workspace - this should be backward compatible, if anyone was referring to it.
  • Sets PROJECT_SOURCE to point to /workspace/<cloned directory>.
  • PROJECT_SOURCE and PROJECT_ROOT should NOT be override-able or configurable by users in the devfile. See discussion on #557035 (comment 2652468930)
  • Add a new internal poststart script, as the first one, before INTERNAL_POSTSTART_COMMAND_CLONE_PROJECT_SCRIPT, which _adds a symlink of /projects which points to /workspace. This should prevent breakage of any scripts which hardcode a path to point to /projects. This symlink will eventually be deprecated.
  • Add devfile validation to explicitly disallow usage of mountSources and sourceMapping until we support them. (this has been pulled out to a separate issue: Add devfile validation to explicitly disallow u... (#557221) • Omar Nasser • Backlog)
  • Add logic to the postStart commands to change the default PWD to PROJECT_SOURCE. This is what CodeSpaces does, and addresses the concerns which are currently being raised.

Implementation plan

See Description and Acceptance Criteria

Edited by 🤖 GitLab Bot 🤖