chore: stop preloading the retired language-server images - #112
Merged
Conversation
Phase 0 cleanup. Every change here is provably inert: rendering the umbrella chart for all five environments before and after produces byte-identical output. Removed: - deploy.sh. Broken and unreferenced. It used --theia-certificates.x= instead of --set (not valid helm syntax, would exit with "unknown flag"), read a ./prod/ directory that does not exist in the repo, and sourced values files from the repo root that do not exist there either. Superseded by GitHub Actions long ago. - value-reference-files/. A sixth copy of values already present in deployments/, referenced by nothing but still being bumped in release PRs. theia-cloud-helm-values.yml still pointed at upstream Docker Hub images (theiacloud/theia-cloud-operator:1.1.0-next). - The monitoring: block in all five environment values files. The theia-monitoring chart is installed as a separate release with no -f values file, and theia-cloud-combined has no monitoring dependency, so these keys reached nothing. Verified the chart hardcoded namespace list is exactly the union of all five environments first, so nothing was silently unmonitored. - The --set theia-certificates.wildcardCertificate/wildcardKey flags. Every environment sets wildcardTLSSecret.enabled=false, so wildcard-secret.yaml renders nothing. Confirmed by rendering theia-certificates with and without the flags: byte-identical. The shared-gateway release still receives the wildcard secrets and is untouched. Kept deliberately: - theia-prometheus-values.yaml moved to docs/reference/ rather than deleted. No workflow installs kube-prometheus-stack, which means that stack was installed by hand and this file is the only surviving record of how. docs/monitoring-setup.md now says so. Docs corrected: the README tree listed a theia-metrics/ chart that does not exist (it is theia-monitoring/) and omitted test2, test3 and both shared-gateway environments. The workflow header listed THEIA_KEYCLOAK_REALM / CLIENT_ID / CLIENT_SECRET as required secrets although none are read anywhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019qeiQRFu8xAMRYWPdZewjG
The external language-server path is being retired. test1, test2, staging and production each pre-pulled four images for it on every node: java-17-no-ls, rust-no-ls, langserver-java, langserver-rust None of those four environments defines an AppDefinition that uses them, so this was pulling several GB per node for apps that do not exist. Preload drops from 14 images to 10 per environment. Also removed from the theia-cloud-combined chart defaults. test3 is deliberately untouched: it is the one environment that actually runs the language-server path, and both of its AppDefinitions are no-ls images, so removing them needs a decision about what test3 is for rather than a deletion. The generic sidecar mechanism is unaffected and stays. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019qeiQRFu8xAMRYWPdZewjG
|
Warning Review limit reachedNext included review available in 37 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
test3 existed to exercise the external language-server path. Both of its
AppDefinitions were no-ls images with langserver sidecars, and its default
landing-page app was java-17-no-ls. With that path retired those images
are no longer built, so test3 would have pointed at images that cannot be
pulled.
It now mirrors test1:
- the theia-appdefinitions override is removed, so it inherits the
chart default app set (8 apps) exactly as test1 does
- default landing-page app is java-17-templates-latest
- additionalApps lists the same 8 entries as test1
- operator and service are unpinned from pr-70 back to latest
- preload drops the four pr-70 language-server images, 14 -> 10
Kept, because they are about where test3 runs rather than what it runs:
storageClassName longhorn, sessionsPerUser 10, the test3 hostnames and
gateway section names, and its own branding.
Incidentally fixes a pre-existing dangling reference: additionalApps
offered java-17-latest, which had no AppDefinition under the old
override, so the landing page listed an app that could not launch.
Verified: test3 renders 48 resources with 8 AppDefinitions, matching
test1 exactly (previously 42 and 2), and every app the landing page
offers now resolves to a real AppDefinition in all five environments.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019qeiQRFu8xAMRYWPdZewjG
Member
Author
Added: test3 normalised to a standard test environmentNow that EduIDE#167 is merged the language-server images are no longer built, so test3 pointing at them would have meant unpullable images. It now mirrors test1.
The Kept — these are about where test3 runs, not what it runs: Fixes a pre-existing bug
No Worth confirming before mergetest3 was on 🤖 Generated with Claude Code |
Only conflict was test3 theia-appdefinitions: this branch removes the override so test3 inherits the chart defaults, main still carried the no-ls apps. Kept the removal. Verified afterwards that both sides survived: no language-server references remain, and the monitoring blocks removed by #111 are still gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019qeiQRFu8xAMRYWPdZewjG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Companion to EduIDE/EduIDE#167, which stops building the language-server images.
Wasted preloading removed
test1,test2,stagingandproductioneach pre-pulled four images on every node:None of those four environments defines an AppDefinition that uses them. This was pulling several GB per node for apps that do not exist. Preload drops from 14 images to 10 per environment.
Also removed from the
theia-cloud-combinedchart defaults.The generic sidecar mechanism is unaffected
sidecarson the v1beta11 CRD and intheia-appdefinitions/templates/appdefinition.yamlare untouched. Only the language-server-specific images go.test3 is deliberately untouched — it needs a decision
test3is the one environment that actually runs this path, and both of its AppDefinitions are no-ls images:Deleting them would leave test3 with zero apps and a default pointing at nothing. That is a question about what test3 is for now, not a deletion — so it is out of this PR.
Separately,
additionalAppsin test3 already listsjava-17-latest, which has no matching AppDefinition — a pre-existing dangling reference worth fixing whichever way test3 goes.🤖 Generated with Claude Code