vcluster icon indicating copy to clipboard operation
vcluster copied to clipboard

fix: returning an error when vcluster config schema changed

Open jjaferson opened this issue 3 weeks ago • 1 comments

What issue type does this pull request address? (keep at least one, remove the others) /kind bugfix

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible) resolves ENG-9408

Please provide a short message that should be published in the vcluster release notes Fixed an issue where vcluster was not being deleted due to the schema difference between vcluster v0.29 and v0.30

What else do we need to know?


[!NOTE] Gracefully handle PrivateNodes schema decode errors during deletion by warning and continuing, enabled by a new custom error and guard checks.

  • CLI (delete):
    • On Helm values → config.Config unmarshal failure due to PrivateNodes schema, log warning and proceed with deletion; skip namespace cleanup when config is unavailable.
  • Config:
    • Add ErrInvalidPrivateNodesConfig and custom PrivateNodes.UnmarshalJSON to wrap decode errors for clearer detection.

Written by Cursor Bugbot for commit c35abd0a4f331732d3b723e50f136010ed0080b5. This will update automatically on new commits. Configure here.

jjaferson avatar Dec 04 '25 15:12 jjaferson

@FabianKramm

The solution is to fix that we error out when deleting a vCluster, not sure why parsing the vCluster config there is needed anyways

The config parsing in the cli was introduced because of the CleanUpNamespace, which depends on the vclusterConfig.Sync.ToHost.Namespaces.Enabled setting in the config. The config is parsed only to determine whether namespace cleanup should be executed.

If we remove the config parsing, the cleanup function would need to run unconditionally which should be fine, since the function already checks whether managed namespaces exist.

There is a bigger fundamental problem which is that we try to parse the config when deleting a vCluster again

I understand that you're essentially saying we should not rely on the vCluster config when managing resources from cli, but rather on the resource specs themselves, which were generated based on that config is that correct?

mfranczy avatar Dec 09 '25 11:12 mfranczy

@mfranczy yes then lets do it like this, don't parse the vCluster config anymore or just parse the sync.toHost.namespaces.enabled unstrictly in a generic struct

Yes the CLI needs to be vCluster version independent and that also means independent of the vCluster config

FabianKramm avatar Dec 11 '25 08:12 FabianKramm

💚 All backports created successfully

Status Branch Result
v0.29
v0.30

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

loft-bot avatar Dec 12 '25 11:12 loft-bot

💚 All backports created successfully

Status Branch Result
v0.29
v0.30

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

loft-bot avatar Dec 12 '25 11:12 loft-bot