Skip to content

Commit

Permalink
Output info message when clusteringress's status was updated successf…
Browse files Browse the repository at this point in the history
…ully (knative#4144)

When `config-domain` configmap was updated, event log from
clusteringress sometimes tells us that `Failed to update status for
ClusterIngress`. Although this error would be solved automatically by
next reconcile loop, we don't know whether it was solved or not
because there are no success log.

To improve it, this patch adds success logs when clusteringress
was updated successfully.
  • Loading branch information
nak3 authored and JRBANCEL committed May 29, 2019
1 parent e9cada2 commit 5fc1cf1
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 5 deletions.
16 changes: 11 additions & 5 deletions pkg/reconciler/clusteringress/clusteringress.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,17 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
// This is important because the copy we loaded from the informer's
// cache may be stale and we don't want to overwrite a prior update
// to status with this stale state.
} else if _, err = c.updateStatus(ci); err != nil {
logger.Warnw("Failed to update clusterIngress status", zap.Error(err))
c.Recorder.Eventf(ci, corev1.EventTypeWarning, "UpdateFailed",
"Failed to update status for ClusterIngress %q: %v", ci.Name, err)
return err
} else {
if _, err = c.updateStatus(ci); err != nil {
logger.Warnw("Failed to update ClusterIngress status", zap.Error(err))
c.Recorder.Eventf(ci, corev1.EventTypeWarning, "UpdateFailed",
"Failed to update status for ClusterIngress %q: %v", ci.Name, err)
return err
} else {
logger.Infof("Updated status for ClusterIngress %q", ci.Name)
c.Recorder.Eventf(ci, corev1.EventTypeNormal, "Updated",
"Updated status for ClusterIngress %q", ci.Name)
}
}
if reconcileErr != nil {
c.Recorder.Event(ci, corev1.EventTypeWarning, "InternalError", reconcileErr.Error())
Expand Down
4 changes: 4 additions & 0 deletions pkg/reconciler/clusteringress/clusteringress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ func TestReconcile_EnableAutoTLS(t *testing.T) {
WantEvents: []string{
Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-clusteringress"),
Eventf(corev1.EventTypeNormal, "Updated", "Updated Gateway %q/%q", system.Namespace(), "knative-ingress-gateway"),
Eventf(corev1.EventTypeNormal, "Updated", "Updated status for ClusterIngress %q", "reconciling-clusteringress"),
},
Key: "reconciling-clusteringress",
}, {
Expand Down Expand Up @@ -256,6 +257,7 @@ func TestReconcile_EnableAutoTLS(t *testing.T) {
}},
WantEvents: []string{
Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-clusteringress"),
Eventf(corev1.EventTypeNormal, "Updated", "Updated status for ClusterIngress %q", "reconciling-clusteringress"),
Eventf(corev1.EventTypeWarning, "InternalError", `gateway.networking.istio.io "knative-ingress-gateway" not found`),
},
// Error should be returned when there is no preinstalled gateways.
Expand Down Expand Up @@ -344,6 +346,7 @@ func TestReconcile_EnableAutoTLS(t *testing.T) {
Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-clusteringress"),
Eventf(corev1.EventTypeNormal, "Created", "Created Secret %s/%s", "istio-system", targetSecretName),
Eventf(corev1.EventTypeNormal, "Updated", "Updated Gateway %q/%q", system.Namespace(), "knative-ingress-gateway"),
Eventf(corev1.EventTypeNormal, "Updated", "Updated status for ClusterIngress %q", "reconciling-clusteringress"),
},
Key: "reconciling-clusteringress",
}, {
Expand Down Expand Up @@ -427,6 +430,7 @@ func TestReconcile_EnableAutoTLS(t *testing.T) {
WantEvents: []string{
Eventf(corev1.EventTypeNormal, "Created", "Created VirtualService %q", "reconciling-clusteringress"),
Eventf(corev1.EventTypeNormal, "Updated", "Updated Secret %s/%s", "istio-system", targetSecretName),
Eventf(corev1.EventTypeNormal, "Updated", "Updated status for ClusterIngress %q", "reconciling-clusteringress"),
},
Key: "reconciling-clusteringress",
}}
Expand Down

0 comments on commit 5fc1cf1

Please sign in to comment.