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

Automatically restart from local devfile when devfile failed to be resolved at startup #22491

Closed
l0rd opened this issue Sep 6, 2023 · 12 comments
Labels
engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system. team/B This team is responsible for the Web Terminal, the DevWorkspace Operator and the IDEs.

Comments

@l0rd
Copy link
Contributor

l0rd commented Sep 6, 2023

Is your enhancement related to a problem? Please describe

There are situation where che-server is not able to resolve a devfile from a git repository using the git service API. A typical example is when it's not GitHub, GitLab, Bitbucket or Azure DevOps repository or when SSH URL is used.

Describe the solution you'd like

Project clone, after cloning the git repository, updates the DevWorkspace object if both conditions are met:

  • The env variable DEVFILE_RESOLUTION=failed is set
  • A devfile exist at the root of the main project

More information

This is part of this epic: #21951

@l0rd l0rd added the kind/enhancement A feature request - must adhere to the feature request template. label Sep 6, 2023
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Sep 6, 2023
@l0rd l0rd added severity/P1 Has a major impact to usage or development of the system. engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Sep 6, 2023
@l0rd l0rd added the team/B This team is responsible for the Web Terminal, the DevWorkspace Operator and the IDEs. label Oct 12, 2023
@amisevsk
Copy link
Contributor

One difficulty in this is that it appears that the dashboard is still modifying the underlying devfile in converting it to a DevWorkspace, so it's not clear how to patch the DevWorkspace from the project-clone container without pulling in a lot of Che-specific logic.

I believe dashboard could avoid this by injecting e.g. environment variables into the editor contribution DevWorkspaceTemplate instead (environment variables from contributions are merged into the main container).

I've created issues

For related tasks for this issue.

@amisevsk
Copy link
Contributor

PR devfile/devworkspace-operator#1193 should address the DWO-side of this issue. In testing these changes with Che, I found a few sticking points, however.

To test, my setup was

  • Basic Che install on OpenShift
  • Manually-created PAT secret in my user's namespace
  • Using a private test repository that contains the Che Dashboard's devfile

I found that #22614 isn't terribly important to getting this to work, as the injected environment variables, etc., are still injected into the end result because they're added to the contribution container as well (so the resulting "flattened" DevWorkspace is generally identical to the successfully-generated one).

To make it work, I had to work around the following issues manually (in addition to adding the new attribute):

  • When using a private repository in the dashboard, if a devfile cannot be resolved the workspace starts from the empty devfile with no projects added, so I had to manually add my project to the DevWorkspace after it was created
  • The DevWorkspace is named empty-<chars>, matching an empty-devfile workspace (it might make more sense to try extract a repository name)

@l0rd
Copy link
Contributor Author

l0rd commented Nov 23, 2023

@amisevsk can we close this one or is still something to check? Also this was the last subtask of Support reading devfiles from private repos other than GitHub/GitLab/Bitbucket/Azure · Issue #21951 · eclipse/che. Have you had the chance to test with a Git service that is not GitHub, Gitlab, Bitbucket and Azure Devops? cc @dmytro-ndp

@amisevsk
Copy link
Contributor

Regarding non-GitHub/Gitlab/Bitbucket/etc. repositories, the DWO-side changes should work for anything so long as the workspace is started with git credentials suitable for cloning from that repository. As for how to test it directly, I don't have any such repositories available, though.

At this point, we have a foundation we can potentially build off of (the controller.devfile.io/bootstrap-devworkspace: true attribute), but there's more to be done make this work for users automatically:

  • The big one is that something (e.g. the dashboard) needs to start using the attribute when appropriate; if we fail to resolve a devfile at startup, we need to start a workspace that uses the attribute and includes the project. This could just be a template based off the empty workspace:
    schemaVersion: 2.2.0
    metadata:
      name: <name>
    components:
      - name: universal-developer-image
        container:
          image: quay.io/devfile/universal-developer-image:ubi8-latest
    # ^^^ standard empty workspace devfile ^^^
    attributes:
      controller.devfile.io/bootstrap-devworkspace: true # Clone repo and start from internal devfile if available
    projects:
      - name: <specified project name>
        git:
          remotes:
            origin: <specified repository>
  • We also need to figure out a flow for when the bootstrap process fails -- what does this look like, and is it understandable to the user? E.g. if the credentials are invalid, the project will still not be cloned and the workspace won't be restarted; they'll just have the project-clone errors log.
  • There are some minor polish issues to work out (e.g. if we fail to resolve a devfile we name the workspace empty-xxxx instead of something based off the URL; if we have a git server that has to use this functionality it could mean having three empty-xxxx workspaces and no way to remember which one is which)

@l0rd
Copy link
Contributor Author

l0rd commented Nov 24, 2023

