Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a predefined error code for build CRD as the prefix of error message #483

Closed
wants to merge 1 commit into from
+189 −32
Closed

Add a predefined error code for build CRD as the prefix of error message #483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions docs/build.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ SPDX-License-Identifier: Apache-2.0
- [Defining the Output](#defining-the-output)
- [Runtime-Image](#Runtime-Image)
- [BuildRun deletion](#BuildRun-deletion)
- [Build error code](#Build-error-code)

## Overview

Expand Down Expand Up @@ -278,3 +279,22 @@ metadata:
annotations:
build.build.dev/build-run-deletion: "true"
```

## Build error code

---
|Unique error code|Error name|Error message|
|---|---|---|
|`1001`|`ListSecretInNamespaceFailed`|`listing secrets in namespace %s failed`|
|`1002`|`NoSecretsInNamespace`|`there are no secrets in namespace %s`|
|`1003`|`SecretsDoNotExist`|`secrets %s do not exist`|
|`1004`|`SecretDoesNotExist`|`secret %s does not exist`|
|`1005`|`UnknownStrategy`|`unknown strategy %v`|
|`1006`|`ListBuildStrategyInNamespaceFailed`|`listing BuildStrategies in ns %s failed`|
|`1007`|`NoneBuildStrategyFoundInNamespace`|`none BuildStrategies found in namespace %s`|
|`1008`|`BuildStrategyDoesNotExistInNamespace`|`buildStrategy %s does not exist in namespace %s`|
|`1009`|`ListClusterBuildStrategyFailed`|`listing ClusterBuildStrategies failed`|
|`1010`|`NoClusterBuildStrategyFound`|`no ClusterBuildStrategies found`|
|`1011`|`ClusterBuildStrategyDoesNotExist`|`clusterBuildStrategy %s does not exist`|
|`1012`|`RuntimePathsCanNotBeEmpty`|`the property 'spec.runtime.paths' must not be empty`|
|`1013`|`SetOwnerReferenceFailed`|`unexpected error when trying to set the ownerreference: %v`|
44 changes: 31 additions & 13 deletions pkg/controller/build/build_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,24 @@ const succeedStatus string = "Succeeded"
const namespace string = "namespace"
const name string = "name"

// error message
const (
InitialBuildErrorCodes = 1000*1 + iota
ListSecretInNamespaceFailed // validate secrets
NoSecretsInNamespace
SecretsDoNotExist
SecretDoesNotExist
UnknownStrategy // validate strategy
ListBuildStrategyInNamespaceFailed
NoneBuildStrategyFoundInNamespace
BuildStrategyDoesNotExistInNamespace
ListClusterBuildStrategyFailed
NoClusterBuildStrategyFound
ClusterBuildStrategyDoesNotExist
RuntimePathsCanNotBeEmpty // validate runtime
SetOwnerReferenceFailed // validate BuildRun OwnerReferences
)

type setOwnerReferenceFunc func(owner, object metav1.Object, scheme *runtime.Scheme) error

// Add creates a new Build Controller and adds it to the Manager. The Manager will set fields on the Controller
Expand Down Expand Up @@ -204,7 +222,7 @@ func (r *ReconcileBuild) Reconcile(request reconcile.Request) (reconcile.Result,

func (r *ReconcileBuild) validateRuntime(runtime *build.Runtime) error {
if len(runtime.Paths) == 0 {
return fmt.Errorf("the property 'spec.runtime.paths' must not be empty")
return fmt.Errorf("%v: the property 'spec.runtime.paths' must not be empty", RuntimePathsCanNotBeEmpty)
}
return nil
}
Expand All @@ -221,7 +239,7 @@ func (r *ReconcileBuild) validateStrategyRef(ctx context.Context, s *build.Strat
return err
}
default:
return fmt.Errorf("unknown strategy %v", *s.Kind)
return fmt.Errorf("%v: unknown strategy %v", UnknownStrategy, *s.Kind)
}
} else {
ctxlog.Info(ctx, "buildStrategy kind is nil, use default NamespacedBuildStrategyKind")
Expand All @@ -236,11 +254,11 @@ func (r *ReconcileBuild) validateBuildStrategy(ctx context.Context, n string, ns
list := &build.BuildStrategyList{}

if err := r.client.List(ctx, list, &client.ListOptions{Namespace: ns}); err != nil {
return errors.Wrapf(err, "listing BuildStrategies in ns %s failed", ns)
return errors.Wrapf(err, "%v: listing BuildStrategies in ns %s failed", ListBuildStrategyInNamespaceFailed, ns)
}

if len(list.Items) == 0 {
return errors.Errorf("none BuildStrategies found in namespace %s", ns)
return errors.Errorf("%v: none BuildStrategies found in namespace %s", NoneBuildStrategyFoundInNamespace, ns)
}

if len(list.Items) > 0 {
Expand All @@ -249,7 +267,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("%v: buildStrategy %s does not exist in namespace %s", BuildStrategyDoesNotExistInNamespace, n, ns)
}
return nil
}
Expand All @@ -258,11 +276,11 @@ func (r *ReconcileBuild) validateClusterBuildStrategy(ctx context.Context, n str
list := &build.ClusterBuildStrategyList{}

if err := r.client.List(ctx, list); err != nil {
return errors.Wrapf(err, "listing ClusterBuildStrategies failed")
return errors.Wrapf(err, "%v: listing ClusterBuildStrategies failed", ListClusterBuildStrategyFailed)
}

if len(list.Items) == 0 {
return errors.Errorf("no ClusterBuildStrategies found")
return errors.Errorf("%v: no ClusterBuildStrategies found", NoClusterBuildStrategyFound)
}

if len(list.Items) > 0 {
Expand All @@ -271,7 +289,7 @@ func (r *ReconcileBuild) validateClusterBuildStrategy(ctx context.Context, n str
return nil
}
}
return fmt.Errorf("clusterBuildStrategy %s does not exist", n)
return fmt.Errorf("%v: clusterBuildStrategy %s does not exist", ClusterBuildStrategyDoesNotExist, n)
}
return nil
}
Expand All @@ -286,11 +304,11 @@ func (r *ReconcileBuild) validateSecrets(ctx context.Context, secretNames []stri
Namespace: ns,
},
); err != nil {
return errors.Wrapf(err, "listing secrets in namespace %s failed", ns)
return errors.Wrapf(err, "%v: listing secrets in namespace %s failed", ListSecretInNamespaceFailed, ns)
}

if len(list.Items) == 0 {
return errors.Errorf("there are no secrets in namespace %s", ns)
return errors.Errorf("%v: there are no secrets in namespace %s", NoSecretsInNamespace, ns)
}

var lookUp = map[string]bool{}
Expand All @@ -308,9 +326,9 @@ func (r *ReconcileBuild) validateSecrets(ctx context.Context, secretNames []stri
}

if len(missingSecrets) > 1 {
return fmt.Errorf("secrets %s do not exist", strings.Join(missingSecrets, ", "))
return fmt.Errorf("%v: secrets %s do not exist", SecretsDoNotExist, strings.Join(missingSecrets, ", "))
} else if len(missingSecrets) > 0 {
return fmt.Errorf("secret %s does not exist", missingSecrets[0])
return fmt.Errorf("%v: secret %s does not exist", SecretDoesNotExist, missingSecrets[0])
}

return nil
Expand All @@ -331,7 +349,7 @@ func (r *ReconcileBuild) validateBuildRunOwnerReferences(ctx context.Context, b
for _, buildRun := range buildRunList.Items {
if index := r.validateBuildOwnerReference(buildRun.OwnerReferences, b); index == -1 {
if err := r.setOwnerReferenceFunc(b, &buildRun, r.scheme); err != nil {
b.Status.Reason = fmt.Sprintf("unexpected error when trying to set the ownerreference: %v", err)
b.Status.Reason = fmt.Sprintf("%v: unexpected error when trying to set the ownerreference: %v", SetOwnerReferenceFailed, err)
if err := r.client.Status().Update(ctx, b); err != nil {
return err
}
Expand Down
105 changes: 87 additions & 18 deletions pkg/controller/build/build_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,13 @@ var _ = Describe("Reconcile Build", func() {
return nil
})

statusCall := ctl.StubFunc(corev1.ConditionFalse, "secret non-existing does not exist")
statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("%v: secret non-existing does not exist", buildController.SecretDoesNotExist))
statusWriter.UpdateCalls(statusCall)

_, err := reconciler.Reconcile(request)
Expect(err).To(HaveOccurred())
Expect(statusWriter.UpdateCallCount()).To(Equal(1))
Expect(err.Error()).To(ContainSubstring("secret non-existing does not exist"))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%v: secret non-existing does not exist", buildController.SecretDoesNotExist)))
})

It("succeeds when the secret exists", func() {
Expand Down Expand Up @@ -162,13 +162,13 @@ var _ = Describe("Reconcile Build", func() {
return nil
})

statusCall := ctl.StubFunc(corev1.ConditionFalse, "secret non-existing does not exist")
statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("%v: secret non-existing does not exist", buildController.SecretDoesNotExist))
statusWriter.UpdateCalls(statusCall)

_, err := reconciler.Reconcile(request)
Expect(err).To(HaveOccurred())
Expect(statusWriter.UpdateCallCount()).To(Equal(1))
Expect(err.Error()).To(ContainSubstring("secret non-existing does not exist"))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%v: secret non-existing does not exist", buildController.SecretDoesNotExist)))
})

It("succeeds when the secret exists", func() {
Expand Down Expand Up @@ -221,13 +221,13 @@ var _ = Describe("Reconcile Build", func() {
return nil
})

statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("secret %s does not exist", registrySecret))
statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("%v: secret %s does not exist", buildController.SecretDoesNotExist, registrySecret))
statusWriter.UpdateCalls(statusCall)

_, err := reconciler.Reconcile(request)
Expect(err).To(HaveOccurred())
Expect(statusWriter.UpdateCallCount()).To(Equal(1))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("secret %s does not exist", registrySecret)))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%v: secret %s does not exist", buildController.SecretDoesNotExist, registrySecret)))
})
It("succeed when the secret exists", func() {

Expand Down Expand Up @@ -266,13 +266,13 @@ var _ = Describe("Reconcile Build", func() {
}
return nil
})
statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("there are no secrets in namespace %s", namespace))
statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("%v: there are no secrets in namespace %s", buildController.NoSecretsInNamespace, 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("there are no secrets in namespace %s", namespace)))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%v: there are no secrets in namespace %s", buildController.NoSecretsInNamespace, namespace)))
})
})

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

statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("clusterBuildStrategy %s does not exist", buildStrategyName))
statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("%v: clusterBuildStrategy %s does not exist", buildController.ClusterBuildStrategyDoesNotExist, buildStrategyName))
statusWriter.UpdateCalls(statusCall)

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

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

statusCall := ctl.StubFunc(corev1.ConditionFalse, "no ClusterBuildStrategies found")
statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("%v: no ClusterBuildStrategies found", buildController.NoClusterBuildStrategyFound))
statusWriter.UpdateCalls(statusCall)

_, err := reconciler.Reconcile(request)
Expect(err).To(HaveOccurred())
Expect(statusWriter.UpdateCallCount()).To(Equal(1))
Expect(err.Error()).To(ContainSubstring("no ClusterBuildStrategies found"))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%v: no ClusterBuildStrategies found", buildController.NoClusterBuildStrategyFound)))
})
})
Context("when spec strategy BuildStrategy is specified", func() {
Expand All @@ -406,13 +406,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("%v: buildStrategy %s does not exist in namespace %s", buildController.BuildStrategyDoesNotExistInNamespace, 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("%v: buildStrategy %s does not exist in namespace %s", buildController.BuildStrategyDoesNotExistInNamespace, buildStrategyName, namespace)))
})
It("succeed when the strategy exists", func() {

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

statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("none BuildStrategies found in namespace %s", namespace))
statusCall := ctl.StubFunc(corev1.ConditionFalse, fmt.Sprintf("%v: none BuildStrategies found in namespace %s", buildController.NoneBuildStrategyFoundInNamespace, 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("none BuildStrategies found in namespace %s", namespace)))
Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("%v: none BuildStrategies found in namespace %s", buildController.NoneBuildStrategyFoundInNamespace, namespace)))
})
})
Context("when spec strategy kind is not specified", func() {
Expand All @@ -485,13 +485,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("%v: buildStrategy %s does not exist in namespace %s", buildController.BuildStrategyDoesNotExistInNamespace, 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("%v: buildStrategy %s does not exist in namespace %s", buildController.BuildStrategyDoesNotExistInNamespace, buildStrategyName, namespace)))

})
It("default to BuildStrategy and succeed if the strategy exists", func() {
Expand All @@ -518,5 +518,74 @@ var _ = Describe("Reconcile Build", func() {
Expect(reconcile.Result{}).To(Equal(result))
})
})

Context("Validate all build error code is correct", func() {
It("1001 error code should represent listing secrets in namespace failed", func() {
errorName, err := ctl.ValidateError(1001)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("ListSecretInNamespaceFailed"))
})
It("1002 error code should represent there are no secrets in namespace", func() {
errorName, err := ctl.ValidateError(1002)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("NoSecretsInNamespace"))
})
It("1003 error code should represent secrets do not exist", func() {
errorName, err := ctl.ValidateError(1003)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("SecretsDoNotExist"))
})
It("1004 error code should represent secret does not exist", func() {
errorName, err := ctl.ValidateError(1004)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("SecretDoesNotExist"))
})
It("1005 error code should represent unknown strategy", func() {
errorName, err := ctl.ValidateError(1005)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("UnknownStrategy"))
})
It("1006 error code should represent listing BuildStrategies in ns failed", func() {
errorName, err := ctl.ValidateError(1006)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("ListBuildStrategyInNamespaceFailed"))
})
It("1007 error code should represent none BuildStrategies found in namespace", func() {
errorName, err := ctl.ValidateError(1007)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("NoneBuildStrategyFoundInNamespace"))
})
It("1008 error code should represent buildStrategy does not exist in namespace", func() {
errorName, err := ctl.ValidateError(1008)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("BuildStrategyDoesNotExistInNamespace"))
})
It("1009 error code should represent listing ClusterBuildStrategies failed", func() {
errorName, err := ctl.ValidateError(1009)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("ListClusterBuildStrategyFailed"))
})
It("1010 error code should represent no ClusterBuildStrategies found", func() {
errorName, err := ctl.ValidateError(1010)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("NoClusterBuildStrategyFound"))
})
It("1011 error code should represent clusterBuildStrategy does not exist", func() {
errorName, err := ctl.ValidateError(1011)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("ClusterBuildStrategyDoesNotExist"))
})
It("1012 error code should represent the property 'spec.runtime.paths' must not be empty", func() {
errorName, err := ctl.ValidateError(1012)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("RuntimePathsCanNotBeEmpty"))
})
It("1013 error code should represent unexpected error when trying to set the ownerreference", func() {
errorName, err := ctl.ValidateError(1013)
Expect(err).ToNot(HaveOccurred())
Expect(errorName).To(Equal("SetOwnerReferenceFailed"))
})
})

})
})
Loading