Merge pull request #759 from fluxcd/log-on-new-artifact

stefanprodan · web-flow · commit ea1bab5a734e · 2022-06-03T16:15:17.000+03:00
Log on new artifact and failure recovery
diff --git a/controllers/bucket_controller.go b/controllers/bucket_controller.go
@@ -370,13 +370,13 @@ func (r *BucketReconciler) reconcile(ctx context.Context, obj *sourcev1.Bucket,
 		res = sreconcile.LowestRequeuingResult(res, recResult)
 	}
 
-	r.notify(oldObj, obj, index, res, resErr)
+	r.notify(ctx, oldObj, obj, index, res, resErr)
 
 	return res, resErr
 }
 
 // notify emits notification related to the reconciliation.
-func (r *BucketReconciler) notify(oldObj, newObj *sourcev1.Bucket, index *etagIndex, res sreconcile.Result, resErr error) {
+func (r *BucketReconciler) notify(ctx context.Context, oldObj, newObj *sourcev1.Bucket, index *etagIndex, res sreconcile.Result, resErr error) {
 	// Notify successful reconciliation for new artifact and recovery from any
 	// failure.
 	if resErr == nil && res == sreconcile.ResultSuccess && newObj.Status.Artifact != nil {
@@ -396,10 +396,12 @@ func (r *BucketReconciler) notify(oldObj, newObj *sourcev1.Bucket, index *etagIn
 		if oldChecksum != newObj.GetArtifact().Checksum {
 			r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,
 				"NewArtifact", message)
+			ctrl.LoggerFrom(ctx).Info(message)
 		} else {
 			if sreconcile.FailureRecovery(oldObj, newObj, bucketFailConditions) {
 				r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,
 					meta.SucceededReason, message)
+				ctrl.LoggerFrom(ctx).Info(message)
 			}
 		}
 	}
diff --git a/controllers/bucket_controller_test.go b/controllers/bucket_controller_test.go
@@ -1264,7 +1264,7 @@ func TestBucketReconciler_notify(t *testing.T) {
 					"bbb": "ddd",
 				},
 			}
-			reconciler.notify(oldObj, newObj, index, tt.res, tt.resErr)
+			reconciler.notify(ctx, oldObj, newObj, index, tt.res, tt.resErr)
 
 			select {
 			case x, ok := <-recorder.Events:
diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go
@@ -286,13 +286,13 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.G
 		res = sreconcile.LowestRequeuingResult(res, recResult)
 	}
 
-	r.notify(oldObj, obj, commit, res, resErr)
+	r.notify(ctx, oldObj, obj, commit, res, resErr)
 
 	return res, resErr
 }
 
 // notify emits notification related to the result of reconciliation.
