Skip to content

Commit 14b9d96

Browse files
committed
Add HypervisorReadyController to centralize Ready condition logic
Introduces a new ReadyController that computes the Ready condition based on other conditions (Onboarding, Offboarded, Terminating, Evicting) and the maintenance spec field. Changes: - Add internal/controller/ready package with controller and tests - Remove Ready condition setting from OnboardingController - Remove Ready condition setting from MaintenanceController - Remove Ready condition setting from OffboardingController - Remove Ready condition setting from HypervisorController - Update OffboardingController to use Offboarded condition for state tracking The ReadyController uses a StatusChangedPredicate to only reconcile when status changes, avoiding unnecessary reconciliation on spec changes. Ready condition priority: 1. Offboarded=True -> Ready=False 2. Terminating=True -> Ready=False 3. Onboarding in progress -> Ready=False 4. Maintenance mode (with eviction state) -> Ready=False 5. Onboarding not succeeded -> Ready=False 6. All checks pass -> Ready=True
1 parent 499dcef commit 14b9d96

7 files changed

Lines changed: 519 additions & 72 deletions

File tree

cmd/main.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import (
4949

5050
kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
5151
"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller"
52+
"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/ready"
5253
"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global"
5354
"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/logger"
5455

@@ -316,6 +317,14 @@ func main() {
316317
os.Exit(1)
317318
}
318319

320+
if err = (&ready.Controller{
321+
Client: mgr.GetClient(),
322+
Scheme: mgr.GetScheme(),
323+
}).SetupWithManager(mgr); err != nil {
324+
setupLog.Error(err, "unable to create controller", "controller", ready.ControllerName)
325+
os.Exit(1)
326+
}
327+
319328
// +kubebuilder:scaffold:builder
320329

321330
if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {

internal/controller/hypervisor_controller.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -112,18 +112,6 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request)
112112
nodeTerminationCondition := FindNodeStatusCondition(node.Status.Conditions, "Terminating")
113113
if nodeTerminationCondition != nil && nodeTerminationCondition.Status == corev1.ConditionTrue {
114114
// Node might be terminating, propagate condition to hypervisor
115-
116-
if readyCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeReady); readyCondition == nil || readyCondition.Status == metav1.ConditionTrue {
117-
// Only set Terminating condition if Ready is still True, otherwise we might overwrite other controllers that already set Ready to False
118-
// In particular if the hypervisor is evicting
119-
meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
120-
Type: kvmv1.ConditionTypeReady,
121-
Status: metav1.ConditionFalse,
122-
Reason: nodeTerminationCondition.Reason,
123-
Message: nodeTerminationCondition.Message,
124-
})
125-
}
126-
127115
meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
128116
Type: kvmv1.ConditionTypeTerminating,
129117
Status: metav1.ConditionStatus(nodeTerminationCondition.Status),

internal/controller/hypervisor_maintenance_controller.go

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,6 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.
109109
return nil
110110
}
111111

112-
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
113-
Type: kvmv1.ConditionTypeReady,
114-
Status: metav1.ConditionTrue,
115-
Reason: kvmv1.ConditionReasonReadyReady,
116-
Message: "Hypervisor is ready",
117-
})
118-
119112
// We need to enable the host as per spec
120113
enableService := services.UpdateOpts{Status: services.ServiceEnabled}
121114
log.Info("Enabling hypervisor", "id", serviceId)
@@ -138,13 +131,6 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.
138131
return nil
139132
}
140133

141-
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
142-
Type: kvmv1.ConditionTypeReady,
143-
Status: metav1.ConditionFalse,
144-
Reason: kvmv1.ConditionReasonReadyMaintenance,
145-
Message: "Hypervisor is disabled",
146-
})
147-
148134
// We need to disable the host as per spec
149135
enableService := services.UpdateOpts{
150136
Status: services.ServiceDisabled,
@@ -194,22 +180,10 @@ func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Contex
194180
message = "Evicted"
195181
reason = kvmv1.ConditionReasonSucceeded
196182
hv.Status.Evicted = true
197-
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
198-
Type: kvmv1.ConditionTypeReady,
199-
Status: metav1.ConditionFalse,
200-
Reason: kvmv1.ConditionReasonReadyEvicted,
201-
Message: "Hypervisor is disabled and evicted",
202-
})
203183
} else {
204184
message = "Evicting"
205185
reason = kvmv1.ConditionReasonRunning
206186
hv.Status.Evicted = false
207-
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
208-
Type: kvmv1.ConditionTypeReady,
209-
Status: metav1.ConditionFalse,
210-
Reason: kvmv1.ConditionReasonReadyEvicting,
211-
Message: "Hypervisor is disabled and evicting",
212-
})
213187
}
214188

