chore: remove OSImageConfig and PIR support#8650
Open
cameronmeissner wants to merge 1 commit into
Open
Conversation
cameronmeissner
requested review from
AbelHu,
Devinwong,
SriHarsha001,
awesomenix,
calvin197,
djsly,
ganeshkumarashok,
lilypan26,
mxj220,
pdamianov-dev,
phealy,
r2k1,
sulixu,
surajssd,
timmy-wright and
zachary-bailey
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy OSImageConfig-based image selection path (and associated references) from AgentBaker’s datamodel and bootstrapping flow, intending to rely solely on SIG image configuration.
Changes:
- Removed
OSImageConfigfromNodeBootstrappingand removed OS image config wiring inGetNodeBootstrapping. - Removed
OSImageConfig/AzureOSImageConfigfromAzureEnvironmentSpecConfigand updated unit/e2e test configs accordingly. - Updated unit tests to stop asserting OSImageConfig-based values.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/agent/datamodel/types.go | Drops OSImageConfig from NodeBootstrapping. |
| pkg/agent/datamodel/azenvtypes.go | Removes OSImageConfig from AzureEnvironmentSpecConfig and deletes AzureOSImageConfig type. |
| pkg/agent/bakerapi.go | Removes the OSImageConfig (cloud→distro) fallback; now requires SIG image config. |
| pkg/agent/bakerapi_test.go | Removes OSImageConfig assertions and the “unknown cloud” test; keeps SIG assertions. |
| e2e/node_config.go | Removes OSImageConfig initialization from e2e node bootstrapping configuration templates. |
Comment on lines
6
to
11
| // AzureEnvironmentSpecConfig is the overall configuration differences in different cloud environments. | ||
| type AzureEnvironmentSpecConfig struct { | ||
| CloudName string `json:"cloudName,omitempty"` | ||
| KubernetesSpecConfig KubernetesSpecConfig `json:"kubernetesSpecConfig,omitempty"` | ||
| EndpointConfig AzureEndpointConfig `json:"endpointConfig,omitempty"` | ||
| OSImageConfig map[Distro]AzureOSImageConfig `json:"osImageConfig,omitempty"` | ||
| CloudName string `json:"cloudName,omitempty"` | ||
| KubernetesSpecConfig KubernetesSpecConfig `json:"kubernetesSpecConfig,omitempty"` | ||
| EndpointConfig AzureEndpointConfig `json:"endpointConfig,omitempty"` | ||
| } |
| Expect(err).To(HaveOccurred()) | ||
| }) | ||
|
|
||
| It("should return an error if distro is neither found in PIR nor found in SIG", func() { |
Contributor
Author
|
blocked until all bootstrapper references of NodeBootstrapping.OSImageConfig are removed independently |
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.
What this PR does / why we need it:
chore: remove OSImageConfig and PIR support since it's no longer used anywhere, reviving: #6709
Which issue(s) this PR fixes:
Fixes #