Skip to content

Commit 00b02a9

Browse files
author
openshift-service-mesh-bot
committed
Automated merge
* upstream/main: Check IstioCNI status for IstioRevisions that depend on IstioCNI (istio-ecosystem#711)
2 parents a4f906d + 6ee0278 commit 00b02a9

File tree

10 files changed

+472
-31
lines changed

10 files changed

+472
-31
lines changed

‎api/v1/istio_types.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,23 @@ const (
234234
IstioReasonReadinessCheckFailed IstioConditionReason = "ReadinessCheckFailed"
235235
)
236236

237+
const (
238+
// IstioConditionDependenciesHealthy signifies whether the dependencies required by this Istio are healthy.
239+
// For example, an Istio with spec.values.pilot.cni.enabled=true requires the IstioCNI resource to be deployed
240+
// and ready for the Istio revision to be considered healthy. The DependenciesHealthy condition is used to indicate that
241+
// the IstioCNI resource is healthy.
242+
IstioConditionDependenciesHealthy IstioConditionType = "DependenciesHealthy"
243+
244+
// IstioReasonIstioCNINotFound indicates that the IstioCNI resource is not found.
245+
IstioReasonIstioCNINotFound IstioConditionReason = "IstioCNINotFound"
246+
247+
// IstioReasonIstioCNINotHealthy indicates that the IstioCNI resource is not healthy.
248+
IstioReasonIstioCNINotHealthy IstioConditionReason = "IstioCNINotHealthy"
249+
250+
// IstioReasonDependencyCheckFailed indicates that the status of the dependencies could not be ascertained.
251+
IstioReasonDependencyCheckFailed IstioConditionReason = "DependencyCheckFailed"
252+
)
253+
237254
const (
238255
// IstioReasonHealthy indicates that the control plane is fully reconciled and that all components are ready.
239256
IstioReasonHealthy IstioConditionReason = "Healthy"

‎api/v1/istiorevision_types.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,23 @@ const (
168168
IstioRevisionReasonUsageCheckFailed IstioRevisionConditionReason = "UsageCheckFailed"
169169
)
170170

171+
const (
172+
// IstioRevisionConditionDependenciesHealthy signifies whether the dependencies required by this IstioRevision are healthy.
173+
// For example, an IstioRevision with spec.values.pilot.cni.enabled=true requires the IstioCNI resource to be deployed
174+
// and ready for the Istio revision to be considered healthy. The DependenciesHealthy condition is used to indicate that
175+
// the IstioCNI resource is healthy.
176+
IstioRevisionConditionDependenciesHealthy IstioRevisionConditionType = "DependenciesHealthy"
177+
178+
// IstioRevisionReasonIstioCNINotFound indicates that the IstioCNI resource is not found.
179+
IstioRevisionReasonIstioCNINotFound IstioRevisionConditionReason = "IstioCNINotFound"
180+
181+
// IstioRevisionReasonIstioCNINotHealthy indicates that the IstioCNI resource is not healthy.
182+
IstioRevisionReasonIstioCNINotHealthy IstioRevisionConditionReason = "IstioCNINotHealthy"
183+
184+
// IstioRevisionDependencyCheckFailed indicates that the status of the dependencies could not be ascertained.
185+
IstioRevisionDependencyCheckFailed IstioRevisionConditionReason = "DependencyCheckFailed"
186+
)
187+
171188
const (
172189
// IstioRevisionReasonHealthy indicates that the control plane is fully reconciled and that all components are ready.
173190
IstioRevisionReasonHealthy IstioRevisionConditionReason = "Healthy"

‎controllers/istio/istio_controller.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ func (r *Reconciler) determineStatus(ctx context.Context, istio *v1.Istio, recon
257257
} else if err == nil {
258258
status.SetCondition(convertCondition(rev.Status.GetCondition(v1.IstioRevisionConditionReconciled)))
259259
status.SetCondition(convertCondition(rev.Status.GetCondition(v1.IstioRevisionConditionReady)))
260+
status.SetCondition(convertCondition(rev.Status.GetCondition(v1.IstioRevisionConditionDependenciesHealthy)))
260261
status.State = convertState(rev.Status.State)
261262
} else {
262263
activeRevisionGetFailed := func(conditionType v1.IstioConditionType) v1.IstioCondition {

‎controllers/istio/istio_controller_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ func TestDetermineStatus(t *testing.T) {
265265
Conditions: []v1.IstioRevisionCondition{
266266
{Type: v1.IstioRevisionConditionReconciled, Status: toConditionStatus(reconciled)},
267267
{Type: v1.IstioRevisionConditionReady, Status: toConditionStatus(ready)},
268+
{Type: v1.IstioRevisionConditionDependenciesHealthy, Status: toConditionStatus(true)},
268269
{Type: v1.IstioRevisionConditionInUse, Status: toConditionStatus(inUse)},
269270
},
270271
},
@@ -330,6 +331,12 @@ func TestDetermineStatus(t *testing.T) {
330331
Reason: v1.IstioRevisionReasonHealthy,
331332
Message: "ready message",
332333
},
334+
{
335+
Type: v1.IstioRevisionConditionDependenciesHealthy,
336+
Status: metav1.ConditionTrue,
337+
Reason: v1.IstioRevisionReasonHealthy,
338+
Message: "dependencies healthy message",
339+
},
333340
},
334341
},
335342
},
@@ -376,6 +383,12 @@ func TestDetermineStatus(t *testing.T) {
376383
Reason: v1.IstioReasonHealthy,
377384
Message: "ready message",
378385
},
386+
{
387+
Type: v1.IstioConditionDependenciesHealthy,
388+
Status: metav1.ConditionTrue,
389+
Reason: v1.IstioReasonHealthy,
390+
Message: "dependencies healthy message",
391+
},
379392
},
380393
ActiveRevisionName: istioKey.Name,
381394
Revisions: v1.RevisionSummary{
@@ -408,6 +421,10 @@ func TestDetermineStatus(t *testing.T) {
408421
Type: v1.IstioConditionReady,
409422
Status: metav1.ConditionTrue,
410423
},
424+
{
425+
Type: v1.IstioConditionDependenciesHealthy,
426+
Status: metav1.ConditionTrue,
427+
},
411428
},
412429
ActiveRevisionName: istioKey.Name,
413430
Revisions: v1.RevisionSummary{
@@ -639,6 +656,11 @@ func TestUpdateStatus(t *testing.T) {
639656
Message: "ready message",
640657
LastTransitionTime: *oneMinuteAgo,
641658
},
659+
{
660+
Type: v1.IstioConditionDependenciesHealthy,
661+
Status: metav1.ConditionTrue,
662+
LastTransitionTime: *oneMinuteAgo,
663+
},
642664
},
643665
ActiveRevisionName: istioKey.Name,
644666
},
@@ -668,6 +690,11 @@ func TestUpdateStatus(t *testing.T) {
668690
Message: "ready message",
669691
LastTransitionTime: *oneMinuteAgo,
670692
},
693+
{
694+
Type: v1.IstioRevisionConditionDependenciesHealthy,
695+
Status: metav1.ConditionTrue,
696+
LastTransitionTime: *oneMinuteAgo,
697+
},
671698
},
672699
},
673700
},
@@ -688,6 +715,10 @@ func TestUpdateStatus(t *testing.T) {
688715
Reason: v1.IstioReasonHealthy,
689716
Message: "ready message",
690717
},
718+
{
719+
Type: v1.IstioConditionDependenciesHealthy,
720+
Status: metav1.ConditionTrue,
721+
},
691722
},
692723
ActiveRevisionName: istioKey.Name,
693724
},