215189
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{

internal/controller/offboarding_controller.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,14 @@ func (r *HypervisorOffboardingReconciler) Reconcile(ctx context.Context, req ctr
6868
return ctrl.Result{}, nil
6969
}
7070

71-
if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeReady) {
71+
// Check if offboarding has already started or completed
72+
offboardedCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded)
73+
if offboardedCondition == nil {
74+
// Start offboarding
7275
return r.setOffboardingCondition(ctx, hv, "Hypervisor is being offboarded, removing host from nova")
7376
}
7477

75-
if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded) {
78+
if offboardedCondition.Status == metav1.ConditionTrue {
7679
return ctrl.Result{}, nil
7780
}
7881

@@ -145,7 +148,7 @@ func (r *HypervisorOffboardingReconciler) Reconcile(ctx context.Context, req ctr
145148
func (r *HypervisorOffboardingReconciler) setOffboardingCondition(ctx context.Context, hv *kvmv1.Hypervisor, message string) (ctrl.Result, error) {
146149
base := hv.DeepCopy()
147150
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
148-
Type: kvmv1.ConditionTypeReady,
151+
Type: kvmv1.ConditionTypeOffboarded,
149152
Status: metav1.ConditionFalse,
150153
Reason: "Offboarding",
151154
Message: message,
@@ -165,12 +168,6 @@ func (r *HypervisorOffboardingReconciler) markOffboarded(ctx context.Context, hv
165168
Reason: "Offboarded",
166169
Message: "Offboarding successful",
167170
})
168-
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
169-
Type: kvmv1.ConditionTypeReady,
170-
Status: metav1.ConditionFalse,
171-
Reason: "Offboarded",
172-
Message: "Offboarding successful",
173-
})
174171
if err := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
175172
k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(OffboardingControllerName)); err != nil {
176173
return fmt.Errorf("cannot update hypervisor status due to %w", err)

internal/controller/onboarding_controller.go

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,6 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request)
110110
status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding)
111111
if status == nil {
112112
base := hv.DeepCopy()
113-
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
114-
Type: kvmv1.ConditionTypeReady,
115-
Status: metav1.ConditionFalse,
116-
Reason: kvmv1.ConditionReasonOnboarding,
117-
Message: "Onboarding in progress",
118-
})
119113

120114
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
121115
Type: kvmv1.ConditionTypeOnboarding,
@@ -151,18 +145,6 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy
151145
}
152146

153147
base := hv.DeepCopy()
154-
ready := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeReady)
155-
if ready != nil {
156-
// Only undo ones own readiness status reporting
157-
if ready.Reason == kvmv1.ConditionReasonOnboarding {
158-
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
159-
Type: kvmv1.ConditionTypeReady,
160-
Status: metav1.ConditionFalse,
161-
Reason: kvmv1.ConditionReasonOnboarding,
162-
Message: "Onboarding aborted",
163-
})
164-
}
165-
}
166148

167149
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
168150
Type: kvmv1.ConditionTypeOnboarding,
@@ -354,13 +336,6 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri
354336
Message: "Onboarding completed",
355337
})
356338

