Merge pull request dana-team#200 from arielsepton/feat/improve-capp-r…

…evision-naming

feat: improve CappRevision naming

internal/kinds/capprevision/adapters/capprevision_adapter.go

@@ -3,16 +3,14 @@ package adapters
 import (
 	"context"
 	"fmt"
-	"math/rand"
-	"strconv"
 	"strings"
-	"time"
 
 	cappv1alpha1 "github.com/dana-team/container-app-operator/api/v1alpha1"
 	"github.com/go-logr/logr"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/selection"
+	"knative.dev/pkg/kmeta"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -24,32 +22,9 @@ var (
 )
 
 const (
-	ClientListLimit    = 100
-	charSet            = "abcdefghijklmnopqrstuvwxyz0123456789"
-	RandomStringLength = 5
-	IndexCut           = 50
+	ClientListLimit = 100
 )
 
-var seededRand = rand.New(rand.NewSource(time.Now().UnixNano()))
-
-// generateRandomString returns a random string of the specified length using characters from the charset.
-func generateRandomString(length int) string {
-	b := make([]byte, length)
-	for i := range b {
-		b[i] = charSet[seededRand.Intn(len(charSet))]
-	}
-	return string(b)
-}
-
-// getSubstringUntilIndex returns a substring from the start of the given string up to (but not including) the character at the specified index.
-// If the index is out of bounds, it returns the entire string.
-func getSubstringUntilIndex(s string, index int) string {
-	if index < 0 || index > len(s) {
-		return s
-	}
-	return s[:index]
-}
-
 // copyAnnotations returns a map of annotations from a Capp that contain the Domain string.
 func copyAnnotations(capp cappv1alpha1.Capp) map[string]string {
 	annotations := make(map[string]string)
@@ -86,8 +61,7 @@ func CreateCappRevision(ctx context.Context, k8sClient client.Client, logger log
 	cappRevision := cappv1alpha1.CappRevision{
 		TypeMeta: metav1.TypeMeta{},
 		ObjectMeta: metav1.ObjectMeta{
-			Name: fmt.Sprintf("%s-%s-v%s", getSubstringUntilIndex(capp.Name, IndexCut),
-				generateRandomString(RandomStringLength), strconv.Itoa(revisionNumber)),
+			Name:        kmeta.ChildName(capp.Name, fmt.Sprintf("-%05d", revisionNumber)),
 			Namespace:   capp.Namespace,
 			Labels:      map[string]string{cappNameLabelKey: capp.Name},
 			Annotations: copyAnnotations(capp),

internal/kinds/capprevision/controllers/controller.go

@@ -57,7 +57,7 @@ func (r *CappRevisionReconciler) Reconcile(ctx context.Context, req reconcile.Re
 		return ctrl.Result{}, nil
 	}
 	if err := syncCappRevision(ctx, r.Client, capp, logger); err != nil {
-		if errors.IsConflict(err) {
+		if errors.IsConflict(err) || errors.IsAlreadyExists(err) {
 			logger.Info(fmt.Sprintf("Conflict detected requeuing: %s", err.Error()))
 			return ctrl.Result{RequeueAfter: RequeueTime}, nil
 		}

test/e2e_tests/capp_revision_e2e_test.go

@@ -6,6 +6,7 @@ import (
 	"strconv"
 
 	"k8s.io/client-go/util/retry"
+	"knative.dev/pkg/kmeta"
 
 	"github.com/dana-team/container-app-operator/test/e2e_tests/mocks"
 	"github.com/dana-team/container-app-operator/test/e2e_tests/testconsts"
@@ -35,6 +36,9 @@ var _ = Describe("Validate CappRevision creation", func() {
 			return len(cappRevisons)
 		}, testconsts.Timeout, testconsts.Interval).ShouldNot(BeZero(), "Should create CappRevisions")
 
+		cappRevisionName := kmeta.ChildName(desiredCapp.Name, fmt.Sprintf("-%05d", 1))
+		utilst.GetCappRevision(k8sClient, cappRevisionName, desiredCapp.Namespace)
+
 		By("Updating Capp")
 		err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 			desiredCapp = utilst.GetCapp(k8sClient, desiredCapp.Name, desiredCapp.Namespace)
@@ -50,6 +54,9 @@ var _ = Describe("Validate CappRevision creation", func() {
 			return len(cappRevisions) > 1
 		}, testconsts.Timeout, testconsts.Interval).Should(BeTrue(), "Should create new CappRevision")
 
+		cappRevisionName = kmeta.ChildName(desiredCapp.Name, fmt.Sprintf("-%05d", 2))
+		utilst.GetCappRevision(k8sClient, cappRevisionName, desiredCapp.Namespace)
+
 		By("Deleting Capp")
 		utilst.DeleteCapp(k8sClient, desiredCapp)
 		Eventually(func() int {
@@ -68,7 +75,7 @@ var _ = Describe("Validate CappRevision creation", func() {
 		desiredCapp.Annotations = make(map[string]string)
 
 		By("Checking many updates to Capp")
-		for i := 0; i <= moreThanRevisionsToKeep; i++ {
+		for i := 1; i < moreThanRevisionsToKeep; i++ {
 			assertValue := fmt.Sprintf("test%s", strconv.Itoa(i))
 			err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 				desiredCapp = utilst.GetCapp(k8sClient, desiredCapp.Name, desiredCapp.Namespace)
@@ -84,14 +91,19 @@ var _ = Describe("Validate CappRevision creation", func() {
 			}, testconsts.Timeout, testconsts.Interval).Should(Equal(assertValue), "Should be equal to the updated value")
 
 			desiredCapp = utilst.GetCapp(k8sClient, desiredCapp.Name, desiredCapp.Namespace)
+			if i < revisionsToKeep {
+				Eventually(func() int {
+					cappRevisons, _ := utilst.GetCappRevisions(context.Background(), k8sClient, *desiredCapp)
+					return len(cappRevisons)
+				}, testconsts.Timeout, testconsts.Interval).Should(Equal(i+1), fmt.Sprintf("Should get %v CappRevisions for %v updates", i+1, i))
+			}
 		}
 
 		Eventually(func() int {
 			cappRevisions, _ := utilst.GetCappRevisions(context.Background(), k8sClient, *desiredCapp)
 			return len(cappRevisions)
 		}, testconsts.Timeout, testconsts.Interval).Should(Equal(revisionsToKeep),
 			fmt.Sprintf("Should limit to %s CappRevision", strconv.Itoa(revisionsToKeep)))
-
 	})
 
 	It(fmt.Sprintf("Should copy annotations containing %s to CappRevision", utilst.Domain), func() {
@@ -110,6 +122,5 @@ var _ = Describe("Validate CappRevision creation", func() {
 			return ""
 		}, testconsts.Timeout, testconsts.Interval).Should(Equal(testAnnotationValue),
 			"Should copy annotations to CappRevision")
-
 	})
 })

test/e2e_tests/utils/capp_revision_adapter.go

@@ -37,3 +37,10 @@ func GetCappRevisions(ctx context.Context, k8sClient client.Client, capp cappv1a
 	err = k8sClient.List(ctx, &cappRevisions, &listOptions)
 	return cappRevisions.Items, err
 }
+
+// GetCappRevision retrieves a CappRevision by name.
+func GetCappRevision(k8sClient client.Client, cappRevisionName, namespace string) *cappv1alpha1.CappRevision {
+	cappRevision := &cappv1alpha1.CappRevision{}
+	GetResource(k8sClient, cappRevision, cappRevisionName, namespace)
+	return cappRevision
+}