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.

Sign up for GitHub

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

Jump to bottom

Support devWorkspace.ignoredUnrecoverableEvents. #1864

Merged
merged 1 commit into from
Jul 4, 2024
Merged

Support devWorkspace.ignoredUnrecoverableEvents. #1864

merged 1 commit into from
Jul 4, 2024

Conversation

monaka
Copy link
Member

@monaka monaka commented Jun 27, 2024 •
edited
Loading

What does this PR do?

Adds devWorkspace.ignoredUnrecoverdableEvents to CheCluster CR.
Che workspaces will become to run on clusters with a node autoscaler like Cluster Autoscaler, Karpenter .

Screenshot/screencast of this PR

image

The workspace pod is waiting for a new node by cluster autoscaler.

What issues does this PR fix or reference?

eclipse-che/che#22598

How to test this PR?

  1. Prepare a patch file if needed:
cat > /tmp/cr-patch.yaml <<EOF
apiVersion: org.eclipse.che/v2
kind: CheCluster
spec:
  devEnvironments:
    ignoredUnrecoverableEvents:
    - FailedScheduling
    - FailedMount
EOF
  1. Deploy the operator:

OpenShift

./build/scripts/olm/test-catalog-from-sources.sh --cr-patch-yaml /tmp/cr-patch.yaml

on Minikube

./build/scripts/minikube-tests/test-operator-from-sources.sh --cr-patch-yaml /tmp/cr-patch.yaml

But I suppose this PR is hard to test on Minikube as it won't have any cluster autoscaler.

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.

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jun 27, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@monaka monaka force-pushed the pr-add-ignoredUnrecoverableEvents branch from f820abd to c62ac10 Compare June 27, 2024 07:07
@monaka monaka marked this pull request as ready for review June 27, 2024 08:32
@tolusha
Copy link
Contributor

tolusha commented Jun 27, 2024

/cc @dkwon17

@openshift-ci openshift-ci bot requested a review from dkwon17 June 27, 2024 08:40
@dkwon17
Copy link
Contributor

dkwon17 commented Jun 27, 2024

Hello @monaka, thank you for the PR, could you please rebase and rerun make update-dev-resources ?

AObuchow
AObuchow reviewed Jun 27, 2024
View reviewed changes
api/v2/checluster_types.go Outdated
@@ -189,6 +189,13 @@ type CheClusterDevEnvironments struct {
// +optional
// +kubebuilder:validation:Enum=Always;IfNotPresent;Never
ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"`
// IgnoredUnrecoverableEvents defines a list of Kubernetes event names that should
// be ignored when deciding to fail a DevWorkspace startup. This option should be used
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be mistaken, but I think we just refer to workspaces instead of DevWorkspaces from a Che perspective? i.e. when deciding to fail a DevWorkspace startup -> when deciding to fail a workspace that is starting.

Same thing for Events listed here will not trigger DevWorkspace failures. -> Events listed here will not trigger workspace failures.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I fixed comments as you suggested.

AObuchow
AObuchow reviewed Jun 27, 2024
View reviewed changes
pkg/deploy/dev-workspace-config/dev_workspace_config.go Outdated
@@ -194,6 +196,14 @@ func updateWorkspaceImagePullPolicy(imagePullPolicy corev1.PullPolicy, workspace
workspaceConfig.ImagePullPolicy = string(imagePullPolicy)
}

func updateIgnoredUnrecoverableEvents(operatorConfig *controllerv1alpha1.OperatorConfiguration, ignoredUnrecoverableEvents []string) {
if ignoredUnrecoverableEvents == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this nil check? Can't we just do operatorConfig.Workspace.IgnoredUnrecoverableEvents = ignoredUnrecoverableEvents?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I removed the nil check.

@AObuchow
Copy link
Contributor

@monaka Great PR so far :) Be sure to please add some tests as well. I recommend checking out the tests for podAnnotations as an example

@monaka monaka force-pushed the pr-add-ignoredUnrecoverableEvents branch 2 times, most recently from d8a134d to 3b37782 Compare July 2, 2024 05:17
@monaka monaka force-pushed the pr-add-ignoredUnrecoverableEvents branch 3 times, most recently from eb12927 to f605695 Compare July 2, 2024 05:39
@monaka monaka marked this pull request as draft July 2, 2024 05:47
@monaka monaka force-pushed the pr-add-ignoredUnrecoverableEvents branch from f605695 to 77f709b Compare July 2, 2024 06:05
@monaka monaka force-pushed the pr-add-ignoredUnrecoverableEvents branch from 77f709b to 925feed Compare July 2, 2024 06:21
@monaka monaka marked this pull request as ready for review July 2, 2024 06:38
@monaka monaka requested a review from AObuchow July 2, 2024 06:38
@monaka
Copy link
Member Author

monaka commented Jul 2, 2024

/test v14-devworkspace-happy-path

@devstudio-release
Copy link

Build 3.16 :: sync-to-downstream_3.x/7060: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7038: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7038: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 62378966 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devstudio-release
Copy link

Build 3.16 :: operator-bundle_3.x/3182: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: sync-to-downstream_3.x/7061: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7039: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7039: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 62379453 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devstudio-release
Copy link

Build 3.16 :: operator-bundle_3.x/3184: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: sync-to-downstream_3.x/7064: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7042: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7042: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 62381553 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devstudio-release
Copy link

Build 3.16 :: operator-bundle_3.x/3185: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: sync-to-downstream_3.x/7066: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7044: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7044: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 62382559 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devstudio-release
Copy link

Build 3.16 :: operator-bundle_3.x/3186: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: sync-to-downstream_3.x/7067: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7045: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7045: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 62382628 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devstudio-release
Copy link

Build 3.16 :: operator-bundle_3.x/3187: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: sync-to-downstream_3.x/7068: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7046: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7046: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 62385338 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devstudio-release
Copy link

Build 3.16 :: operator-bundle_3.x/3188: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: sync-to-downstream_3.x/7069: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7047: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.16 :: get-sources-rhpkg-container-build_3.x/7047: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 62388162 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@ibuziuk
Copy link
Member

ibuziuk commented Jul 25, 2024

@monaka thank you for the contribution, new feature is part of the RN - https://twitter.com/eclipse_che/status/1816081779607928954

@monaka monaka mentioned this pull request Aug 4, 2024
Closed
@AObuchow AObuchow mentioned this pull request Aug 22, 2024
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AObuchow AObuchow AObuchow approved these changes

@tolusha tolusha tolusha approved these changes

@SDawley SDawley Awaiting requested review from SDawley SDawley is a code owner

@ibuziuk ibuziuk Awaiting requested review from ibuziuk ibuziuk is a code owner

@dkwon17 dkwon17 Awaiting requested review from dkwon17

Assignees

@tolusha tolusha

@AObuchow AObuchow

Labels
lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@monaka @tolusha @dkwon17 @AObuchow @devstudio-release @ibuziuk @openshift-merge-robot