357-
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
358-
Type: kvmv1.ConditionTypeReady,
359-
Status: metav1.ConditionTrue,
360-
Reason: kvmv1.ConditionReasonReadyReady,
361-
Message: "Hypervisor is ready",
362-
})
363-
364339
return ctrl.Result{}, r.patchStatus(ctx, hv, base)
365340
}
366341

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
/*
2+
SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors
3+
SPDX-License-Identifier: Apache-2.0
4+
5+
Licensed under the Apache License, Version 2.0 (the "License");
6+
you may not use this file except in compliance with the License.
7+
You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package ready
19+
20+
import (
21+
"context"
22+
23+
"k8s.io/apimachinery/pkg/api/equality"
24+
"k8s.io/apimachinery/pkg/api/meta"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
"k8s.io/apimachinery/pkg/runtime"
27+
ctrl "sigs.k8s.io/controller-runtime"
28+
"sigs.k8s.io/controller-runtime/pkg/builder"
29+
k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
30+
"sigs.k8s.io/controller-runtime/pkg/event"
31+
logger "sigs.k8s.io/controller-runtime/pkg/log"
32+
"sigs.k8s.io/controller-runtime/pkg/predicate"
33+
34+
kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
35+
)
36+
37+
const (
38+
ControllerName = "ready"
39+
)
40+
41+
// StatusChangedPredicate triggers reconciliation only when the status subresource changes.
42+
// This is the inverse of GenerationChangedPredicate which triggers on spec changes.
43+
type StatusChangedPredicate struct{}
44+
45+
var _ predicate.Predicate = StatusChangedPredicate{}
46+
47+
func (StatusChangedPredicate) Create(_ event.CreateEvent) bool {
48+
// Reconcile on create to compute initial Ready condition
49+
return true
50+
}
51+
52+
func (StatusChangedPredicate) Delete(_ event.DeleteEvent) bool {
53+
// No need to reconcile on delete
54+
return false
55+
}
56+
57+
func (StatusChangedPredicate) Update(e event.UpdateEvent) bool {
58+
if e.ObjectOld == nil || e.ObjectNew == nil {
59+
return false
60+
}
61+
62+
oldHv, ok := e.ObjectOld.(*kvmv1.Hypervisor)
63+
if !ok {
64+
return false
65+
}
66+
newHv, ok := e.ObjectNew.(*kvmv1.Hypervisor)
67+
if !ok {
68+
return false
69+
}
70+
71+
// Only trigger if status changed (ignore spec-only changes)
72+
return !equality.Semantic.DeepEqual(oldHv.Status, newHv.Status)
73+
}
74+
75+
func (StatusChangedPredicate) Generic(_ event.GenericEvent) bool {
76+
return true
77+
}
78+
79+
// Controller reconciles the Ready condition based on other conditions
80+
type Controller struct {
81+
k8sclient.Client
82+
Scheme *runtime.Scheme
83+
}
84+
85+
// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors,verbs=get;list;watch
86+
// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors/status,verbs=get;list;watch;patch
87+
88+
func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
89+
log := logger.FromContext(ctx).WithName(req.Name)
90+
ctx = logger.IntoContext(ctx, log)
91+
92+
hv := &kvmv1.Hypervisor{}
93+
if err := r.Get(ctx, req.NamespacedName, hv); err != nil {
94+
return ctrl.Result{}, k8sclient.IgnoreNotFound(err)
95+
}
96+
97+
base := hv.DeepCopy()
98+
99+
// Compute Ready condition based on other conditions
100+
readyCondition := ComputeReadyCondition(hv)
101+
meta.SetStatusCondition(&hv.Status.Conditions, readyCondition)
102+
103+
if equality.Semantic.DeepEqual(hv.Status, base.Status) {
104+
return ctrl.Result{}, nil
105+
}
106+
107+
log.Info("Updating Ready condition", "status", readyCondition.Status, "reason", readyCondition.Reason)
108+
return ctrl.Result{}, r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
109+
k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(ControllerName))
110+
}
111+
112+
// ComputeReadyCondition determines the Ready condition based on other conditions
113+
func ComputeReadyCondition(hv *kvmv1.Hypervisor) metav1.Condition {
114+
// Priority 1: Offboarded
115+
if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded) {
116+
return metav1.Condition{
117+
Type: kvmv1.ConditionTypeReady,
118+
Status: metav1.ConditionFalse,
119+
Reason: "Offboarded",
120+
Message: "Hypervisor has been offboarded",
121+
}
122+
}
123+
124+
// Priority 2: Terminating
125+
if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) {
126+
return metav1.Condition{
127+
Type: kvmv1.ConditionTypeReady,
128+
Status: metav1.ConditionFalse,
129+
Reason: kvmv1.ConditionReasonTerminating,
130+
Message: "Hypervisor is terminating",
131+
}
132+
}
133+
134+
// Priority 3/5: Onboarding state
135+
onboardingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding)
136+
if onboardingCondition == nil {
137+
// Onboarding not started
138+
return metav1.Condition{
139+
Type: kvmv1.ConditionTypeReady,
140+
Status: metav1.ConditionFalse,
141+
Reason: kvmv1.ConditionReasonOnboarding,
142+
Message: "Onboarding not started",
143+
}
144+
}
145+
146+
// Active onboarding (Status=True means onboarding is in progress)
147+
if onboardingCondition.Status == metav1.ConditionTrue {
148+
return metav1.Condition{
149+
Type: kvmv1.ConditionTypeReady,
150+
Status: metav1.ConditionFalse,
151+
Reason: kvmv1.ConditionReasonOnboarding,
152+
Message: "Onboarding in progress: " + onboardingCondition.Message,
153+
}
154+
}
155+
156+
// Onboarding aborted
157+
if onboardingCondition.Reason == kvmv1.ConditionReasonAborted {
158+
return metav1.Condition{
159+
Type: kvmv1.ConditionTypeReady,
160+
Status: metav1.ConditionFalse,
161+
Reason: kvmv1.ConditionReasonOnboarding,
162+
Message: "Onboarding was aborted",
163+
}
164+
}
165+
166+
// Priority 4: Maintenance mode
167+
if hv.Spec.Maintenance != "" && hv.Spec.Maintenance != kvmv1.MaintenanceUnset {
168+
evictingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting)
169+
if evictingCondition != nil {
170+
if evictingCondition.Status == metav1.ConditionTrue {
171+
return metav1.Condition{
172+
Type: kvmv1.ConditionTypeReady,
173+
Status: metav1.ConditionFalse,
174+
Reason: kvmv1.ConditionReasonReadyEvicting,
175+
Message: "Hypervisor is disabled and evicting",
176+
}
177+
}
178+
if evictingCondition.Status == metav1.ConditionFalse {
179+
return metav1.Condition{
180+
Type: kvmv1.ConditionTypeReady,
181+
Status: metav1.ConditionFalse,
182+
Reason: kvmv1.ConditionReasonReadyEvicted,
183+
Message: "Hypervisor is disabled and evicted",
184+
}
185+
}
186+
}
187+
return metav1.Condition{
188+
Type: kvmv1.ConditionTypeReady,
189+
Status: metav1.ConditionFalse,
190+
Reason: kvmv1.ConditionReasonReadyMaintenance,
191+
Message: "Hypervisor is in maintenance mode",
192+
}
193+
}
194+
195+
// Check onboarding succeeded
196+
if onboardingCondition.Reason != kvmv1.ConditionReasonSucceeded {
197+
return metav1.Condition{
198+
Type: kvmv1.ConditionTypeReady,
199+
Status: metav1.ConditionFalse,
200+
Reason: kvmv1.ConditionReasonOnboarding,
201+
Message: "Onboarding not yet completed",
202+
}
203+
}
204+
205+
// Priority 6: All checks passed - Ready
206+
return metav1.Condition{
207+
Type: kvmv1.ConditionTypeReady,
208+
Status: metav1.ConditionTrue,
209+
Reason: kvmv1.ConditionReasonReadyReady,
210+
Message: "Hypervisor is ready",
211+
}
212+
}
213+
214+
func (r *Controller) SetupWithManager(mgr ctrl.Manager) error {
215+
return ctrl.NewControllerManagedBy(mgr).
216+
Named(ControllerName).
217+
For(&kvmv1.Hypervisor{}, builder.WithPredicates(StatusChangedPredicate{})).
218+
Complete(r)
219+
}

0 commit comments

Comments
 (0)