Skip to content

Commit 041ad5d

Browse files
stefanhipfelstefanhipfel
authored
Remove Completed state from ServerMaintenance (#286)
* Remove completed state from ServerMaintenance (delete crd to complete maintenance) * make generate && make docs && make helm * fixes review comments
1 parent 1185a90 commit 041ad5d

File tree

4 files changed

+56
-62
lines changed

4 files changed

+56
-62
lines changed

‎api/v1alpha1/servermaintenance_types.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@ const (
6262
// ServerMaintenanceStateInMaintenance specifies that the server is in maintenance.
6363
ServerMaintenanceStateInMaintenance ServerMaintenanceState = "InMaintenance"
6464
// ServerMaintenanceStateCompleted specifies that the server maintenance has been completed.
65-
ServerMaintenanceStateCompleted ServerMaintenanceState = "Completed"
66-
// ServerMaintenanceStateFailed specifies that the server maintenance has failed.
6765
ServerMaintenanceStateFailed ServerMaintenanceState = "Failed"
6866
)
6967

‎docs/api-reference/api.md‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2410,12 +2410,9 @@ ServerBootConfigurationTemplate
24102410
<th>Description</th>
24112411
</tr>
24122412
</thead>
2413-
<tbody><tr><td><p>&#34;Completed&#34;</p></td>
2413+
<tbody><tr><td><p>&#34;Failed&#34;</p></td>
24142414
<td><p>ServerMaintenanceStateCompleted specifies that the server maintenance has been completed.</p>
24152415
</td>
2416-
</tr><tr><td><p>&#34;Failed&#34;</p></td>
2417-
<td><p>ServerMaintenanceStateFailed specifies that the server maintenance has failed.</p>
2418-
</td>
24192416
</tr><tr><td><p>&#34;InMaintenance&#34;</p></td>
24202417
<td><p>ServerMaintenanceStateInMaintenance specifies that the server is in maintenance.</p>
24212418
</td>

‎internal/controller/servermaintenance_controller.go‎

Lines changed: 30 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
)
2525

2626
const (
27+
// ServerMaintenanceFinalizer is the finalizer for the ServerMaintenance resource.
2728
ServerMaintenanceFinalizer = "metal.ironcore.dev/servermaintenance"
2829
)
2930

@@ -89,15 +90,13 @@ func (r *ServerMaintenanceReconciler) ensureServerMaintenanceStateTransition(ctx
8990
return r.handlePendingState(ctx, log, serverMaintenance)
9091
case metalv1alpha1.ServerMaintenanceStateInMaintenance:
9192
return r.handleInMaintenanceState(ctx, log, serverMaintenance)
92-
case metalv1alpha1.ServerMaintenanceStateCompleted:
93-
return r.handleCompletedState(ctx, log, serverMaintenance)
9493
case metalv1alpha1.ServerMaintenanceStateFailed:
95-
return r.handleFailedState(ctx, log, serverMaintenance)
94+
return r.handleFailedState(log, serverMaintenance)
9695
}
9796
return ctrl.Result{}, nil
9897
}
9998