In the description of this issue I proposed to trigger try the restart from local devfile when the env variable DEVFILE_RESOLUTION=failed is found. We can use an attribute too if you prefer. But yes, dashboard should add this attribute if the resolution fails (and the DWO/cloner should remove it when it resolves the devfile?). The mechanism is in place DWO/cloner side right? If that's the case we need to create an issue for team B with a clear explanation on what project cloner currently expects.

If the process fails I am expecting the workspace startup to fail and the dashboard showing the error and a message as it does it for normal flow no?

For workspace naming: dashboard currently infers the name from the last string of the GIT+SSH URL (it doesn't name it "empty").

For testing @dmytro-ndp can help you with a gogs repository or you can get one at https://sr.ht/ in minutes

@amisevsk
Copy link
Contributor

amisevsk commented Nov 24, 2023

The issue with triggering the functionality based on an env var is that it's much harder to set and unset that environment variable in a workspace (which container do you put it on/which container do you eventually remove it from? DWO does not allow configuring project-clone environment variables currently). An attribute is much easier, as the option applies to the whole workspace rather than an individual container.

Currently, what DWO will do (on nightly builds) is edit the DevWorkspace to a) remove the attribute and b) overwrite the contents with the contents of the devfile, which results in the workspace restarting normally as though the dashboard had been able to resolve the devfile in the first place (except for some Che-specific env var handling as described above).

However, if the project cannot be cloned or no devfile can be found, the workspace will not fail to start. In this case, it's unclear how we should handle this on the DWO side:

  • If we fail to clone the repository, we don't know if there's a devfile to bootstrap from; we currently do not fail workspaces in this case though, so that users can debug why the clone failed in the first place.
  • If we succeed in cloning the repo but can't find a devfile, on the other hand, we maybe want to still run the workspace? This would be similar to how repositories with no devfile are handled currently.

The issue is that it's hard to distinguish the case [failed to do specific bootstrap process] from other failure cases (e.g. [cannot clone repo], [devfile does not exist for repo]).

@amisevsk
Copy link
Contributor

Regarding testing via sr.ht, I may be missing something but I don't see how to get a repo with a devfile there without paying for the service (which is something I don't necessarily want to do for the purpose of verifying this issue). However, the project clone container was able to clone the repository without problem, at which point devfile resolution should proceed as normal (i.e. no issues).

In this case -- cloning a sr.ht repo without a devfile in it -- devfile resolution (and therefore bootstrapping) failed. However, failing the workspace startup here would be the incorrect option in my opinion, as I still want to be able to be able to use the built-in flows for creating a devfile for this workspace (write a workspace in Code, restart from local devfile, etc.).

Also, in cloning this repository, my workspace was named empty-nopi -- maybe the devfile name inference isn't working in this case.

@l0rd
Copy link
Contributor Author

l0rd commented Nov 24, 2023

However, if the project cannot be cloned or no devfile can be found, the workspace will not fail to start. In this case, it's unclear how we should handle this on the DWO side

This is a problem no matter if the DW had the attribute bootstrap-devworkspace or not. I mean, in both cases the developer ends up with an empty workspace and should be informed that cloning failed: the fact that the devfile hasn't been resolved is not relevant in this case.

If we succeed in cloning the repo but can't find a devfile, on the other hand, we maybe want to still run the workspace? This would be similar to how repositories with no devfile are handled currently.

Sure, this is something I tried to specify in the description. The re-start should happen only if a devfile is found. And the missing devfile case should be considered ok (no need to restart) as we support repos with no devfile too.

@l0rd
Copy link
Contributor Author

l0rd commented Nov 24, 2023

For sourcehut: you should try the git+ssh, not the https URL. You can still register without https://meta.sr.ht/profile. I have created this repo with a devfile. But anyway, if you have issues there is Dmytro gogs instance.

@l0rd
Copy link
Contributor Author

l0rd commented Nov 24, 2023

2023-11-24 20 36 26

@amisevsk
Copy link
Contributor

Worth noting that the same mechanism for ssh+git URLs works for https URLs here (i.e. if we start using the DWO functionality, it would work for the sourcehut http URLs as well). This bootstrap process works for any URL you can pass to git clone.

Adding the attribute to the workspace after start (and then restarting manually) results in the workspace completing the bootstrap process and running using the provided devfile:

bootstrap-flow-2023-11-24_15.22.10.mp4

(For whatever reason, the UI wasn't able to grab the logs of the bootstrapping run of project-clone before the container finished running and updated the DevWorkspace)

@l0rd
Copy link
Contributor Author

l0rd commented Jan 15, 2024

Fixed with #22697

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system. team/B This team is responsible for the Web Terminal, the DevWorkspace Operator and the IDEs.
Projects
None yet
Development

No branches or pull requests

3 participants