support GEP-31 in place update#38
Merged
anton-paulovich merged 7 commits intoJun 22, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughExports ChangesUSIFeature In-Place Update Support
Configuration and Toolchain Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cloudprofilesync/imageupdater.go (1)
91-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExisting versions are not reconciled for
InPlaceUpdates.Line 89 computes
supportInPlaceUpdate, but existing-version paths only patch architectures. If a version already exists,InPlaceUpdatesstays stale (never added/removed), so spec can drift from source capabilities.Proposed fix pattern
if idx, exists := existingVersions[sourceImage.Version]; exists { image.Versions[idx].Architectures = sourceImage.Architectures + if supportInPlaceUpdate { + image.Versions[idx].InPlaceUpdates = &gardenerv1beta1.InPlaceUpdates{Supported: true} + } else { + image.Versions[idx].InPlaceUpdates = nil + } } else { ... } @@ if idx, exists := existingVersions[sourceImage.CleanVersion]; exists { existing := &image.Versions[idx] for _, arch := range sourceImage.Architectures { if !slices.Contains(existing.Architectures, arch) { existing.Architectures = append(existing.Architectures, arch) } } + if supportInPlaceUpdate { + existing.InPlaceUpdates = &gardenerv1beta1.InPlaceUpdates{Supported: true} + } else { + existing.InPlaceUpdates = nil + } } else { ... }Also applies to: 118-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cloudprofilesync/imageupdater.go` around lines 91 - 93, When an existing version is found in the existingVersions map within the block where `if idx, exists := existingVersions[sourceImage.Version]; exists`, only the Architectures field is being patched but the InPlaceUpdates field is not being reconciled. The supportInPlaceUpdate variable that was computed should also be used to update the InPlaceUpdates field of the existing version to prevent it from drifting from source capabilities. Apply this same reconciliation pattern to both occurrences of this existing version update logic.
🧹 Nitpick comments (1)
cloudprofilesync/source.go (1)
34-35: ⚡ Quick winRename misspelled exported constant before it spreads.
USIDevFeatrueat Line 34 is typoed (should beUSIDevFeature). Since it is exported, this will leak a misspelled API name into consumers and tests.Proposed fix
- USIDevFeatrue = "_usidev" + USIDevFeature = "_usidev" @@ - USIDevFeatrue: {}, + USIDevFeature: {},Also applies to: 45-45
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cloudprofilesync/source.go` around lines 34 - 35, The exported constant USIDevFeatrue contains a typo (should be USIDevFeature). Rename this constant to the correct spelling USIDevFeature. Since this constant is exported and appears in multiple locations (including line 45 as noted), search for all usages of the misspelled constant name USIDevFeatrue throughout the codebase and update them to use the corrected spelling USIDevFeature to ensure consistency across all references.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cloudprofilesync/imageupdater_test.go`:
- Around line 341-358: The test case at the It block for "reflect inplace update
ability to machineimage" needs to be extended to cover the clean-version path.
Modify the mockSource.images SourceImage to include a CleanVersion field set to
a different value than the Version field (e.g., "1.0" while Version is "1.0.0").
After calling updater.Update and verifying the first version entry, add an
additional assertion to check that the clean-version entry also has
InPlaceUpdates.Supported set to true, ensuring the test exercises both the
legacy append path and the new clean-version path in ImageUpdater.Update.
---
Outside diff comments:
In `@cloudprofilesync/imageupdater.go`:
- Around line 91-93: When an existing version is found in the existingVersions
map within the block where `if idx, exists :=
existingVersions[sourceImage.Version]; exists`, only the Architectures field is
being patched but the InPlaceUpdates field is not being reconciled. The
supportInPlaceUpdate variable that was computed should also be used to update
the InPlaceUpdates field of the existing version to prevent it from drifting
from source capabilities. Apply this same reconciliation pattern to both
occurrences of this existing version update logic.
---
Nitpick comments:
In `@cloudprofilesync/source.go`:
- Around line 34-35: The exported constant USIDevFeatrue contains a typo (should
be USIDevFeature). Rename this constant to the correct spelling USIDevFeature.
Since this constant is exported and appears in multiple locations (including
line 45 as noted), search for all usages of the misspelled constant name
USIDevFeatrue throughout the codebase and update them to use the corrected
spelling USIDevFeature to ensure consistency across all references.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dca6ed2c-0edb-47dc-a74f-5d5c785a04d4
📒 Files selected for processing (4)
cloudprofilesync/imageupdater.gocloudprofilesync/imageupdater_test.gocloudprofilesync/source.gogo.mod
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds propagation of in-place update support (GEP-31) from OCI source image capabilities into the generated CloudProfile machineImages[].versions[].inPlaceUpdates fields, with accompanying tests, and updates some Go/tooling metadata.
Changes:
- Introduces feature/capability constants and uses them when parsing OCI annotations into
Capabilities. - Derives and writes
MachineImageVersion.InPlaceUpdates.Supportedbased on the presence of the_usifeature, and adds/extends test coverage. - Updates the module Go version and adjusts golangci-lint configuration.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
go.mod |
Updates the module Go version directive. |
cloudprofilesync/source.go |
Adds feature/capability constants and updates capability construction/allowlisting. |
cloudprofilesync/imageupdater.go |
Sets InPlaceUpdates on created machine image versions based on capabilities. |
cloudprofilesync/imageupdater_test.go |
Adds tests asserting in-place update propagation behavior. |
.golangci.yaml |
Minor formatting tweaks and updates go-version-pattern (currently with a regex issue). |
Comments suppressed due to low confidence (1)
cloudprofilesync/imageupdater.go:93
- When a version already exists in the CloudProfile, only
Architecturesis updated. If in-place update support is discovered later (or capabilities are added after initial reconcile),InPlaceUpdateswill never be set for existing entries. Consider updatingInPlaceUpdatesin theexistsbranch as well (at least when support is true).
supportInPlaceUpdate := slices.Contains(sourceImage.Capabilities[FeatureCapability], USIFeature)
// Always write the full tag version (legacy path, safe for running Shoots).
if idx, exists := existingVersions[sourceImage.Version]; exists {
image.Versions[idx].Architectures = sourceImage.Architectures
} else {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary by CodeRabbit