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

Avoid setting defaults in CRD for fields where the default might change #22063

Closed
amisevsk opened this issue Mar 16, 2023 · 2 comments
Closed
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator area/dashboard kind/bug Outline of a bug - must adhere to the bug report template. severity/P1 Has a major impact to usage or development of the system.
Milestone

Comments

@amisevsk
Copy link
Contributor

Describe the bug

The Che Operator defines default values in its CheCluster CRD, even though some values are not constant and may change over time. For example, .devEnvironments.defaultComponents is defined with the default value (see here)

defaultComponents:
- container:
    image: quay.io/devfile/universal-developer-image:ubi8-38da5c2
  name: universal-developer-image

(ref: CRD)

This has the effect of fixing these fields at CheCluster creation time; CRD default values tell the cluster to automatically fill unset fields with the default. Once the fields are filled with the defaults, these fields are not updated by new defaults.

Since the CheCluster is intended to be long-lived over multiple Eclipse Che versions, keeping the defaults from when the CheCluster was created is likely unintentional behavior.

Che version

7.61@latest

Steps to reproduce

  1. Create a CheCluster without setting .devEnvironments.defaultComponents
  2. Check that .devEnvironments.defaultComponents is populated with default value
  3. Update Che to the next version (with a new default value)
  4. Check that existing CheCluster does not get updated .devEnvironments.defaultComponents

Expected behavior

Default values that may change with future versions of Che should use defaults stored in the operator itself, with the default value being used when the CheCluster field is unset. This would mean

  • Users are not required to specify values for these fields
  • Default values can be updated as required
  • It's possible to distinguish between actual values provided by the admin and values defaulted at install time.

Runtime

other (please specify in additional context)

Screenshots

No response

Installation method

other (please specify in additional context)

Environment

other (please specify in additional context)

Eclipse Che Logs

No response

Additional context

Related to issue #21979

This bug can potentially lead to hard-to-diagnose issues in the future -- for example, if a user installed Che before container build support was added, podman build may fail in an empty workspace even after the user has upgraded to a version of Che that supports container build. In trying to reproduce the issue, a new install of Che might not reproduce the issue due to an updated default.

For CheClusters that have fields set by defaults, the only workaround is to delete the fields from the CheCluster manually.

@amisevsk amisevsk added kind/bug Outline of a bug - must adhere to the bug report template. area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator area/install Issues related to installation, including offline/air gap and initial setup labels Mar 16, 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 Mar 16, 2023
@tolusha
Copy link
Contributor

tolusha commented Mar 17, 2023

That's mostly about spec.devEnvironments defaults.

  • defaultEditor & defaultComponents are not consumed by che-operator. They are needed for dashboard, so the defaults values can be easily moved to dashboard component.

@tolusha tolusha mentioned this issue Mar 17, 2023
50 tasks
@amisevsk amisevsk added severity/P1 Has a major impact to usage or development of the system. area/dashboard and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Mar 17, 2023
@tolusha tolusha removed the area/install Issues related to installation, including offline/air gap and initial setup label Mar 20, 2023
@tolusha
Copy link
Contributor

tolusha commented Mar 30, 2023

Some DevEnvironemnts defaults moved from CheCluster to environment variables
eclipse-che/che-dashboard#759
eclipse-che/che-operator#1642

@tolusha tolusha closed this as completed Mar 30, 2023
@tolusha tolusha added this to the 7.64 milestone Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator area/dashboard kind/bug Outline of a bug - must adhere to the bug report template. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

3 participants