-func (r *GitRepositoryReconciler) notify(oldObj, newObj *sourcev1.GitRepository, commit git.Commit, res sreconcile.Result, resErr error) {
+func (r *GitRepositoryReconciler) notify(ctx context.Context, oldObj, newObj *sourcev1.GitRepository, commit git.Commit, res sreconcile.Result, resErr error) {
 	// Notify successful reconciliation for new artifact, no-op reconciliation
 	// and recovery from any failure.
 	if r.shouldNotify(oldObj, newObj, res, resErr) {
@@ -319,10 +319,12 @@ func (r *GitRepositoryReconciler) notify(oldObj, newObj *sourcev1.GitRepository,
 		if oldChecksum != newObj.GetArtifact().Checksum {
 			r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,
 				"NewArtifact", message)
+			ctrl.LoggerFrom(ctx).Info(message)
 		} else {
 			if sreconcile.FailureRecovery(oldObj, newObj, gitRepositoryFailConditions) {
 				r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,
 					meta.SucceededReason, message)
+				ctrl.LoggerFrom(ctx).Info(message)
 			}
 		}
 	}
diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go
@@ -1928,7 +1928,7 @@ func TestGitRepositoryReconciler_notify(t *testing.T) {
 				EventRecorder: recorder,
 				features:      features.FeatureGates(),
 			}
-			reconciler.notify(oldObj, newObj, tt.commit, tt.res, tt.resErr)
+			reconciler.notify(ctx, oldObj, newObj, tt.commit, tt.res, tt.resErr)
 
 			select {
 			case x, ok := <-recorder.Events:
diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go
@@ -287,13 +287,13 @@ func (r *HelmChartReconciler) reconcile(ctx context.Context, obj *sourcev1.HelmC
 		res = sreconcile.LowestRequeuingResult(res, recResult)
 	}
 
-	r.notify(oldObj, obj, &build, res, resErr)
+	r.notify(ctx, oldObj, obj, &build, res, resErr)
 
 	return res, resErr
 }
 
 // notify emits notification related to the reconciliation.
-func (r *HelmChartReconciler) notify(oldObj, newObj *sourcev1.HelmChart, build *chart.Build, res sreconcile.Result, resErr error) {
+func (r *HelmChartReconciler) notify(ctx context.Context, oldObj, newObj *sourcev1.HelmChart, build *chart.Build, res sreconcile.Result, resErr error) {
 	// Notify successful reconciliation for new artifact and recovery from any
 	// failure.
 	if resErr == nil && res == sreconcile.ResultSuccess && newObj.Status.Artifact != nil {
@@ -311,10 +311,12 @@ func (r *HelmChartReconciler) notify(oldObj, newObj *sourcev1.HelmChart, build *
 		if oldChecksum != newObj.GetArtifact().Checksum {
 			r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,
 				reasonForBuild(build), build.Summary())
+			ctrl.LoggerFrom(ctx).Info(build.Summary())
 		} else {
 			if sreconcile.FailureRecovery(oldObj, newObj, helmChartFailConditions) {
 				r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,
 					reasonForBuild(build), build.Summary())
+				ctrl.LoggerFrom(ctx).Info(build.Summary())
 			}
 		}
 	}
diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go
@@ -1917,7 +1917,7 @@ func TestHelmChartReconciler_notify(t *testing.T) {
 				Path:     "some/path",
 				Packaged: true,
 			}
-			reconciler.notify(oldObj, newObj, build, tt.res, tt.resErr)
+			reconciler.notify(ctx, oldObj, newObj, build, tt.res, tt.resErr)
 
 			select {
 			case x, ok := <-recorder.Events:
diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go
@@ -250,13 +250,13 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.
 		res = sreconcile.LowestRequeuingResult(res, recResult)
 	}
 
-	r.notify(oldObj, obj, chartRepo, res, resErr)
+	r.notify(ctx, oldObj, obj, chartRepo, res, resErr)
 
 	return res, resErr
 }
 
 // notify emits notification related to the reconciliation.
-func (r *HelmRepositoryReconciler) notify(oldObj, newObj *sourcev1.HelmRepository, chartRepo repository.ChartRepository, res sreconcile.Result, resErr error) {
+func (r *HelmRepositoryReconciler) notify(ctx context.Context, oldObj, newObj *sourcev1.HelmRepository, chartRepo repository.ChartRepository, res sreconcile.Result, resErr error) {
 	// Notify successful reconciliation for new artifact and recovery from any
 	// failure.
 	if resErr == nil && res == sreconcile.ResultSuccess && newObj.Status.Artifact != nil {
@@ -281,11 +281,13 @@ func (r *HelmRepositoryReconciler) notify(oldObj, newObj *sourcev1.HelmRepositor
 		if oldChecksum != newObj.GetArtifact().Checksum {
 			r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,
 				"NewArtifact", message)
+			ctrl.LoggerFrom(ctx).Info(message)
 		} else {
 			if sreconcile.FailureRecovery(oldObj, newObj, helmRepositoryFailConditions) {
 				r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,
 					meta.SucceededReason, message)
 			}
+			ctrl.LoggerFrom(ctx).Info(message)
 		}
 	}
 }
diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go
@@ -229,11 +229,11 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta
 		ready := conditions.Get(obj, meta.ReadyCondition)
 		// Became ready from not ready.
 		if !conditions.IsReady(oldObj) && conditions.IsReady(obj) {
-			r.Eventf(obj, corev1.EventTypeNormal, ready.Reason, ready.Message)
+			r.eventLogf(ctx, obj, corev1.EventTypeNormal, ready.Reason, ready.Message)
 		}
 		// Became not ready from ready.
 		if conditions.IsReady(oldObj) && !conditions.IsReady(obj) {
-			r.Eventf(obj, corev1.EventTypeWarning, ready.Reason, ready.Message)
+			r.eventLogf(ctx, obj, corev1.EventTypeWarning, ready.Reason, ready.Message)
 		}
 	}()
 
diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go
@@ -1069,7 +1069,7 @@ func TestHelmRepositoryReconciler_notify(t *testing.T) {
 			chartRepo := repository.ChartRepository{
 				URL: "some-address",
 			}
-			reconciler.notify(oldObj, newObj, chartRepo, tt.res, tt.resErr)
+			reconciler.notify(ctx, oldObj, newObj, chartRepo, tt.res, tt.resErr)
 
 			select {
 			case x, ok := <-recorder.Events:

Original file line number	Diff line number	Diff line change
`@@ -370,13 +370,13 @@ func (r BucketReconciler) reconcile(ctx context.Context, obj sourcev1.Bucket,`
`370`	`370`	`res = sreconcile.LowestRequeuingResult(res, recResult)`
`371`	`371`	`}`
`372`	`372`
`373`		`- r.notify(oldObj, obj, index, res, resErr)`
	`373`	`+ r.notify(ctx, oldObj, obj, index, res, resErr)`
`374`	`374`
`375`	`375`	`return res, resErr`
`376`	`376`	`}`
`377`	`377`
`378`	`378`	`// notify emits notification related to the reconciliation.`
`379`		`-func (r BucketReconciler) notify(oldObj, newObj sourcev1.Bucket, index *etagIndex, res sreconcile.Result, resErr error) {`
	`379`	`+func (r BucketReconciler) notify(ctx context.Context, oldObj, newObj sourcev1.Bucket, index *etagIndex, res sreconcile.Result, resErr error) {`
`380`	`380`	`// Notify successful reconciliation for new artifact and recovery from any`
`381`	`381`	`// failure.`
`382`	`382`	`if resErr == nil && res == sreconcile.ResultSuccess && newObj.Status.Artifact != nil {`
`@@ -396,10 +396,12 @@ func (r BucketReconciler) notify(oldObj, newObj sourcev1.Bucket, index *etagIn`
`396`	`396`	`if oldChecksum != newObj.GetArtifact().Checksum {`
`397`	`397`	`r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,`
`398`	`398`	`"NewArtifact", message)`
	`399`	`+ ctrl.LoggerFrom(ctx).Info(message)`
`399`	`400`	`} else {`
`400`	`401`	`if sreconcile.FailureRecovery(oldObj, newObj, bucketFailConditions) {`
`401`	`402`	`r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,`
`402`	`403`	`meta.SucceededReason, message)`
	`404`	`+ ctrl.LoggerFrom(ctx).Info(message)`
`403`	`405`	`}`
`404`	`406`	`}`
`405`	`407`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1264,7 +1264,7 @@ func TestBucketReconciler_notify(t *testing.T) {`
`1264`	`1264`	`"bbb": "ddd",`
`1265`	`1265`	`},`
`1266`	`1266`	`}`
`1267`		`- reconciler.notify(oldObj, newObj, index, tt.res, tt.resErr)`
	`1267`	`+ reconciler.notify(ctx, oldObj, newObj, index, tt.res, tt.resErr)`
`1268`	`1268`
`1269`	`1269`	`select {`
`1270`	`1270`	`case x, ok := <-recorder.Events:`
Original file line number	Diff line number	Diff line change
`@@ -286,13 +286,13 @@ func (r GitRepositoryReconciler) reconcile(ctx context.Context, obj sourcev1.G`
`286`	`286`	`res = sreconcile.LowestRequeuingResult(res, recResult)`
`287`	`287`	`}`
`288`	`288`
`289`		`- r.notify(oldObj, obj, commit, res, resErr)`
	`289`	`+ r.notify(ctx, oldObj, obj, commit, res, resErr)`
`290`	`290`
`291`	`291`	`return res, resErr`
`292`	`292`	`}`
`293`	`293`
`294`	`294`	`// notify emits notification related to the result of reconciliation.`
`295`		`-func (r GitRepositoryReconciler) notify(oldObj, newObj sourcev1.GitRepository, commit git.Commit, res sreconcile.Result, resErr error) {`
	`295`	`+func (r GitRepositoryReconciler) notify(ctx context.Context, oldObj, newObj sourcev1.GitRepository, commit git.Commit, res sreconcile.Result, resErr error) {`
`296`	`296`	`// Notify successful reconciliation for new artifact, no-op reconciliation`
`297`	`297`	`// and recovery from any failure.`
`298`	`298`	`if r.shouldNotify(oldObj, newObj, res, resErr) {`
`@@ -319,10 +319,12 @@ func (r GitRepositoryReconciler) notify(oldObj, newObj sourcev1.GitRepository,`
`319`	`319`	`if oldChecksum != newObj.GetArtifact().Checksum {`
`320`	`320`	`r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,`
`321`	`321`	`"NewArtifact", message)`
	`322`	`+ ctrl.LoggerFrom(ctx).Info(message)`
`322`	`323`	`} else {`
`323`	`324`	`if sreconcile.FailureRecovery(oldObj, newObj, gitRepositoryFailConditions) {`
`324`	`325`	`r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,`
`325`	`326`	`meta.SucceededReason, message)`
	`327`	`+ ctrl.LoggerFrom(ctx).Info(message)`
`326`	`328`	`}`
`327`	`329`	`}`
`328`	`330`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1928,7 +1928,7 @@ func TestGitRepositoryReconciler_notify(t *testing.T) {`
`1928`	`1928`	`EventRecorder: recorder,`
`1929`	`1929`	`features: features.FeatureGates(),`
`1930`	`1930`	`}`
`1931`		`- reconciler.notify(oldObj, newObj, tt.commit, tt.res, tt.resErr)`
	`1931`	`+ reconciler.notify(ctx, oldObj, newObj, tt.commit, tt.res, tt.resErr)`
`1932`	`1932`
`1933`	`1933`	`select {`
`1934`	`1934`	`case x, ok := <-recorder.Events:`
Original file line number	Diff line number	Diff line change
`@@ -287,13 +287,13 @@ func (r HelmChartReconciler) reconcile(ctx context.Context, obj sourcev1.HelmC`
`287`	`287`	`res = sreconcile.LowestRequeuingResult(res, recResult)`
`288`	`288`	`}`
`289`	`289`
`290`		`- r.notify(oldObj, obj, &build, res, resErr)`
	`290`	`+ r.notify(ctx, oldObj, obj, &build, res, resErr)`
`291`	`291`
`292`	`292`	`return res, resErr`
`293`	`293`	`}`
`294`	`294`
`295`	`295`	`// notify emits notification related to the reconciliation.`
`296`		`-func (r HelmChartReconciler) notify(oldObj, newObj sourcev1.HelmChart, build *chart.Build, res sreconcile.Result, resErr error) {`
	`296`	`+func (r HelmChartReconciler) notify(ctx context.Context, oldObj, newObj sourcev1.HelmChart, build *chart.Build, res sreconcile.Result, resErr error) {`
`297`	`297`	`// Notify successful reconciliation for new artifact and recovery from any`
`298`	`298`	`// failure.`
`299`	`299`	`if resErr == nil && res == sreconcile.ResultSuccess && newObj.Status.Artifact != nil {`
`@@ -311,10 +311,12 @@ func (r HelmChartReconciler) notify(oldObj, newObj sourcev1.HelmChart, build *`
`311`	`311`	`if oldChecksum != newObj.GetArtifact().Checksum {`
`312`	`312`	`r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,`
`313`	`313`	`reasonForBuild(build), build.Summary())`
	`314`	`+ ctrl.LoggerFrom(ctx).Info(build.Summary())`
`314`	`315`	`} else {`
`315`	`316`	`if sreconcile.FailureRecovery(oldObj, newObj, helmChartFailConditions) {`
`316`	`317`	`r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,`
`317`	`318`	`reasonForBuild(build), build.Summary())`
	`319`	`+ ctrl.LoggerFrom(ctx).Info(build.Summary())`
`318`	`320`	`}`
`319`	`321`	`}`
`320`	`322`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1917,7 +1917,7 @@ func TestHelmChartReconciler_notify(t *testing.T) {`
`1917`	`1917`	`Path: "some/path",`
`1918`	`1918`	`Packaged: true,`
`1919`	`1919`	`}`
`1920`		`- reconciler.notify(oldObj, newObj, build, tt.res, tt.resErr)`
	`1920`	`+ reconciler.notify(ctx, oldObj, newObj, build, tt.res, tt.resErr)`
`1921`	`1921`
`1922`	`1922`	`select {`
`1923`	`1923`	`case x, ok := <-recorder.Events:`
Original file line number	Diff line number	Diff line change
`@@ -250,13 +250,13 @@ func (r HelmRepositoryReconciler) reconcile(ctx context.Context, obj sourcev1.`
`250`	`250`	`res = sreconcile.LowestRequeuingResult(res, recResult)`
`251`	`251`	`}`
`252`	`252`
`253`		`- r.notify(oldObj, obj, chartRepo, res, resErr)`
	`253`	`+ r.notify(ctx, oldObj, obj, chartRepo, res, resErr)`
`254`	`254`
`255`	`255`	`return res, resErr`
`256`	`256`	`}`
`257`	`257`
`258`	`258`	`// notify emits notification related to the reconciliation.`
`259`		`-func (r HelmRepositoryReconciler) notify(oldObj, newObj sourcev1.HelmRepository, chartRepo repository.ChartRepository, res sreconcile.Result, resErr error) {`
	`259`	`+func (r HelmRepositoryReconciler) notify(ctx context.Context, oldObj, newObj sourcev1.HelmRepository, chartRepo repository.ChartRepository, res sreconcile.Result, resErr error) {`
`260`	`260`	`// Notify successful reconciliation for new artifact and recovery from any`
`261`	`261`	`// failure.`
`262`	`262`	`if resErr == nil && res == sreconcile.ResultSuccess && newObj.Status.Artifact != nil {`
`@@ -281,11 +281,13 @@ func (r HelmRepositoryReconciler) notify(oldObj, newObj sourcev1.HelmRepositor`
`281`	`281`	`if oldChecksum != newObj.GetArtifact().Checksum {`
`282`	`282`	`r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,`
`283`	`283`	`"NewArtifact", message)`
	`284`	`+ ctrl.LoggerFrom(ctx).Info(message)`
`284`	`285`	`} else {`
`285`	`286`	`if sreconcile.FailureRecovery(oldObj, newObj, helmRepositoryFailConditions) {`
`286`	`287`	`r.AnnotatedEventf(newObj, annotations, corev1.EventTypeNormal,`
`287`	`288`	`meta.SucceededReason, message)`
`288`	`289`	`}`
	`290`	`+ ctrl.LoggerFrom(ctx).Info(message)`
`289`	`291`	`}`
`290`	`292`	`}`
`291`	`293`	`}`
Original file line number	Diff line number	Diff line change
`@@ -229,11 +229,11 @@ func (r HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj v1beta`
`229`	`229`	`ready := conditions.Get(obj, meta.ReadyCondition)`
`230`	`230`	`// Became ready from not ready.`
`231`	`231`	`if !conditions.IsReady(oldObj) && conditions.IsReady(obj) {`
`232`		`- r.Eventf(obj, corev1.EventTypeNormal, ready.Reason, ready.Message)`
	`232`	`+ r.eventLogf(ctx, obj, corev1.EventTypeNormal, ready.Reason, ready.Message)`
`233`	`233`	`}`
`234`	`234`	`// Became not ready from ready.`
`235`	`235`	`if conditions.IsReady(oldObj) && !conditions.IsReady(obj) {`
`236`		`- r.Eventf(obj, corev1.EventTypeWarning, ready.Reason, ready.Message)`
	`236`	`+ r.eventLogf(ctx, obj, corev1.EventTypeWarning, ready.Reason, ready.Message)`
`237`	`237`	`}`
`238`	`238`	`}()`
`239`	`239`
Original file line number	Diff line number	Diff line change
`@@ -1069,7 +1069,7 @@ func TestHelmRepositoryReconciler_notify(t *testing.T) {`
`1069`	`1069`	`chartRepo := repository.ChartRepository{`
`1070`	`1070`	`URL: "some-address",`
`1071`	`1071`	`}`
`1072`		`- reconciler.notify(oldObj, newObj, chartRepo, tt.res, tt.resErr)`
	`1072`	`+ reconciler.notify(ctx, oldObj, newObj, chartRepo, tt.res, tt.resErr)`
`1073`	`1073`
`1074`	`1074`	`select {`
`1075`	`1075`	`case x, ok := <-recorder.Events:`