Add BMCSettingsSet type and controller
#471
Conversation
github-actions
bot
added
the
documentation
davidgrun
force-pushed
the
BMCSettingsSet
branch
from
27d9690 to
fa7c2fa
Compare
| //avoid unnecessary updates | ||
| if r.refsAreEqual(bmcSettingsCopy.Spec.ServerMaintenanceRefs, mergedRefs) && | ||
| r.templatesAreEqual(&bmcSettingsCopy.Spec.BMCSettingsTemplate, bmcSettingsTemplate) { | ||
| log.V(2).Info("No changes needed for BMCSettings", "BMCSettings", bmcSettingsCopy.Name) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
isnt this duplicate?
the method CreateOrPatch already checks before updating/patching the resources.
There was a problem hiding this comment.
correct removed the duplicate check and the unnecessary functions for check this in the new pr
- Added BMCSettingsSetReconciler to manage BMCSettingsSet resources. - Implemented reconciliation logic for creating, updating, and deleting BMCSettings based on BMC resources. - Introduced finalizer handling for BMCSettingsSet to manage cleanup during deletion. - Added methods to handle owned BMCSettings and update their status based on the reconciliation state. - Implemented logic to create missing BMCSettings and delete orphaned ones. - Added tests for BMCSettingsSet controller to ensure correct reconciliation behavior. test: Add unit tests for BMCSettingsSet controller - Created comprehensive tests for BMCSettingsSet controller to validate reconciliation logic. - Ensured correct behavior when BMC resources are created, updated, or deleted. - Verified that BMCSettings are generated correctly based on BMC labels. - Added tests for updating BMCSettingsSet and ensuring status updates are reflected. chore: Update test suite to include BMCSettingsSet reconciler - Modified suite_test.go to register BMCSettingsSetReconciler with the test manager. - Ensured all resources are cleaned up after tests, including BMCSettingsSet. fix: Update BMCSettings webhook to include BMCSettingsTemplate - Refactored BMCSettingsSpec to include BMCSettingsTemplate for better structure. - Updated webhook tests to validate the new structure of BMCSettings resources.
…cumentation and tests. To ensure make and lint works
…ing; adjust RBAC roles and CRD version
…enanceRefs filtering logic with clientutils.ListAndFilter
davidgrun
force-pushed
the
BMCSettingsSet
branch
from
b98ae3d to
22f5cc3
Compare
… in CRD Add an update in BMCSettingSet test to check the patch Mechanism
…prove server state handling. For this use the new server claim handling Update biossettings_controller_test: add const for bar variable that go lint works correctly
| Eventually(Update(serverClaim02, func() { | ||
| metautils.SetAnnotation(serverClaim02, metalv1alpha1.ServerMaintenanceApprovalKey, "true") | ||
| })).Should(Succeed()) | ||
|
|
||
| Eventually(Object(bmcSettings02)).Should(SatisfyAny( | ||
| HaveField("Status.State", metalv1alpha1.BMCSettingsStateInProgress), | ||
| HaveField("Status.State", metalv1alpha1.BMCSettingsStateApplied), | ||
| )) | ||
|
|
||
| Eventually(Object(bmcSettings02)).Should(SatisfyAll( | ||
| HaveField("Status.State", metalv1alpha1.BMCSettingsStateApplied), | ||
| )) |
There was a problem hiding this comment.
duplicate code?
There was a problem hiding this comment.
Not in this case. During testing, there were sometimes problems when the transition from InProgress to StateApplied took too long. Therefore, the first step checks that the status is either InProgress or Applied, and then in the second step, we ensure that it actually lands on StateApplied. This makes the test process somewhat more stable.
Nuckal777
left a comment
Nuckal777
left a comment
There was a problem hiding this comment.
Thanks. 👍 PR needs a rebase and the test need to be adjusted due to the removal of DeleteAllMetalResources(). Also, I would consider removing support for ServerMaintenanceRefs on the BMCSettingsSet. It complicates the code a lot and for practical uses I would imagine most people would just like to apply the set and have ServerMaintenances be automatically managed, instead of micro-managing them.
afritzler
changed the title
BMCSettingsSet type and controller
…and ensure BMCSettingsSet state management. Formated bmcsettingsset sample
… and update related deepcopy methods and test
Nuckal777
left a comment
Nuckal777
left a comment
There was a problem hiding this comment.
Thanks, just some comments regarding cosmetics from my side.
| bmcNameMap := make(map[string]metalv1alpha1.BMC) | ||
| for _, bmc := range bmcList.Items { | ||
| bmcNameMap[bmc.Name] = bmc | ||
| } |
| Name: bmcSettingsSet.Name + "-" + bmc02.Name, | ||
| }, | ||
| } | ||
| Consistently(Get(bmcSettings02)).Should(MatchError(ContainSubstring("not found"))) |
There was a problem hiding this comment.
Nitpick: Should(Satisfy(apierrors.IsNotFound)) would avoid matching the error message.
Added BMCSettingsSetReconciler to manage BMCSettingsSet resources.
fixes: #419