From f1102ec097ed5b312e38aa74a8f7162ffb1c22b2 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Sat, 16 Nov 2019 20:00:48 +0900 Subject: [PATCH 1/3] Mark LoadBalancerReady false when VirtualService failed to be reconciled Currently LoadBalancerReady is not updated even when VirtualService failed to be reconciled. This patch updated LoadBalancerReady with the reason and message by using returned error. --- pkg/apis/networking/v1alpha1/ingress_lifecycle.go | 9 ++++++++- pkg/apis/networking/v1alpha1/ingress_lifecycle_test.go | 6 +++++- pkg/reconciler/ingress/ingress.go | 3 ++- pkg/reconciler/ingress/ingress_test.go | 9 ++++++--- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/pkg/apis/networking/v1alpha1/ingress_lifecycle.go b/pkg/apis/networking/v1alpha1/ingress_lifecycle.go index f227e75afeb4..d727bbf25f78 100644 --- a/pkg/apis/networking/v1alpha1/ingress_lifecycle.go +++ b/pkg/apis/networking/v1alpha1/ingress_lifecycle.go @@ -29,6 +29,8 @@ var ingressCondSet = apis.NewLivingConditionSet( IngressConditionLoadBalancerReady, ) +var VirtualServiceNotReconciledReason = "ReconcileVirtualServiceFailed" + // GetGroupVersionKind returns SchemeGroupVersion of an Ingress func (i *Ingress) GetGroupVersionKind() schema.GroupVersionKind { return SchemeGroupVersion.WithKind("Ingress") @@ -73,11 +75,16 @@ func (is *IngressStatus) MarkLoadBalancerReady(lbs []LoadBalancerIngressStatus, // MarkLoadBalancerPending marks the "IngressConditionLoadBalancerReady" condition to unknown to // reflect that the load balancer is not ready yet. -func (is *IngressStatus) MarkLoadBalancerPending() { +func (is *IngressStatus) MarkLoadBalancerNotReady() { ingressCondSet.Manage(is).MarkUnknown(IngressConditionLoadBalancerReady, "Uninitialized", "Waiting for VirtualService to be ready") } +// MarkLoadBalancerFailed marks the "IngressConditionLoadBalancerReady" condition to false. +func (is *IngressStatus) MarkLoadBalancerFailed(reason, message string) { + ingressCondSet.Manage(is).MarkFalse(IngressConditionLoadBalancerReady, reason, message) +} + // MarkIngressNotReady marks the "IngressConditionReady" condition to unknown. func (is *IngressStatus) MarkIngressNotReady(reason, message string) { ingressCondSet.Manage(is).MarkUnknown(IngressConditionReady, reason, message) diff --git a/pkg/apis/networking/v1alpha1/ingress_lifecycle_test.go b/pkg/apis/networking/v1alpha1/ingress_lifecycle_test.go index 2ee77d9b1c14..284d7a6c7c01 100644 --- a/pkg/apis/networking/v1alpha1/ingress_lifecycle_test.go +++ b/pkg/apis/networking/v1alpha1/ingress_lifecycle_test.go @@ -83,10 +83,14 @@ func TestIngressTypicalFlow(t *testing.T) { apitestv1.CheckConditionOngoing(r.duck(), IngressConditionReady, t) // Then ingress is pending. - r.MarkLoadBalancerPending() + r.MarkLoadBalancerNotReady() apitestv1.CheckConditionOngoing(r.duck(), IngressConditionLoadBalancerReady, t) apitestv1.CheckConditionOngoing(r.duck(), IngressConditionReady, t) + r.MarkLoadBalancerFailed("some reason", "some message") + apitestv1.CheckConditionFailed(r.duck(), IngressConditionLoadBalancerReady, t) + apitestv1.CheckConditionFailed(r.duck(), IngressConditionLoadBalancerReady, t) + // Then ingress has address. r.MarkLoadBalancerReady( []LoadBalancerIngressStatus{{DomainInternal: "gateway.default.svc"}}, diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index 5cf2f7c3a29a..c262d60c4fa1 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -168,6 +168,7 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ia *v1alpha1.Ingress) logger.Infof("Creating/Updating VirtualServices") ia.Status.ObservedGeneration = ia.GetGeneration() if err := r.reconcileVirtualServices(ctx, ia, vses); err != nil { + ia.Status.MarkLoadBalancerFailed(v1alpha1.VirtualServiceNotReconciledReason, err.Error()) return err } @@ -218,7 +219,7 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ia *v1alpha1.Ingress) privateLbs := getLBStatus(privateGatewayServiceURLFromContext(ctx)) ia.Status.MarkLoadBalancerReady(lbs, publicLbs, privateLbs) } else { - ia.Status.MarkLoadBalancerPending() + ia.Status.MarkLoadBalancerNotReady() } // TODO(zhiminx): Mark Route status to indicate that Gateway is configured. diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index 5312d14d7272..6033b7acf4b4 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -278,14 +278,17 @@ func TestReconcile(t *testing.T) { v1alpha1.IngressStatus{ Status: duckv1.Status{ Conditions: duckv1.Conditions{{ - Type: v1alpha1.IngressConditionLoadBalancerReady, - Status: corev1.ConditionTrue, + Type: v1alpha1.IngressConditionLoadBalancerReady, + Reason: v1alpha1.VirtualServiceNotReconciledReason, + Severity: apis.ConditionSeverityError, + Message: "failed to update VirtualService: inducing failure for update virtualservices", + Status: corev1.ConditionFalse, }, { Type: v1alpha1.IngressConditionNetworkConfigured, Status: corev1.ConditionTrue, }, { Type: v1alpha1.IngressConditionReady, - Status: corev1.ConditionUnknown, + Status: corev1.ConditionFalse, Severity: apis.ConditionSeverityError, Reason: notReconciledReason, Message: notReconciledMessage, From 433605fd877ccef8bff646c2b3eb3528b7a0bd46 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Sat, 16 Nov 2019 20:50:33 +0900 Subject: [PATCH 2/3] Fix go lint warning --- pkg/apis/networking/v1alpha1/ingress_lifecycle.go | 4 +++- pkg/reconciler/ingress/ingress.go | 2 +- pkg/reconciler/ingress/ingress_test.go | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/apis/networking/v1alpha1/ingress_lifecycle.go b/pkg/apis/networking/v1alpha1/ingress_lifecycle.go index d727bbf25f78..be98e8d7316c 100644 --- a/pkg/apis/networking/v1alpha1/ingress_lifecycle.go +++ b/pkg/apis/networking/v1alpha1/ingress_lifecycle.go @@ -29,7 +29,9 @@ var ingressCondSet = apis.NewLivingConditionSet( IngressConditionLoadBalancerReady, ) -var VirtualServiceNotReconciledReason = "ReconcileVirtualServiceFailed" +// VirtualServiceNotReconciled is used for the reason of MarkLoadBalancerFailed +// when VirtualService is failed to be reconciled. +var VirtualServiceNotReconciled = "ReconcileVirtualServiceFailed" // GetGroupVersionKind returns SchemeGroupVersion of an Ingress func (i *Ingress) GetGroupVersionKind() schema.GroupVersionKind { diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index c262d60c4fa1..3836f757fcf8 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -168,7 +168,7 @@ func (r *Reconciler) reconcileIngress(ctx context.Context, ia *v1alpha1.Ingress) logger.Infof("Creating/Updating VirtualServices") ia.Status.ObservedGeneration = ia.GetGeneration() if err := r.reconcileVirtualServices(ctx, ia, vses); err != nil { - ia.Status.MarkLoadBalancerFailed(v1alpha1.VirtualServiceNotReconciledReason, err.Error()) + ia.Status.MarkLoadBalancerFailed(v1alpha1.VirtualServiceNotReconciled, err.Error()) return err } diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index 6033b7acf4b4..e2623c920e27 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -279,7 +279,7 @@ func TestReconcile(t *testing.T) { Status: duckv1.Status{ Conditions: duckv1.Conditions{{ Type: v1alpha1.IngressConditionLoadBalancerReady, - Reason: v1alpha1.VirtualServiceNotReconciledReason, + Reason: v1alpha1.VirtualServiceNotReconciled, Severity: apis.ConditionSeverityError, Message: "failed to update VirtualService: inducing failure for update virtualservices", Status: corev1.ConditionFalse, From 069a51bb990040b04b95b647b33b41e19f2a749f Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 18 Nov 2019 17:55:49 +0900 Subject: [PATCH 3/3] Fix comment --- pkg/apis/networking/v1alpha1/ingress_lifecycle.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/networking/v1alpha1/ingress_lifecycle.go b/pkg/apis/networking/v1alpha1/ingress_lifecycle.go index be98e8d7316c..fe840dc5069c 100644 --- a/pkg/apis/networking/v1alpha1/ingress_lifecycle.go +++ b/pkg/apis/networking/v1alpha1/ingress_lifecycle.go @@ -75,7 +75,7 @@ func (is *IngressStatus) MarkLoadBalancerReady(lbs []LoadBalancerIngressStatus, ingressCondSet.Manage(is).MarkTrue(IngressConditionLoadBalancerReady) } -// MarkLoadBalancerPending marks the "IngressConditionLoadBalancerReady" condition to unknown to +// MarkLoadBalancerNotReady marks the "IngressConditionLoadBalancerReady" condition to unknown to // reflect that the load balancer is not ready yet. func (is *IngressStatus) MarkLoadBalancerNotReady() { ingressCondSet.Manage(is).MarkUnknown(IngressConditionLoadBalancerReady, "Uninitialized",