Skip to content

Add BIOSSettingsSet resource and controller#383

Merged
afritzler merged 17 commits into
mainfrom
biosSettingSet
Aug 13, 2025
Merged

Add BIOSSettingsSet resource and controller#383
afritzler merged 17 commits into
mainfrom
biosSettingSet

Conversation

@nagadeesh-nagaraja

@nagadeesh-nagaraja nagadeesh-nagaraja commented Jun 27, 2025

Copy link
Copy Markdown
Contributor

Fixes #349

Comment thread internal/controller/biossettingsset_controller.go Outdated
Comment thread internal/controller/biossettingsset_controller.go Outdated
Comment thread internal/controller/biossettingsset_controller.go
Comment thread internal/controller/biossettingsset_controller.go Outdated
Comment thread internal/controller/biossettingsset_controller.go Outdated
Comment thread internal/controller/biossettingsset_controller.go Outdated
Comment thread internal/controller/suite_test.go
@nagadeesh-nagaraja nagadeesh-nagaraja moved this to In Progress in Roadmap Jul 3, 2025
@nagadeesh-nagaraja
nagadeesh-nagaraja marked this pull request as ready for review July 7, 2025 10:37
@nagadeesh-nagaraja
nagadeesh-nagaraja requested review from Nuckal777, SzymonSAP, afritzler and defo89 and removed request for defo89 July 7, 2025 10:37
Comment thread api/v1alpha1/biossettingsset_types.go Outdated
@nagadeesh-nagaraja

Copy link
Copy Markdown
Contributor Author

moving to draft to wait for PR with biosSettingsFlow and BiosVersionSet to complete. this will help reduce double work :)

@nagadeesh-nagaraja
nagadeesh-nagaraja marked this pull request as draft July 10, 2025 12:46
@nagadeesh-nagaraja
nagadeesh-nagaraja marked this pull request as ready for review July 29, 2025 08:06

@Nuckal777 Nuckal777 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. 👍 Logic seems sound in general. This PR needs a rebase once #403 is in.

Comment thread api/v1alpha1/biossettingsset_types.go Outdated
BIOSSettingsTemplate BIOSSettingsTemplate `json:"biosSettingsTemplate,omitempty"`

// ServerSelector specifies a label selector to identify the servers that are to be selected.
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="ServerSelector is immutable"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the serverSelector immutable here? This is not done for other Set types.

@nagadeesh-nagaraja nagadeesh-nagaraja Aug 12, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that it might be good to restrict it? I will need to add it to others as well.
basically, if they change the selector, whole thing will work like newly created one anyway, because it will delete all current and create new one..

why not create new resource at first place?

let me know what you think?

@afritzler afritzler Aug 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just recalculate the dependencies and create/update/delete all child objects? What is the problem with that?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I guess we can fix that later.

Comment thread docs/concepts/biossettingsset.md Outdated
Comment thread internal/controller/biossettingsset_controller.go Outdated
Comment thread internal/controller/biossettingsset_controller.go Outdated
Comment thread internal/controller/biossettingsset_controller.go Outdated
Comment thread internal/controller/biossettingsset_controller.go Outdated
Comment thread internal/controller/biossettingsset_controller.go Outdated
@nagadeesh-nagaraja

Copy link
Copy Markdown
Contributor Author

Thanks. 👍 Logic seems sound in general. This PR needs a rebase once #403 is in.

@Nuckal777 or we could merge this into #403's branch.. if this PR looks good, then merge all together to main.

@afritzler afritzler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm from my side. Just some minor cosmetics.

Comment thread api/v1alpha1/biossettings_types.go Outdated
Comment thread internal/controller/biossettings_controller.go Outdated
@afritzler afritzler added the enhancement New feature or request label Aug 13, 2025
@afritzler
afritzler merged commit 33920c7 into main Aug 13, 2025
16 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Roadmap Aug 13, 2025
@afritzler
afritzler deleted the biosSettingSet branch August 13, 2025 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-change area/metal-automation documentation Improvements or additions to documentation enhancement New feature or request size/XXL

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Add BIOSSettingSet type and controller

5 participants