che-operator icon indicating copy to clipboard operation
che-operator copied to clipboard

feat: configure workspace start timeout from Che Cluster CR

Open AObuchow opened this issue 3 years ago • 5 comments

What does this PR do?

This PR allows the configuration of the DWOC's progressTimeout field from the Che Cluster CR's spec.devEnvironments.startTimeout field.

Note: This PR is based off of https://github.com/eclipse-che/che-operator/pull/1565, which itself is based of off https://github.com/eclipse-che/che-operator/pull/1549. Both of those PRs had merge conflicts, which I have resolved in this PR.

However, https://github.com/eclipse-che/che-operator/pull/1565 depends on DWO 0.18.0 which has not been released yet. Thus, this PR cannot be merged until it is released (a modification to this PR will also be required to make it depend on DWO 0.18.0 instead of @dkwon17's fork).

This PR being blocked until DWO 0.18.0 should be okay, since https://github.com/eclipse/che/issues/21770 has a workaround and https://github.com/eclipse-che/che-operator/pull/1549 just improves the ease of doing container builds (which is already possible IIRC?). CC: @ibuziuk

Screenshot/screencast of this PR

n/a

What issues does this PR fix or reference?

eclipse/che#21770

How to test this PR?

Tests were added for this PR.

To manually test:

  1. Checkout this PR
  2. Deploy Che: build/scripts/olm/test-catalog-from-sources.sh (for OpenShift)
  3. Edit the Che Cluster CR and add the devEnvironments.startTimeout field: kubectl edit checluster eclipse-che -n eclipse-che
Spec:
(...)
  Dev Environments:
    Default Components:
      Container:
        Image:           quay.io/devfile/universal-developer-image:ubi8-38da5c2
        Source Mapping:  /projects
      Name:              universal-developer-image
    Default Editor:      che-incubator/che-code/insiders
    Default Namespace:
      Auto Provision:                      true
      Template:                            <username>-che
    Disable Container Build Capabilities:  true
    Seconds Of Inactivity Before Idling:   1800
    Seconds Of Run Before Idling:          -1
+    Start Timeout:                         7m
    Storage:
      Pvc Strategy:  per-user
  Git Services:
  Networking:
    Auth:
      Gateway:
        Config Labels:
          App:        che
          Component:  che-gateway-config
  1. Verify that config.workspace.progressTimeout in the DWOC has the same value as the devEnvironments.startTimeout field from the Che Cluster CR: kubectl describe dwoc -n eclipse-che
Name:         devworkspace-config
Namespace:    eclipse-che
Labels:       <none>
Annotations:  <none>
API Version:  controller.devfile.io/v1alpha1
Config:
  Workspace:
+    Progress Timeout:  7m
(...)
  1. Remove the devEnvironments.startTimeout field from the Che Cluster CR: kubectl edit checluster eclipse-che -n eclipse-che
Spec:
(...)
  Dev Environments:
    Default Components:
      Container:
        Image:           quay.io/devfile/universal-developer-image:ubi8-38da5c2
        Source Mapping:  /projects
      Name:              universal-developer-image
    Default Editor:      che-incubator/che-code/insiders
    Default Namespace:
      Auto Provision:                      true
      Template:                            <username>-che
    Disable Container Build Capabilities:  true
    Seconds Of Inactivity Before Idling:   1800
    Seconds Of Run Before Idling:          -1
-    Start Timeout:                         7m
    Storage:
      Pvc Strategy:  per-user
  Git Services:
  Networking:
    Auth:
      Gateway:
        Config Labels:
          App:        che
          Component:  che-gateway-config
  1. Verify that the config.workspace.progressTimeout from the DWOC is now gone: kubectl describe dwoc -n eclipse-che
Name:         devworkspace-config
Namespace:    eclipse-che
Labels:       <none>
Annotations:  <none>
API Version:  controller.devfile.io/v1alpha1
Config:
  Workspace:
Kind:  DevWorkspaceOperatorConfig
Metadata:
(...)

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

