Resolved verification issues

Signed-off-by: Raghav Bhatnagar <raghavbhatnagar96@gmail.com>

deploy/crds/shipwright.io_buildruns.yaml

@@ -654,13 +654,13 @@ spec:
                         minimum: 1
                         type: integer
                       ttlAfterFailed:
-                        description: TtlAfterFailed defines the maximum duration of
+                        description: TTLAfterFailed defines the maximum duration of
                           time the failed buildrun should exist.
                         format: duration
                         type: string
                       ttlAfterSucceeded:
-                        description: TtlAfterFailed defines the maximum duration of
-                          time the succeeded buildrun should exist.
+                        description: TTLAfterSucceeded defines the maximum duration
+                          of time the succeeded buildrun should exist.
                         format: duration
                         type: string
                     type: object

deploy/crds/shipwright.io_builds.yaml

@@ -343,13 +343,13 @@ spec:
                     minimum: 1
                     type: integer
                   ttlAfterFailed:
-                    description: TtlAfterFailed defines the maximum duration of time
+                    description: TTLAfterFailed defines the maximum duration of time
                       the failed buildrun should exist.
                     format: duration
                     type: string
                   ttlAfterSucceeded:
-                    description: TtlAfterFailed defines the maximum duration of time
-                      the succeeded buildrun should exist.
+                    description: TTLAfterSucceeded defines the maximum duration of
+                      time the succeeded buildrun should exist.
                     format: duration
                     type: string
                 type: object

docs/build.md

@@ -568,9 +568,9 @@ An example of a user using both TTL and Limit retention fields. In case of such
 
 **NOTE**: When changes are made to `retention.failedLimit` and `retention.succeededLimit` values, they come into effect as soon as the build is applied, thereby enforcing the new limits. On the other hand, changing the `retention.ttlAfterFailed` and `retention.ttlAfterSucceeded` values will only affect new buildruns. Old buildruns will adhere to the old TTL retention values.
 
-#### Build_limit_cleanup controller
+#### Build limit cleanup controller
 
-Builds are watched by the build_limit_cleanup controller for the following preconditions:
+Builds are watched by the build limit cleanup controller for the following preconditions:
 
 - Update on `Build` resource if either `retention.failedLimit` or `retention.succeeded` are set
 - Create on `Build` resource if either `retention.failedLimit` or `retention.succeeded` are set

docs/buildrun.md

@@ -171,9 +171,9 @@ We have two controllers that ensure that buildruns can be deleted automatically
   - `build.spec.retention.failedLimit`: This parameter ensures that the number of failed buildruns do not exceed this limit. If the limit is exceeded, the oldest failed buildruns are deleted till the limit is satisfied.
   - `build.spec.retention.succeededLimit`: This parameter ensures that the number of successful buildruns do not exceed this limit. If the limit is exceeded, the oldest successful buildruns are deleted till the limit is satisfied.
 
-### Buildrun_ttl_cleanup controller
+### Buildrun ttl cleanup controller
 
-Buildruns are watched by the buildrun_ttl_cleanup controller for the following preconditions:
+Buildruns are watched by the buildrun ttl cleanup controller for the following preconditions:
 
 - Update on `Buildrun` resource if either `ttlAfterSucceeded` or `ttlAfterFailed` are set.
 - Create on `Buildrun` resource if either `ttlAfterSucceeded` or `ttlAfterFailed` are set.

pkg/apis/build/v1alpha1/build_types.go

