Skip to content

Commit

Permalink
Merge pull request #445 from zhangtbj/refine_error_message
Browse files Browse the repository at this point in the history
Change error messages start with a lower case letter
  • Loading branch information
openshift-merge-robot authored Oct 21, 2020
2 parents 5d391cd + 2359fd8 commit 50419d7
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 16 deletions.
8 changes: 4 additions & 4 deletions pkg/controller/build/build_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler) error
if oldAnnot[build.AnnotationBuildRunDeletion] != newAnnot[build.AnnotationBuildRunDeletion] {
ctxlog.Debug(
ctx,
"Updating predicated passed, the annotation was modified.",
"updating predicated passed, the annotation was modified.",
namespace,
e.MetaNew.GetNamespace(),
name,
Expand Down Expand Up @@ -172,7 +172,7 @@ func (r *ReconcileBuild) Reconcile(request reconcile.Request) (reconcile.Result,
updateErr := r.client.Status().Update(ctx, b)
return reconcile.Result{}, fmt.Errorf("errors: %v %v", err, updateErr)
}
ctxlog.Info(ctx, "build strategy found", namespace, b.Namespace, name, b.Name, "strategy", b.Spec.StrategyRef.Name)
ctxlog.Info(ctx, "buildStrategy found", namespace, b.Namespace, name, b.Name, "strategy", b.Spec.StrategyRef.Name)
}

// validate if "spec.runtime" attributes are valid
Expand Down Expand Up @@ -220,7 +220,7 @@ func (r *ReconcileBuild) validateStrategyRef(ctx context.Context, s *build.Strat
return fmt.Errorf("unknown strategy %v", *s.Kind)
}
} else {
ctxlog.Info(ctx, "BuildStrategy kind is nil, use default NamespacedBuildStrategyKind")
ctxlog.Info(ctx, "buildStrategy kind is nil, use default NamespacedBuildStrategyKind")
if err := r.validateBuildStrategy(ctx, s.Name, ns); err != nil {
return err
}
Expand All @@ -245,7 +245,7 @@ func (r *ReconcileBuild) validateBuildStrategy(ctx context.Context, n string, ns
return nil
}
}
return fmt.Errorf("BuildStrategy %s does not exist in namespace %s", n, ns)
return fmt.Errorf("buildStrategy %s does not exist in namespace %s", n, ns)
}
return nil
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/build/build_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,13 +404,13 @@ var _ = Describe("Reconcile Build", func() {
return nil
})

statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("BuildStrategy %s does not exist in namespace %s", buildStrategyName, namespace))
statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("buildStrategy %s does not exist in namespace %s", buildStrategyName, namespace))
statusWriter.UpdateCalls(statusCall)

_, err := reconciler.Reconcile(request)
Expect(err).To(HaveOccurred())
Expect(statusWriter.UpdateCallCount()).To(Equal(1))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("BuildStrategy %s does not exist in namespace %s", buildStrategyName, namespace)))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("buildStrategy %s does not exist in namespace %s", buildStrategyName, namespace)))
})
It("succeed when the strategy exists", func() {

Expand Down Expand Up @@ -483,13 +483,13 @@ var _ = Describe("Reconcile Build", func() {
return nil
})

statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("BuildStrategy %s does not exist in namespace %s", buildStrategyName, namespace))
statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("buildStrategy %s does not exist in namespace %s", buildStrategyName, namespace))
statusWriter.UpdateCalls(statusCall)