100-
func (r *ServerMaintenanceReconciler) handlePendingState(ctx context.Context, log logr.Logger, serverMaintenance *metalv1alpha1.ServerMaintenance) (ctrl.Result, error) {
99+
func (r *ServerMaintenanceReconciler) handlePendingState(ctx context.Context, log logr.Logger, serverMaintenance *metalv1alpha1.ServerMaintenance) (result ctrl.Result, err error) {
101100
server, err := r.getServerRef(ctx, serverMaintenance)
102101
if err != nil {
103102
return ctrl.Result{}, err
@@ -110,6 +109,10 @@ func (r *ServerMaintenanceReconciler) handlePendingState(ctx context.Context, lo
110109
}
111110
if server.Spec.ServerClaimRef == nil {
112111
log.V(1).Info("Server has no claim, move to maintenance right away", "Server", server.Name)
112+
if err = r.updateServerRef(ctx, log, serverMaintenance, server); err != nil {
113+
log.Error(err, "failed to patch server maintenance ref")
114+
return ctrl.Result{}, err
115+
}
113116
if modified, err := r.patchMaintenanceState(ctx, serverMaintenance, metalv1alpha1.ServerMaintenanceStateInMaintenance); err != nil || modified {
114117
return ctrl.Result{}, err
115118
}
@@ -142,12 +145,20 @@ func (r *ServerMaintenanceReconciler) handlePendingState(ctx context.Context, lo
142145
return ctrl.Result{}, nil
143146
}
144147
log.V(1).Info("Server approved for maintenance", "Server", server.Name, "Maintenance", serverMaintenance.Name)
148+
if err = r.updateServerRef(ctx, log, serverMaintenance, server); err != nil {
149+
log.Error(err, "failed to patch server maintenance ref")
150+
return ctrl.Result{}, err
151+
}
145152
if modified, err := r.patchMaintenanceState(ctx, serverMaintenance, metalv1alpha1.ServerMaintenanceStateInMaintenance); err != nil || modified {
146153
return ctrl.Result{}, err
147154
}
148155
}
149156
if serverMaintenance.Spec.Policy == metalv1alpha1.ServerMaintenancePolicyEnforced {
150157
log.V(1).Info("Enforcing maintenance", "Server", server.Name, "Maintenance", serverMaintenance.Name)
158+
if err = r.updateServerRef(ctx, log, serverMaintenance, server); err != nil {
159+
log.Error(err, "failed to patch server maintenance ref")
160+
return ctrl.Result{}, err
161+
}
151162
if modified, err := r.patchMaintenanceState(ctx, serverMaintenance, metalv1alpha1.ServerMaintenanceStateInMaintenance); err != nil || modified {
152163
return ctrl.Result{}, err
153164
}
@@ -160,10 +171,6 @@ func (r *ServerMaintenanceReconciler) handleInMaintenanceState(ctx context.Conte
160171
if err != nil {
161172
return ctrl.Result{}, err
162173
}
163-
// put server in maintenance
164-
if err := r.patchServerRef(ctx, log, serverMaintenance, server); err != nil {
165-
return ctrl.Result{}, err
166-
}
167174
config, err := r.applyServerBootConfiguration(ctx, log, serverMaintenance, server)
168175
if err != nil {
169176
return ctrl.Result{}, err
@@ -239,41 +246,29 @@ func (r *ServerMaintenanceReconciler) setAndPatchServerPowerState(ctx context.Co
239246
return nil
240247
}
241248

242-
func (r *ServerMaintenanceReconciler) patchServerRef(ctx context.Context, log logr.Logger, maintenance *metalv1alpha1.ServerMaintenance, server *metalv1alpha1.Server) error {
249+
func (r *ServerMaintenanceReconciler) updateServerRef(ctx context.Context, log logr.Logger, maintenance *metalv1alpha1.ServerMaintenance, server *metalv1alpha1.Server) error {
243250
if server.Spec.ServerMaintenanceRef != nil {
244251
log.V(1).Info("Server is already in Maintenance", "Server", server.Name, "Maintenance", server.Spec.ServerMaintenanceRef.Name)
245252
return nil
246253
}
247-
if server.Spec.ServerMaintenanceRef == nil {
248-
serverBase := server.DeepCopy()
249-
server.Spec.ServerMaintenanceRef = &v1.ObjectReference{
250-
APIVersion: "metal.ironcore.dev/v1alpha1",
251-
Kind: "ServerMaintenance",
252-
Namespace: maintenance.Namespace,
253-
Name: maintenance.Name,
254-
UID: maintenance.UID,
255-
}
256-
if err := r.Patch(ctx, server, client.MergeFrom(serverBase)); err != nil {
257-
return fmt.Errorf("failed to patch maintenance ref for server: %w", err)
258-
}
259-
log.V(1).Info("Patched ServerMaintenance reference on Server", "Server", server.Name, "ServerMaintenanceeRef", maintenance.Name)
260-
}
261-
return nil
262-
}
263-
264-
func (r *ServerMaintenanceReconciler) handleCompletedState(ctx context.Context, log logr.Logger, serverMaintenance *metalv1alpha1.ServerMaintenance) (ctrl.Result, error) {
265-
server, err := r.getServerRef(ctx, serverMaintenance)
266-
if err != nil {
267-
return ctrl.Result{}, err
254+
server.Spec.ServerMaintenanceRef = &v1.ObjectReference{
255+
APIVersion: "metal.ironcore.dev/v1alpha1",
256+
Kind: "ServerMaintenance",
257+
Namespace: maintenance.Namespace,
258+
Name: maintenance.Name,
259+
UID: maintenance.UID,
268260
}
269-
log.V(1).Info("Server maintenance completed", "Server", server.Name)
270-
if err := r.cleanup(ctx, log, server); err != nil {
271-
return ctrl.Result{}, err
261+
// use update to not overwrite ServerMaintenanceRef if another maintenance was quicker
262+
if err := r.Update(ctx, server); err != nil {
263+
return fmt.Errorf("failed to patch maintenance ref for server: %w", err)
272264
}
273-
return ctrl.Result{}, nil
265+
log.V(1).Info("Updated ServerMaintenance reference on Server", "Server", server.Name, "ServerMaintenanceeRef", maintenance.Name)
266+
267+
return nil
274268
}
275269

276-
func (r *ServerMaintenanceReconciler) handleFailedState(ctx context.Context, log logr.Logger, serverMaintenance *metalv1alpha1.ServerMaintenance) (ctrl.Result, error) {
270+
func (r *ServerMaintenanceReconciler) handleFailedState(log logr.Logger, serverMaintenance *metalv1alpha1.ServerMaintenance) (ctrl.Result, error) {
271+
log.V(1).Info("ServerMaintenance failed", "ServerMaintenance", serverMaintenance.Name)
277272
return ctrl.Result{}, nil
278273
}
279274

‎internal/controller/servermaintenance_controller_test.go‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -187,14 +187,9 @@ var _ = Describe("ServerMaintenance Controller", func() {
187187
Eventually(Object(serverMaintenance)).Should(SatisfyAll(
188188
HaveField("Status.State", metalv1alpha1.ServerMaintenanceStateInMaintenance),
189189
))
190-
By("Setting ServerMaintenance to Completed")
191-
Eventually(UpdateStatus(serverMaintenance, func() {
192-
serverMaintenance.Status.State = metalv1alpha1.ServerMaintenanceStateCompleted
193-
})).Should(Succeed())
190+
By("Deleting ServerMaintenance to finish the maintennce on the server")
191+
Eventually(k8sClient.Delete).WithArguments(ctx, serverMaintenance).Should(Succeed())
194192

195-
Eventually(Object(serverMaintenance)).Should(SatisfyAll(
196-
HaveField("Status.State", metalv1alpha1.ServerMaintenanceStateCompleted),
197-
))
198193
By("Checking the Server is not in maintenance and cleaned up")
199194
Eventually(Object(server)).Should(SatisfyAll(
200195
HaveField("Status.State", metalv1alpha1.ServerStateReserved),
@@ -212,7 +207,7 @@ var _ = Describe("ServerMaintenance Controller", func() {
212207

213208
serverMaintenance01 := &metalv1alpha1.ServerMaintenance{
214209
ObjectMeta: metav1.ObjectMeta{
215-
Name: "test-server-maintenance",
210+
Name: "test-server-maintenance01",
216211
Namespace: ns.Name,
217212
Annotations: map[string]string{
218213
metalv1alpha1.ServerMaintenanceReasonAnnotationKey: "test-maintenance",
@@ -253,9 +248,26 @@ var _ = Describe("ServerMaintenance Controller", func() {
253248
},
254249
}
255250
Expect(k8sClient.Create(ctx, serverMaintenance01)).To(Succeed())
251+
By("Checking the ServerMaintenanceRef")
252+
Eventually(Object(server)).Should(SatisfyAll(
253+
HaveField("Spec.ServerMaintenanceRef", Not(BeNil())),
254+
HaveField("Spec.MaintenanceBootConfigurationRef", Not(BeNil())),
255+
))
256256
Eventually(Object(serverMaintenance01)).Should(SatisfyAll(
257257
HaveField("Status.State", metalv1alpha1.ServerMaintenanceStateInMaintenance),
258258
))
259+
bootConfig := &metalv1alpha1.ServerBootConfiguration{}
260+
Eventually(k8sClient.Get).WithArguments(ctx, types.NamespacedName{
261+
Name: server.Spec.MaintenanceBootConfigurationRef.Name,
262+
Namespace: server.Spec.MaintenanceBootConfigurationRef.Namespace,
263+
}, bootConfig).Should(Succeed())
264+
265+
By("Patching the boot configuration to a Ready state")
266+
Eventually(UpdateStatus(bootConfig, func() {
267+
bootConfig.Status.State = metalv1alpha1.ServerBootConfigurationStateReady
268+
})).Should(Succeed())
269+
270+
By("Creating a second ServerMaintenance object")
259271
Expect(k8sClient.Create(ctx, serverMaintenance02)).To(Succeed())
260272
Eventually(Object(serverMaintenance02)).Should(SatisfyAll(
261273
HaveField("Status.State", metalv1alpha1.ServerMaintenanceStatePending),
@@ -265,21 +277,13 @@ var _ = Describe("ServerMaintenance Controller", func() {
265277
Eventually(Object(server)).Should(SatisfyAll(
266278
HaveField("Status.State", metalv1alpha1.ServerStateMaintenance),
267279
))
268-
269-
By("Setting first ServerMaintenance to Completed")
270-
Eventually(UpdateStatus(serverMaintenance01, func() {
271-
serverMaintenance01.Status.State = metalv1alpha1.ServerMaintenanceStateCompleted
272-
})).Should(Succeed())
273-
274-
Eventually(Object(serverMaintenance01)).Should(SatisfyAll(
275-
HaveField("Status.State", metalv1alpha1.ServerMaintenanceStateCompleted),
280+
By("Checking the second ServerMaintenance is still pending")
281+
Eventually(Object(serverMaintenance02)).Should(SatisfyAll(
282+
HaveField("Status.State", metalv1alpha1.ServerMaintenanceStatePending),
276283
))
277284

278-
Eventually(Object(server)).Should(SatisfyAll(
279-
HaveField("Status.State", metalv1alpha1.ServerStateAvailable),
280-
HaveField("Spec.ServerMaintenanceRef", BeNil()),
281-
HaveField("Spec.MaintenanceBootConfigurationRef", BeNil()),
282-
))
285+
By("Deleting first ServerMaintenance to finish the maintennce on the server")
286+
Eventually(k8sClient.Delete).WithArguments(ctx, serverMaintenance01).Should(Succeed())
283287

284288
Eventually(Object(serverMaintenance02)).Should(SatisfyAll(
285289
HaveField("Status.State", metalv1alpha1.ServerMaintenanceStateInMaintenance),

0 commit comments

Comments
 (0)