-
Notifications
You must be signed in to change notification settings - Fork 27
SANDBOX-1392: Drop dependency on Che instance #488
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
Conversation
Signed-off-by: Feny Mehta <fbm3307@gmail.com>
WalkthroughThis change removes Che-related configuration and status fields from the v1alpha1 API, accompanying docs, and generated artifacts: CheConfig, CheSecret, CheStatus, MemberOperatorConfigSpec.che, and Routes.cheDashboardURL are deleted across types, deepcopy, OpenAPI, and docs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
alexeykazakov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe that the Konflux folks use CheConfig fields in the toolchain config but I wonder if dropping it from CRD can break operator upgrade for them... I don't think so but I'm not 100% sure.
@MatousJobanek WDYT?
MatousJobanek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but the https://github.com/codeready-toolchain/api/actions/runs/18030938845/job/51306989126?pr=488 workflow failed for other repos as well - can you please open a PR in the other repos with the update of the api dependency to make sure that we won't break anything?
Signed-off-by: Feny Mehta <fbm3307@gmail.com>
MatousJobanek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's clean up also the comment
api/api/v1alpha1/memberstatus_types.go
Line 49 in 2be085a
| // Routes/URLs of the cluster, such as Console and Che Dashboard URLs |
and the constants:
api/api/v1alpha1/toolchainstatus_types.go
Lines 55 to 60 in 2be085a
| ToolchainStatusMemberStatusCheRouteUnavailableReason = "CheRouteUnavailable" | |
| ToolchainStatusMemberStatusCheUserAPICheckFailedReason = "CheUserAPICheckFailed" | |
| ToolchainStatusMemberStatusCheNotRequiredReason = "CheNotRequired" | |
| ToolchainStatusMemberStatusCheAdminUserNotConfiguredReason = "CheAdminUserNotConfigured" | |
| ToolchainStatusMemberStatusCheUserDeletionNotEnabledReason = "CheUserDeletionNotEnabled" | |
| ToolchainStatusMemberStatusCheReadyReason = "CheReady" |
Signed-off-by: Feny Mehta <fbm3307@gmail.com>
|
Thank you for finding this, Have removed it now , PTAL |
|
I simply played with the regex to avoid the non-relevant matches: |



Description
This PR is to remove the che related unused code
Checks
Did you run
make generatetarget? yesDid
make generatechange anything in other projects (host-operator, member-operator)? yesIn case of new CRD, did you the following? NA
resources/setup/roles/host.yamlin the sandbox-sre repositoryPROJECTfile: https://github.com/codeready-toolchain/host-operator/blob/master/PROJECTCSVfile: https://github.com/codeready-toolchain/host-operator/blob/master/config/manifests/bases/host-operator.clusterserviceversion.yamlIn case other projects are changed, please provides PR links.
Summary by CodeRabbit