_, err := reconciler.Reconcile(request)
Expect(err).To(HaveOccurred())
Expect(statusWriter.UpdateCallCount()).To(Equal(1))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("BuildStrategy %s does not exist in namespace %s", buildStrategyName, namespace)))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("buildStrategy %s does not exist in namespace %s", buildStrategyName, namespace)))

})
It("default to BuildStrategy and succeed if the strategy exists", func() {
Expand Down
12 changes: 6 additions & 6 deletions pkg/controller/buildrun/buildrun_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,11 @@ func handleError(message string, listOfErrors ...error) error {
// ValidateBuildRegistration verifies that a referenced Build is properly registered
func (r *ReconcileBuildRun) ValidateBuildRegistration(ctx context.Context, build *buildv1alpha1.Build, buildRun *buildv1alpha1.BuildRun) error {
if build.Status.Registered == "" {
err := fmt.Errorf("The Build is not yet validated, build: %s", build.Name)
err := fmt.Errorf("the Build is not yet validated, build: %s", build.Name)
return err
}
if build.Status.Registered != corev1.ConditionTrue {
err := fmt.Errorf("The Build is not registered correctly, build: %s, registered status: %s, reason: %s", build.Name, build.Status.Registered, build.Status.Reason)
err := fmt.Errorf("the Build is not registered correctly, build: %s, registered status: %s, reason: %s", build.Name, build.Status.Registered, build.Status.Reason)
updateErr := r.updateBuildRunErrorStatus(ctx, buildRun, err.Error())
return handleError("Build is not ready", err, updateErr)
}
Expand Down Expand Up @@ -324,7 +324,7 @@ func (r *ReconcileBuildRun) Reconcile(request reconcile.Request) (reconcile.Resu
return reconcile.Result{}, err
}
} else {
ctxlog.Info(ctx, "TaskRun already exists", namespace, request.Namespace, name, request.Name)
ctxlog.Info(ctx, "taskRun already exists", namespace, request.Namespace, name, request.Name)

err = r.GetBuildRunObject(ctx, lastTaskRun.Labels[buildv1alpha1.LabelBuildRun], request.Namespace, buildRun)
if err != nil && !apierrors.IsNotFound(err) {
Expand All @@ -338,7 +338,7 @@ func (r *ReconcileBuildRun) Reconcile(request reconcile.Request) (reconcile.Resu
// finishes which would be missed otherwise. But, if the TaskRun was already completed and the status
// synchronized into the BuildRun, then yet another reconciliation is not necessary.
if buildRun.Status.CompletionTime != nil {
ctxlog.Info(ctx, "BuildRun already marked completed", namespace, request.Namespace, name, request.Name)
ctxlog.Info(ctx, "buildRun already marked completed", namespace, request.Namespace, name, request.Name)
return reconcile.Result{}, nil
}

Expand Down Expand Up @@ -461,7 +461,7 @@ func (r *ReconcileBuildRun) retrieveServiceAccount(ctx context.Context, build *b
if err != nil {
return nil, err
}
ctxlog.Debug(ctx, "Automatic generation of service account", namespace, serviceAccount.Namespace, name, serviceAccount.Name, "Operation", op)
ctxlog.Debug(ctx, "automatic generation of service account", namespace, serviceAccount.Namespace, name, serviceAccount.Name, "Operation", op)
} else {
// If ServiceAccount or the name of ServiceAccount in buildRun is nil, use pipeline serviceaccount
if buildRun.Spec.ServiceAccount == nil || buildRun.Spec.ServiceAccount.Name == nil {
Expand Down Expand Up @@ -560,7 +560,7 @@ func (r *ReconcileBuildRun) createTaskRun(ctx context.Context, build *buildv1alp
// Set OwnerReference for BuildRun and TaskRun
if err := r.setOwnerReferenceFunc(buildRun, generatedTaskRun, r.scheme); err != nil {
updateErr := r.updateBuildRunErrorStatus(ctx, buildRun, err.Error())
return nil, handleError("Failed to set OwnerReference for BuildRun and TaskRun", err, updateErr)
return nil, handleError("failed to set OwnerReference for BuildRun and TaskRun", err, updateErr)
}

return generatedTaskRun, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/buildrun/buildrun_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ var _ = Describe("Reconcile BuildRun", func() {

_, err := reconciler.Reconcile(buildRunRequest)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal(fmt.Sprintf("The Build is not yet validated, build: %s", buildName)))
Expect(err.Error()).To(Equal(fmt.Sprintf("the Build is not yet validated, build: %s", buildName)))
Expect(client.StatusCallCount()).To(Equal(0))
})

Expand Down
2 changes: 1 addition & 1 deletion test/integration/build_to_buildruns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ var _ = Describe("Integration tests Build and BuildRuns", func() {
br, err := tb.GetBRTillCompletion(buildRunObject.Name)
Expect(err).To(BeNil())

Expect(br.Status.Reason).To(Equal(fmt.Sprintf("The Build is not registered correctly, build: %s, registered status: False, reason: secret fake-secret does not exist", BUILD+tb.Namespace)))
Expect(br.Status.Reason).To(Equal(fmt.Sprintf("the Build is not registered correctly, build: %s, registered status: False, reason: secret fake-secret does not exist", BUILD+tb.Namespace)))
})
})

Expand Down

0 comments on commit 50419d7

Please sign in to comment.