Repository navigation
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| HelmInstaller res = | ||
| getHelmInstallService() | ||
| .installChart( | ||
| getHelmConfiguration(region, user), |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved correctness issues can create unintended releases, retain stale values, or fail after an upgrade has already succeeded.
5 open issues
What changed in this PR
Adds in-place service upgrades to Onyxia’s existing service lifecycle API.
Changes:
- Adds an upgrade endpoint and request DTO, reusing chart schema validation.
- Executes Helm upgrades, updates optional metadata, and publishes upgrade events.
- Reports containers in terminating pods as not ready.
| File | Description |
|---|---|
| onyxia-model/src/main/java/fr/insee/onyxia/model/dto/UpgradeServiceDTO.java | Defines upgrade parameters. |
| onyxia-api/src/main/java/fr/insee/onyxia/api/services/impl/HelmAppsService.java | Implements upgrades and adjusts readiness reporting. |
| onyxia-api/src/main/java/fr/insee/onyxia/api/services/AppsService.java | Adds the upgrade service contract. |
| onyxia-api/src/main/java/fr/insee/onyxia/api/events/UpgradeServiceEvent.java | Defines the upgrade event. |
| onyxia-api/src/main/java/fr/insee/onyxia/api/controller/api/mylab/MyLabController.java | Adds the endpoint and shared schema validation. |
Review effort: Balanced
| @RequestBody UpgradeServiceDTO requestDTO) | ||
| throws Exception { | ||
| User user = userProvider.getUser(region); | ||
| Service userService = helmAppsService.getUserService(region, project, user, serviceId); |
Comment on lines
+608
to
+610
| int split = chart.lastIndexOf('-'); | ||
| String chartName = chart.substring(0, split); | ||
| String currentVersion = chart.substring(split + 1); |
| .orElseThrow(NotFoundException::new); | ||
|
|
||
| Map<String, Object> fusion = new HashMap<>(); | ||
| fusion.putAll((Map<String, Object>) requestDTO.getOptions()); |
| skipTlsVerify, | ||
| timeout, | ||
| caFile, | ||
| false); |
| metadata.put(SHARE, String.valueOf(share)); | ||
| } | ||
| if (!metadata.isEmpty()) { | ||
| patchOnyxiaSecret(region, project, user, serviceId, metadata); |
|
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.
Motivation
Onyxia can install, delete and suspend/resume a service, but not re-deploy an existing one with different parameters. To change something as simple as resource limits or network access, users had to delete the service and create a new one, losing its release.
This adds an upgrade operation that re-runs
helm upgradeon the existing release.What changed
PUT /my-lab/app/{serviceId}with anUpgradeServiceDTObody:options(required): the values of the release.packageVersion(optional): defaults to the chart version of the running service.friendlyNameandshare(optional): the current values are kept when omitted.ready: trueduring its grace period, so a restarting service still appeared as running. Containers of terminating pods are now reported as not ready.Usage example
optionsreplaces the values of the release (--reuse-valuesis not used): omitted keys fall back to the chart defaults, so users should send the complete set of values.Notes
vscode-python: changing resource limits restarts the pod and the service shows as starting until it is ready again.helm upgradeis returned as a regular API error, and a follow-up can surface the Helm message to the user.