AObuchow avatar Dec 07 '22 20:12 AObuchow

Hi @AObuchow. Thanks for your PR.

I'm waiting for a eclipse-che member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci[bot] avatar Dec 07 '22 20:12 openshift-ci[bot]

This PR is set to be a draft until DWO 0.18.0 is released.

@tolusha if you would like to merge this PR (and the 2 other PR's included in it), it might be efficient to follow the testing steps from this PR, as well as https://github.com/eclipse-che/che-operator/pull/1549 and https://github.com/eclipse-che/che-operator/pull/1565 all at once.

If you would like to merge the PR's one-by-one, just ping me and I'll rebase my earlier PR :)

AObuchow avatar Dec 07 '22 20:12 AObuchow

Codecov Report

Merging #1576 (dfdcbaf) into main (149a39f) will decrease coverage by 0.01%. The diff coverage is 100.00%.

:exclamation: Current head dfdcbaf differs from pull request most recent head 9b02186. Consider uploading reports for the commit 9b02186 to get more accurate results

@@            Coverage Diff             @@
##             main    #1576      +/-   ##
==========================================
- Coverage   60.62%   60.61%   -0.02%     
==========================================
  Files          73       73              
  Lines        6316     6314       -2     
==========================================
- Hits         3829     3827       -2     
  Misses       2146     2146              
  Partials      341      341              
Impacted Files Coverage Δ
api/v2/checluster_types.go 30.43% <ø> (ø)
...eploy/dev-workspace-config/dev_workspace_config.go 91.37% <100.00%> (+0.81%) :arrow_up:
pkg/deploy/gateway/traefik_config_util.go 91.80% <0.00%> (-0.74%) :arrow_down:
controllers/devworkspace/solver/che_routing.go 79.36% <0.00%> (-0.06%) :arrow_down:
api/v2/zz_generated.deepcopy.go 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Dec 07 '22 20:12 codecov[bot]

@AObuchow Let's wait for DWO v0.18.0 and merge a single PR

tolusha avatar Dec 08 '22 09:12 tolusha

DWO updated to v0.18.0

tolusha avatar Dec 22 '22 13:12 tolusha

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: AObuchow, tolusha

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci[bot] avatar Dec 22 '22 13:12 openshift-ci[bot]

/test v11-operator-test

tolusha avatar Dec 23 '22 07:12 tolusha

New changes are detected. LGTM label has been removed.

openshift-ci[bot] avatar Dec 23 '22 21:12 openshift-ci[bot]

