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

Allow configuring workspace start timeout via CheCluster spec #21820

Closed
amisevsk opened this issue Nov 16, 2022 · 4 comments
Closed

Allow configuring workspace start timeout via CheCluster spec #21820

amisevsk opened this issue Nov 16, 2022 · 4 comments
Assignees
Labels
area/devworkspace-operator kind/enhancement A feature request - must adhere to the feature request template. new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes severity/P2 Has a minor but important impact to the usage or development of the system. status/release-notes-review-needed Issues that needs to be reviewed by the doc team for the Release Notes wording
Milestone

Comments

@amisevsk
Copy link
Contributor

amisevsk commented Nov 16, 2022

Is your enhancement related to a problem? Please describe

Since workspace start is managed by the DevWorkspace Operator, the timeout before a starting workspace is marked as failed is now configured through the DevWorkspace Operator Config (DWOC) CR, deprecating older Che properties.

For certain CheCluster fields (e.g. PVC configuration), the Che Operator manages these fields by creating a custom DWOC that is used for all workspaces. This should be extended to allow setting .config.workspace.progressTimeout in the DWOC based on a CheCluster field as well.

Describe the solution you'd like

Start timeout for DevWorkspaces can be configured through the CheCluster spec

Describe alternatives you've considered

It's currently possible to work around this by setting the field in the CheCluster-owned DWOC in the install namespace. As this field is not managed by Che, its value is not reverted by the Che Operator.

Release Notes Text

The timeout to consider workspaces startup as failed was hardcoded and could not be configured by administrators. With this new feature Che administrators can specify the workspaces startup timeout in the CheCluster custom resource.

@amisevsk amisevsk added the kind/enhancement A feature request - must adhere to the feature request template. label Nov 16, 2022
@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 Nov 16, 2022
@AObuchow AObuchow mentioned this issue Nov 17, 2022
68 tasks
@Kasturi1820 Kasturi1820 added severity/P2 Has a minor but important impact to the usage or development of the system. area/devworkspace-operator and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Nov 18, 2022
@tolusha
Copy link
Contributor

tolusha commented Nov 29, 2022

I think startTimeout field is not introduced in operator config yeat.

@amisevsk
Copy link
Contributor Author

My bad -- the field in DWOC is progressTimeout (i.e. .config.workspace.progressTimeout) since it applies to both starting and failing workspaces. I've updated the description above to reflect this.

Probably makes sense to use startTimeout in the Che config either way -- WDYT?

@tolusha
Copy link
Contributor

tolusha commented Nov 30, 2022

Probably makes sense to use startTimeout in the Che config either way -- WDYT?

Make sense for me as well.

@AObuchow
Copy link

AObuchow commented Dec 6, 2022

For this feature, I'm not sure if I should make it so that the DWOC field workspace.progressTimeout is overridden even if the corresponding Che Cluster CR field is not set. The outcome of this would be that leaving the Che Cluster field blank would cause DWO to use the default progress timeout value of 5 minutes.

i.e. Che Cluster CR devEnvironments.startTimeout = "" (unset) -> sets .config.workspace.progressTimeout = "" -> default progress timeout value of 5m is used.

The only affected users from this would be those who are using the workaround @amisevsk gave:

It's currently possible to work around this by setting the field in the CheCluster-owned DWOC in the install namespace. As this field is not managed by Che, its value is not reverted by the Che Operator.

If these users had set a non-default progress timeout value, and then upgrade Che (once my patch is merged), the progress timeout value in the DWOC would be reset to 5 minutes, and they would have to modify the timeout field in the Che Cluster CR.

I don't think this is really a breaking change (since it only affects the workaround), but it may be worth considering.

CC: @ibuziuk

AObuchow added a commit to AObuchow/che-operator that referenced this issue Dec 6, 2022
AObuchow added a commit to AObuchow/che-operator that referenced this issue Dec 7, 2022
@ibuziuk ibuziuk mentioned this issue Dec 13, 2022
82 tasks
@tolusha tolusha closed this as completed Dec 28, 2022
@tolusha tolusha added this to the 7.59 milestone Dec 28, 2022
@l0rd l0rd added new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes status/info-needed More information is needed before the issue can move into the “analyzing” state for engineering. status/release-notes-review-needed Issues that needs to be reviewed by the doc team for the Release Notes wording and removed status/info-needed More information is needed before the issue can move into the “analyzing” state for engineering. labels Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devworkspace-operator kind/enhancement A feature request - must adhere to the feature request template. new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes severity/P2 Has a minor but important impact to the usage or development of the system. status/release-notes-review-needed Issues that needs to be reviewed by the doc team for the Release Notes wording
Projects
None yet
Development

No branches or pull requests

6 participants