‎controllers/istiorevision/istiorevision_controller.go

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ import (
5757
"istio.io/istio/pkg/ptr"
5858
)
5959

60+
const istioCniName = "default"
61+
6062
// Reconciler reconciles an IstioRevision object
6163
type Reconciler struct {
6264
client.Client
@@ -213,6 +215,8 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
213215
// The handler triggers the reconciliation of the referenced IstioRevision CR so that its InUse condition is updated.
214216
revisionTagHandler := wrapEventHandler(logger, handler.EnqueueRequestsFromMapFunc(r.mapRevisionTagToReconcileRequest))
215217

218+
istioCniHandler := wrapEventHandler(logger, handler.EnqueueRequestsFromMapFunc(r.mapIstioCniToReconcileRequests))
219+
216220
return ctrl.NewControllerManagedBy(mgr).
217221
WithOptions(controller.Options{
218222
LogConstructor: func(req *reconcile.Request) logr.Logger {
@@ -258,6 +262,9 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
258262
Watches(&admissionv1.MutatingWebhookConfiguration{}, ownedResourceHandler).
259263
Watches(&admissionv1.ValidatingWebhookConfiguration{}, ownedResourceHandler, builder.WithPredicates(validatingWebhookConfigPredicate())).
260264

265+
// +lint-watches:ignore: IstioCNI (not found in charts, but this controller needs to watch it to update the IstioRevision status)
266+
Watches(&v1.IstioCNI{}, istioCniHandler).
267+
261268
// +lint-watches:ignore: ValidatingAdmissionPolicy (TODO: fix this when CI supports golang 1.22 and k8s 1.30)
262269
// +lint-watches:ignore: ValidatingAdmissionPolicyBinding (TODO: fix this when CI supports golang 1.22 and k8s 1.30)
263270
// +lint-watches:ignore: CustomResourceDefinition (prevents `make lint-watches` from bugging us about CRDs)
@@ -269,6 +276,8 @@ func (r *Reconciler) determineStatus(ctx context.Context, rev *v1.IstioRevision,
269276
reconciledCondition := r.determineReconciledCondition(reconcileErr)
270277
readyCondition, err := r.determineReadyCondition(ctx, rev)
271278
errs.Add(err)
279+
dependenciesHealthyCondition, err := r.determineDependenciesHealthyCondition(ctx, rev)
280+
errs.Add(err)
272281

273282
inUseCondition, err := r.determineInUseCondition(ctx, rev)
274283
errs.Add(err)
@@ -277,8 +286,9 @@ func (r *Reconciler) determineStatus(ctx context.Context, rev *v1.IstioRevision,
277286
status.ObservedGeneration = rev.Generation
278287
status.SetCondition(reconciledCondition)
279288
status.SetCondition(readyCondition)
289+
status.SetCondition(dependenciesHealthyCondition)
280290
status.SetCondition(inUseCondition)
281-
status.State = deriveState(reconciledCondition, readyCondition)
291+
status.State = deriveState(reconciledCondition, readyCondition, dependenciesHealthyCondition)
282292
return status, errs.Error()
283293
}
284294

@@ -298,11 +308,11 @@ func (r *Reconciler) updateStatus(ctx context.Context, rev *v1.IstioRevision, re
298308
return errs.Error()
299309
}
300310

301-
func deriveState(reconciledCondition, readyCondition v1.IstioRevisionCondition) v1.IstioRevisionConditionReason {
302-
if reconciledCondition.Status != metav1.ConditionTrue {
303-
return reconciledCondition.Reason
304-
} else if readyCondition.Status != metav1.ConditionTrue {
305-
return readyCondition.Reason
311+
func deriveState(conditions ...v1.IstioRevisionCondition) v1.IstioRevisionConditionReason {
312+
for _, c := range conditions {
313+
if c.Status != metav1.ConditionTrue {
314+
return c.Reason
315+
}
306316
}
307317
return v1.IstioRevisionReasonHealthy
308318
}
@@ -375,6 +385,43 @@ func (r *Reconciler) determineReadyCondition(ctx context.Context, rev *v1.IstioR
375385
return c, nil
376386
}
377387

388+
func (r *Reconciler) determineDependenciesHealthyCondition(ctx context.Context, rev *v1.IstioRevision) (v1.IstioRevisionCondition, error) {
389+
if revision.DependsOnIstioCNI(rev) {
390+
cni := v1.IstioCNI{}
391+
if err := r.Client.Get(ctx, client.ObjectKey{Name: istioCniName}, &cni); err != nil {
392+
if apierrors.IsNotFound(err) {
393+
return v1.IstioRevisionCondition{
394+
Type: v1.IstioRevisionConditionDependenciesHealthy,
395+
Status: metav1.ConditionFalse,
396+
Reason: v1.IstioRevisionReasonIstioCNINotFound,
397+
Message: "IstioCNI resource does not exist",
398+
}, nil
399+
}
400+
401+
return v1.IstioRevisionCondition{
402+
Type: v1.IstioRevisionConditionDependenciesHealthy,
403+
Status: metav1.ConditionUnknown,
404+
Reason: v1.IstioRevisionDependencyCheckFailed,
405+
Message: fmt.Sprintf("failed to get IstioCNI status: %v", err),
406+
}, fmt.Errorf("get failed: %w", err)
407+
}
408+
409+
if cni.Status.State != v1.IstioCNIReasonHealthy {
410+
return v1.IstioRevisionCondition{
411+
Type: v1.IstioRevisionConditionDependenciesHealthy,
412+
Status: metav1.ConditionFalse,
413+
Reason: v1.IstioRevisionReasonIstioCNINotHealthy,
414+
Message: "IstioCNI resource status indicates that the component is not healthy",
415+
}, nil
416+
}
417+
}
418+
419+
return v1.IstioRevisionCondition{
420+
Type: v1.IstioRevisionConditionDependenciesHealthy,
421+
Status: metav1.ConditionTrue,
422+
}, nil
423+
}
424+
378425
func (r *Reconciler) determineInUseCondition(ctx context.Context, rev *v1.IstioRevision) (v1.IstioRevisionCondition, error) {
379426
c := v1.IstioRevisionCondition{Type: v1.IstioRevisionConditionInUse}
380427

@@ -550,6 +597,21 @@ func (r *Reconciler) mapRevisionTagToReconcileRequest(ctx context.Context, revis
550597
return nil
551598
}
552599

600+
// mapIstioCniToReconcileRequests returns reconcile requests for all IstioRevisions that depend on IstioCNI
601+
func (r *Reconciler) mapIstioCniToReconcileRequests(ctx context.Context, _ client.Object) []reconcile.Request {
602+
list := v1.IstioRevisionList{}
603+
if err := r.Client.List(ctx, &list); err != nil {
604+
return nil
605+
}
606+
var reqs []reconcile.Request
607+
for _, rev := range list.Items {
608+
if revision.DependsOnIstioCNI(&rev) {
609+
reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{Name: rev.Name}})
610+
}
611+
}
612+
return reqs
613+
}
614+
553615
// ignoreStatusChange returns a predicate that ignores watch events where only the resource status changes; if
554616
// there are any other changes to the resource, the event is not ignored.
555617
// This ensures that the controller doesn't reconcile the entire IstioRevision every time the status of an owned

‎controllers/istiorevision/istiorevision_controller_test.go

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -205,47 +205,70 @@ func TestValidate(t *testing.T) {
205205

206206
func TestDeriveState(t *testing.T) {
207207
testCases := []struct {
208-
name string
209-
reconciledCondition v1.IstioRevisionCondition
210-
readyCondition v1.IstioRevisionCondition
211-
expectedState v1.IstioRevisionConditionReason
208+
name string
209+
conditions []v1.IstioRevisionCondition
210+
expectedState v1.IstioRevisionConditionReason
212211
}{
213212
{
214-
name: "healthy",
215-
reconciledCondition: newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionTrue, ""),
216-
readyCondition: newCondition(v1.IstioRevisionConditionReady, metav1.ConditionTrue, ""),
217-
expectedState: v1.IstioRevisionReasonHealthy,
213+
name: "healthy",
214+
conditions: []v1.IstioRevisionCondition{
215+
newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionTrue, ""),
216+
newCondition(v1.IstioRevisionConditionReady, metav1.ConditionTrue, ""),
217+
newCondition(v1.IstioRevisionConditionDependenciesHealthy, metav1.ConditionTrue, ""),
218+
},
219+
expectedState: v1.IstioRevisionReasonHealthy,
218220
},
219221
{
220-
name: "not reconciled",
221-
reconciledCondition: newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionFalse, v1.IstioRevisionReasonReconcileError),
222-
readyCondition: newCondition(v1.IstioRevisionConditionReady, metav1.ConditionTrue, ""),
223-
expectedState: v1.IstioRevisionReasonReconcileError,
222+
name: "not reconciled",
223+
conditions: []v1.IstioRevisionCondition{
224+
newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionFalse, v1.IstioRevisionReasonReconcileError),
225+
newCondition(v1.IstioRevisionConditionReady, metav1.ConditionTrue, ""),
226+
newCondition(v1.IstioRevisionConditionDependenciesHealthy, metav1.ConditionTrue, ""),
227+
},
228+
expectedState: v1.IstioRevisionReasonReconcileError,
224229
},
225230
{
226-
name: "not ready",
227-
reconciledCondition: newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionTrue, ""),
228-
readyCondition: newCondition(v1.IstioRevisionConditionReady, metav1.ConditionFalse, v1.IstioRevisionReasonIstiodNotReady),
229-
expectedState: v1.IstioRevisionReasonIstiodNotReady,
231+
name: "not ready",
232+
conditions: []v1.IstioRevisionCondition{
233+
newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionTrue, ""),
234+
newCondition(v1.IstioRevisionConditionReady, metav1.ConditionFalse, v1.IstioRevisionReasonIstiodNotReady),
235+
newCondition(v1.IstioRevisionConditionDependenciesHealthy, metav1.ConditionTrue, ""),
236+
},
237+
expectedState: v1.IstioRevisionReasonIstiodNotReady,
230238
},
231239
{
232-
name: "readiness unknown",
233-
reconciledCondition: newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionTrue, ""),
234-
readyCondition: newCondition(v1.IstioRevisionConditionReady, metav1.ConditionUnknown, v1.IstioRevisionReasonReadinessCheckFailed),
235-
expectedState: v1.IstioRevisionReasonReadinessCheckFailed,
240+
name: "readiness unknown",
241+
conditions: []v1.IstioRevisionCondition{
242+
newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionTrue, ""),
243+
newCondition(v1.IstioRevisionConditionReady, metav1.ConditionUnknown, v1.IstioRevisionReasonReadinessCheckFailed),
244+
newCondition(v1.IstioRevisionConditionDependenciesHealthy, metav1.ConditionTrue, ""),
245+
},
246+
expectedState: v1.IstioRevisionReasonReadinessCheckFailed,
236247
},
237248
{
238-
name: "not reconciled nor ready",
239-
reconciledCondition: newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionFalse, v1.IstioRevisionReasonReconcileError),
240-
readyCondition: newCondition(v1.IstioRevisionConditionReady, metav1.ConditionFalse, v1.IstioRevisionReasonIstiodNotReady),
241-
expectedState: v1.IstioRevisionReasonReconcileError, // reconcile reason takes precedence over ready reason
249+
name: "not reconciled nor ready",
250+
conditions: []v1.IstioRevisionCondition{
251+
newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionFalse, v1.IstioRevisionReasonReconcileError),
252+
newCondition(v1.IstioRevisionConditionReady, metav1.ConditionFalse, v1.IstioRevisionReasonIstiodNotReady),
253+
newCondition(v1.IstioRevisionConditionDependenciesHealthy, metav1.ConditionTrue, ""),
254+
},
255+
expectedState: v1.IstioRevisionReasonReconcileError, // reconcile reason takes precedence over ready reason
256+
},
257+
{
258+
name: "dependencies not ready",
259+
conditions: []v1.IstioRevisionCondition{
260+
newCondition(v1.IstioRevisionConditionReconciled, metav1.ConditionTrue, ""),
261+
newCondition(v1.IstioRevisionConditionReady, metav1.ConditionTrue, ""),
262+
newCondition(v1.IstioRevisionConditionDependenciesHealthy, metav1.ConditionFalse, v1.IstioRevisionReasonIstioCNINotHealthy),
263+
},
264+
expectedState: v1.IstioRevisionReasonIstioCNINotHealthy,
242265
},
243266
}
244267

245268
for _, tc := range testCases {
246269
t.Run(tc.name, func(t *testing.T) {
247270
g := NewWithT(t)
248-
result := deriveState(tc.reconciledCondition, tc.readyCondition)
271+
result := deriveState(tc.conditions...)
249272
g.Expect(result).To(Equal(tc.expectedState))
250273
})
251274
}

0 commit comments

Comments
 (0)