@tolusha I updated the PR to use startTimeoutSeconds as well as the PR description (note that this PR still includes the commits from https://github.com/eclipse-che/che-operator/pull/1549, which can be removed if desired.

https://github.com/eclipse-che/che-operator/pull/1587#discussion_r1056632088 might also be worth addressing in this PR as an extra commit.

AObuchow avatar Dec 23 '22 21:12 AObuchow

Also once this PR looks good to merge I can autosquash the fixup commits

AObuchow avatar Dec 23 '22 21:12 AObuchow

@tolusha sorry for the force-pushes, I tried to resolve the PR checks unsuccessfully. Not sure why make update-dev-resources is not giving the desired output, and make fmt results in no changes.

AObuchow avatar Dec 23 '22 21:12 AObuchow

@AObuchow

I've updated dev resources and added some annotations for startTimeoutSeconds field:

	// +optional
	// +kubebuilder:validation:Minimum:=1
	// +kubebuilder:default:=300

Do you have any concerns? I am wondering if minimum value can be 0.

I tried to resolve the PR checks unsuccessfully. Not sure why make update-dev-resources is not giving the desired output, and make fmt results in no changes.

It might be related to Golang and kustomize version , which was recently updated in https://github.com/eclipse-che/che-operator/pull/1586 So, pls clean up bin directly (new versions of utils will be downloaded automatically) and set up Go 1.18

tolusha avatar Dec 24 '22 08:12 tolusha

/test v11-devworkspace-happy-path

tolusha avatar Dec 26 '22 09:12 tolusha

@AObuchow: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/v11-devworkspace-happy-path 90314e52a38ef058b7a4cb26e35dab87d64bdada link true /test v11-devworkspace-happy-path

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci[bot] avatar Dec 26 '22 11:12 openshift-ci[bot]

Build 3.5 :: push-latest-container-to-quay_3.x/1419: SUCCESS

Copied: devspaces-rhel8-operator; /job/DS_CI/job/update-digests_3.x triggered;
/job/DS_CI/job/Releng/job/copyIIBsToQuay triggered for OCP v4.12 v4.11 v4.10

devstudio-release avatar Dec 28 '22 08:12 devstudio-release

Build 3.5 :: sync-to-downstream_3.x/1843: SUCCESS

Build container: devspaces-operator synced; /DS_CI/get-sources-rhpkg-container-build_3.x/1798 triggered;

devstudio-release avatar Dec 28 '22 08:12 devstudio-release

Build 3.5 :: operator_3.x/160: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.x/1843 triggered

devstudio-release avatar Dec 28 '22 08:12 devstudio-release

Build 3.5 :: copyIIBsToQuay/536: SUCCESS

arches = x86_64, s390x, ppc64le;
  * LATEST DS OPERATOR BUNDLE = <a href=https://quay.io/repository/devspaces/devspaces-operator-bundle?tab=tags>registry-proxy.engineering.redhat.com/rh-osbs/devspaces-operator-bundle:3.5-22
  * LATEST DWO OPERATOR BUNDLE = <a href=https://quay.io/repository/devworkspace/devworkspace-operator-bundle?tab=tags>registry-proxy.engineering.redhat.com/rh-osbs/devworkspace-operator-bundle:0.18-1
+ s390x-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.5-v4.12-404486-402447-s390x
  + quay.io/devspaces/iib:3.5-v4.12-s390x
  + quay.io/devspaces/iib:next-v4.12-s390x
  + quay.io/devspaces/iib:3.5-v4.11-404485-402443-s390x
  + quay.io/devspaces/iib:3.5-v4.11-s390x
  + quay.io/devspaces/iib:next-v4.11-s390x
  + quay.io/devspaces/iib:3.5-v4.10-404484-402439-s390x
  + quay.io/devspaces/iib:3.5-v4.10-s390x
  + quay.io/devspaces/iib:next-v4.10-s390x
+ x86_64-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.5-v4.12-404486-402447-x86_64
  + quay.io/devspaces/iib:3.5-v4.12-x86_64
  + quay.io/devspaces/iib:next-v4.12-x86_64
  + quay.io/devspaces/iib:3.5-v4.11-404485-402443-x86_64
  + quay.io/devspaces/iib:3.5-v4.11-x86_64
  + quay.io/devspaces/iib:next-v4.11-x86_64
  + quay.io/devspaces/iib:3.5-v4.10-404484-402439-x86_64
  + quay.io/devspaces/iib:3.5-v4.10-x86_64
  + quay.io/devspaces/iib:next-v4.10-x86_64
  * LATEST DS OPERATOR BUNDLE = <a href=https://quay.io/repository/devspaces/devspaces-operator-bundle?tab=tags>registry-proxy.engineering.redhat.com/rh-osbs/devspaces-operator-bundle:3.5-23
+ ppc64le-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.5-v4.12-404486-402447-ppc64le
  + quay.io/devspaces/iib:3.5-v4.12-ppc64le
  + quay.io/devspaces/iib:next-v4.12-ppc64le
  + quay.io/devspaces/iib:3.5-v4.11-404485-402443-ppc64le
  + quay.io/devspaces/iib:3.5-v4.11-ppc64le
  + quay.io/devspaces/iib:next-v4.11-ppc64le
  + quay.io/devspaces/iib:3.5-v4.10-404484-402439-ppc64le
  + quay.io/devspaces/iib:3.5-v4.10-ppc64le
  + quay.io/devspaces/iib:next-v4.10-ppc64le

devstudio-release avatar Dec 28 '22 09:12 devstudio-release