Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if size == "" { | ||
| size = legacyDefaultSize | ||
| } | ||
| size = string(appsv2.Size(size).Canonical()) |
There was a problem hiding this comment.
🟡 Legacy xxlarge per-app resource overrides dropped in conversion
When a converted v1 config uses the legacy xxlarge size, Canonical() rewrites the sizing lookup key to 2xlarge, but the user's own values still key their per-size block under xxlarge, so it is not found. The per-application resource requests and limits are dropped, and the migrated app falls back to default manifest sizing.
Prompt for agents
In legacyResourcesFromSection (api/v1/weightsandbiases_conversion_overrides.go), the resolved size is canonicalized (xxlarge -> 2xlarge) before being used to index the user's own v1 values sizing block at path {"sizing", size, "resources"}. That sizing block is user-supplied v1 helm values, which for a legacy config are keyed by the legacy spelling (xxlarge). Canonicalizing the lookup key to 2xlarge means a user block keyed sizing.xxlarge.resources is no longer matched, silently dropping the per-application resource override (regression vs. the pre-PR behavior which used the raw size string and mirrored the legacy chart). Consider looking up the sizing block using both the raw size and its canonical form (e.g. merge sizing[size] and, if different, sizing[canonical]), or simply keep the raw size for this user-values lookup, so both xxlarge- and 2xlarge-keyed v1 sizing blocks resolve correctly. Note the manifest-side lookups in internal/controller/reconciler/sizing.go legitimately canonicalize because manifests now use 2xlarge keys; this conversion path reads user data and must not assume canonical keys.
Was this helpful? React with 👍 or 👎 to provide feedback.
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
1 participant
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.
Summary
2xlargethe canonical v2 size spelling while retainingxxlargeas a compatibility aliasTesting
go test ./api/v1 ./api/v2 ./internal/controller/reconciler ./internal/webhook/v2go vet ./...make manifests sync-crd-embed