@@ -232,16 +232,16 @@ type BuildRetention struct {
 	// +optional
 	// +kubebuilder:validation:Minimum=1
 	SucceededLimit *uint `json:"succeededLimit,omitempty"`
-	// TtlAfterFailed defines the maximum duration of time the failed buildrun should exist.
+	// TTLAfterFailed defines the maximum duration of time the failed buildrun should exist.
 	//
 	// +optional
 	// +kubebuilder:validation:Format=duration
-	TtlAfterFailed *metav1.Duration `json:"ttlAfterFailed,omitempty"`
-	// TtlAfterFailed defines the maximum duration of time the succeeded buildrun should exist.
+	TTLAfterFailed *metav1.Duration `json:"ttlAfterFailed,omitempty"`
+	// TTLAfterSucceeded defines the maximum duration of time the succeeded buildrun should exist.
 	//
 	// +optional
 	// +kubebuilder:validation:Format=duration
-	TtlAfterSucceeded *metav1.Duration `json:"ttlAfterSucceeded,omitempty"`
+	TTLAfterSucceeded *metav1.Duration `json:"ttlAfterSucceeded,omitempty"`
 }
 
 func init() {

pkg/controller/controller.go

@@ -16,9 +16,9 @@ import (
 	"github.com/shipwright-io/build/pkg/config"
 	"github.com/shipwright-io/build/pkg/ctxlog"
 	"github.com/shipwright-io/build/pkg/reconciler/build"
-	"github.com/shipwright-io/build/pkg/reconciler/build_limit_cleanup"
+	"github.com/shipwright-io/build/pkg/reconciler/buildlimitcleanup"
 	"github.com/shipwright-io/build/pkg/reconciler/buildrun"
-	"github.com/shipwright-io/build/pkg/reconciler/buildrun_ttl_cleanup"
+	"github.com/shipwright-io/build/pkg/reconciler/buildrunttlcleanup"
 	"github.com/shipwright-io/build/pkg/reconciler/buildstrategy"
 	"github.com/shipwright-io/build/pkg/reconciler/clusterbuildstrategy"
 )
@@ -65,11 +65,11 @@ func NewManager(ctx context.Context, config *config.Config, cfg *rest.Config, op
 		return nil, err
 	}
 
-	if err := build_limit_cleanup.Add(ctx, config, mgr); err != nil {
+	if err := buildlimitcleanup.Add(ctx, config, mgr); err != nil {
 		return nil, err
 	}
 
-	if err := buildrun_ttl_cleanup.Add(ctx, config, mgr); err != nil {
+	if err := buildrunttlcleanup.Add(ctx, config, mgr); err != nil {
 		return nil, err
 	}
 

pkg/reconciler/build_limit_cleanup/build_limit_cleanup.go → pkg/reconciler/buildlimitcleanup/build_limit_cleanup.go

@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: Apache-2.0
 
-package build_limit_cleanup
+package buildlimitcleanup
 
 import (
 	"context"
@@ -94,7 +94,7 @@ func (r *ReconcileBuild) Reconcile(ctx context.Context, request reconcile.Reques
 				return buildRunSucceeded[i].Status.CompletionTime.Before(buildRunSucceeded[j].Status.CompletionTime)
 			})
 			lenOfList := len(buildRunSucceeded)
-			for i := 0; lenOfList-i > int(*b.Spec.Retention.SucceededLimit); i += 1 {
+			for i := 0; lenOfList-i > int(*b.Spec.Retention.SucceededLimit); i++ {
 				ctxlog.Info(ctx, "Deleting succeeded buildrun as cleanup limit has been reached.", namespace, request.Namespace, name, buildRunSucceeded[i].Name)
 				deleteBuildRunErr := r.client.Delete(ctx, &buildRunSucceeded[i], &client.DeleteOptions{})
 				if deleteBuildRunErr != nil {
@@ -114,7 +114,7 @@ func (r *ReconcileBuild) Reconcile(ctx context.Context, request reconcile.Reques
 				return buildRunFailed[i].Status.CompletionTime.Before(buildRunFailed[j].Status.CompletionTime)
 			})
 			lenOfList := len(buildRunFailed)
-			for i := 0; lenOfList-i > int(*b.Spec.Retention.FailedLimit); i += 1 {
+			for i := 0; lenOfList-i > int(*b.Spec.Retention.FailedLimit); i++ {
 				ctxlog.Info(ctx, "Deleting failed buildrun as cleanup limit has been reached.", namespace, request.Namespace, name, buildRunFailed[i].Name)
 				deleteBuildRunErr := r.client.Delete(ctx, &buildRunFailed[i], &client.DeleteOptions{})
 				if deleteBuildRunErr != nil {

pkg/reconciler/build_limit_cleanup/controller.go → pkg/reconciler/buildlimitcleanup/controller.go

@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: Apache-2.0
 
-package build_limit_cleanup
+package buildlimitcleanup
 
 import (
 	"context"

pkg/reconciler/buildrun_ttl_cleanup/buildrun_ttl_cleanup.go → pkg/reconciler/buildrunttlcleanup/buildrun_ttl_cleanup.go

@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: Apache-2.0
 
-package buildrun_ttl_cleanup
+package buildrunttlcleanup
 
 import (
 	"context"
@@ -77,8 +77,8 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
 	switch condition.Status {
 
 	case corev1.ConditionTrue:
-		if br.Status.BuildSpec.Retention.TtlAfterSucceeded != nil {
-			if br.Status.CompletionTime.Add(br.Status.BuildSpec.Retention.TtlAfterSucceeded.Duration).Before(time.Now()) {
+		if br.Status.BuildSpec.Retention.TTLAfterSucceeded != nil {
+			if br.Status.CompletionTime.Add(br.Status.BuildSpec.Retention.TTLAfterSucceeded.Duration).Before(time.Now()) {
 				ctxlog.Info(ctx, "Deleting successful buildrun as ttl has been reached.", namespace, request.Namespace, name, request.Name)
 				deleteBuildRunErr := r.client.Delete(ctx, br, &client.DeleteOptions{})
 				if deleteBuildRunErr != nil {
@@ -89,14 +89,14 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
 					return reconcile.Result{}, deleteBuildRunErr
 				}
 			} else {
-				timeLeft := br.Status.CompletionTime.Add(br.Status.BuildSpec.Retention.TtlAfterSucceeded.Duration).Sub(time.Now())
+				timeLeft := time.Until(br.Status.CompletionTime.Add(br.Status.BuildSpec.Retention.TTLAfterSucceeded.Duration))
 				return reconcile.Result{Requeue: true, RequeueAfter: timeLeft}, nil
 			}
 		}
 
 	case corev1.ConditionFalse:
-		if br.Status.BuildSpec.Retention.TtlAfterFailed != nil {
-			if br.Status.CompletionTime.Add(br.Status.BuildSpec.Retention.TtlAfterFailed.Duration).Before(time.Now()) {
+		if br.Status.BuildSpec.Retention.TTLAfterFailed != nil {
+			if br.Status.CompletionTime.Add(br.Status.BuildSpec.Retention.TTLAfterFailed.Duration).Before(time.Now()) {
 				ctxlog.Info(ctx, "Deleting failed buildrun as ttl has been reached.", namespace, request.Namespace, name, request.Name)
 				deleteBuildRunErr := r.client.Delete(ctx, br, &client.DeleteOptions{})
 				if deleteBuildRunErr != nil {
@@ -107,7 +107,7 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req
 					return reconcile.Result{}, deleteBuildRunErr
 				}
 			} else {
-				timeLeft := br.Status.CompletionTime.Add(br.Status.BuildSpec.Retention.TtlAfterFailed.Duration).Sub(time.Now())
+				timeLeft := time.Until(br.Status.CompletionTime.Add(br.Status.BuildSpec.Retention.TTLAfterFailed.Duration))
 				return reconcile.Result{Requeue: true, RequeueAfter: timeLeft}, nil
 			}
 		}

pkg/reconciler/buildrun_ttl_cleanup/controller.go → pkg/reconciler/buildrunttlcleanup/controller.go

@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: Apache-2.0
 
-package buildrun_ttl_cleanup
+package buildrunttlcleanup
 
 import (
 	"context"
@@ -59,7 +59,7 @@ func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler, maxCo
 			o := e.Object.(*buildv1alpha1.BuildRun)
 
 			if (o.Status.BuildSpec != nil) && (o.Status.BuildSpec.Retention != nil) &&
-				(o.Status.BuildSpec.Retention.TtlAfterFailed != nil || o.Status.BuildSpec.Retention.SucceededLimit != nil) {
+				(o.Status.BuildSpec.Retention.TTLAfterFailed != nil || o.Status.BuildSpec.Retention.SucceededLimit != nil) {
 				return true
 			}
 			return false
@@ -68,7 +68,7 @@ func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler, maxCo
 			o := e.ObjectNew.(*buildv1alpha1.BuildRun)
 
 			if (o.Status.BuildSpec != nil) && (o.Status.BuildSpec.Retention != nil) &&
-				(o.Status.BuildSpec.Retention.TtlAfterFailed != nil || o.Status.BuildSpec.Retention.SucceededLimit != nil) {
+				(o.Status.BuildSpec.Retention.TTLAfterFailed != nil || o.Status.BuildSpec.Retention.SucceededLimit != nil) {
 				return true
 			}
 			return false

test/integration/buildrun_cleanup_test.go

@@ -74,7 +74,7 @@ var _ = Describe("Integration tests for retention limits and ttls for succeeded
 			br, err := tb.GetBRTillCompletion(buildRunObject.Name)
 			Expect(err).To(BeNil())
 			Expect(br.Status.GetCondition(v1alpha1.Succeeded).Status).To(Equal(corev1.ConditionTrue))
-			br, err = tb.GetBRTillNotFound(buildRunObject.Name, time.Second*1, time.Minute)
+			_, err = tb.GetBRTillNotFound(buildRunObject.Name, time.Second*1, time.Minute)
 			Expect(apierrors.IsNotFound(err)).To(BeTrue())
 		})
 	})
@@ -176,6 +176,7 @@ var _ = Describe("Integration tests for retention limits and ttls for succeeded
 			buildRunObject.Name = BUILDRUN + tb.Namespace + "-fail-2"
 			buildRunObject.Spec.BuildRef.Name = BUILD + tb.Namespace + "-fail"
 			err = tb.CreateBR(buildRunObject)
+			Expect(err).To(BeNil())
 			br4, err := tb.GetBRTillCompletion(buildRunObject.Name)
 			Expect(err).To(BeNil())
 			Expect(br4.Status.GetCondition(v1alpha1.Succeeded).Status).To(Equal(corev1.ConditionFalse))
@@ -251,7 +252,7 @@ var _ = Describe("Integration tests for retention limits and ttls of buildRuns t
 			br, err := tb.GetBRTillCompletion(buildRunObject.Name)
 			Expect(err).To(BeNil())
 			Expect(br.Status.GetCondition(v1alpha1.Succeeded).Status).To(Equal(corev1.ConditionFalse))
-			br, err = tb.GetBRTillNotFound(buildRunObject.Name, time.Second*1, time.Minute)
+			_, err = tb.GetBRTillNotFound(buildRunObject.Name, time.Second*1, time.Minute)
 			Expect(apierrors.IsNotFound(err)).To(BeTrue())
 		})
 	})