ensure controllers have a chance to run at least once (#128)

* ensure controllers have a chance to run at least once

Signed-off-by: Manabu McCloskey <[email protected]>

Author: nabuskey

api/v1alpha1/custom_package_types.go

@@ -23,18 +23,17 @@ type CustomPackageList struct {
 
 // CustomPackageSpec controls the installation of the custom applications.
 type CustomPackageSpec struct {
-	// Replicate specifies whether to replicate remote or local contents to the local gitea server.
-	// +kubebuilder:default:=false
-	Replicate bool `json:"replicate"`
+	ArgoCD ArgoCDPackageSpec `json:"argoCD,omitempty"`
 	// GitServerURL specifies the base URL for the git server for API calls.
 	// for example, https://gitea.cnoe.localtest.me:8443
-	GitServerURL string `json:"gitServerURL"`
+	GitServerURL           string          `json:"gitServerURL"`
+	GitServerAuthSecretRef SecretReference `json:"gitServerAuthSecretRef"`
 	// InternalGitServeURL specifies the base URL for the git server accessible within the cluster.
 	// for example, http://my-gitea-http.gitea.svc.cluster.local:3000
-	InternalGitServeURL    string          `json:"internalGitServeURL"`
-	GitServerAuthSecretRef SecretReference `json:"gitServerAuthSecretRef"`
-
-	ArgoCD ArgoCDPackageSpec `json:"argoCD,omitempty"`
+	InternalGitServeURL string `json:"internalGitServeURL"`
+	// Replicate specifies whether to replicate remote or local contents to the local gitea server.
+	// +kubebuilder:default:=false
+	Replicate bool `json:"replicate"`
 }
 
 type ArgoCDPackageSpec struct {

api/v1alpha1/gitrepository_types.go

@@ -5,7 +5,6 @@ import (
 )
 
 type GitRepositorySpec struct {
-	Source GitRepositorySource `json:"source,omitempty"`
 	// GitURL is the base URL of Git server used for API calls.
 	// +kubebuilder:validation:Required
 	// +kubebuilder:validation:Pattern=`^https?:\/\/.+$`
@@ -14,7 +13,8 @@ type GitRepositorySpec struct {
 	InternalGitURL string `json:"internalGitURL"`
 	// SecretRef is the reference to secret that contain Git server credentials
 	// +kubebuilder:validation:Optional
-	SecretRef SecretReference `json:"secretRef"`
+	SecretRef SecretReference     `json:"secretRef"`
+	Source    GitRepositorySource `json:"source,omitempty"`
 }
 
 type GitRepositorySource struct {
@@ -54,9 +54,8 @@ type GitRepositoryStatus struct {
 	InternalGitRepositoryUrl string `json:"internalGitRepositoryUrl"`
 	// Path is the path within the repository that contains the files.
 	// +kubebuilder:validation:Optional
-	Path string `json:"path"`
-
-	Synced bool `json:"synced"`
+	Path   string `json:"path"`
+	Synced bool   `json:"synced"`
 }
 
 // +kubebuilder:object:root=true

api/v1alpha1/localbuild_types.go

@@ -7,6 +7,14 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+const (
+	// LastObservedCLIStartTimeAnnotation indicates when the controller acted on a resource.
+	LastObservedCLIStartTimeAnnotation = "cnoe.io/last-observed-cli-start-time"
+	// CliStartTimeAnnotation indicates when the CLI was invoked.
+	CliStartTimeAnnotation = "cnoe.io/cli-start-time"
+	FieldManager           = "idpbuilder"
+)
+
 // ArgoPackageConfigSpec Allows for configuration of the ArgoCD Installation.
 // If no fields are specified then the binary embedded resources will be used to intall ArgoCD.
 type ArgoPackageConfigSpec struct {

api/v1alpha1/zz_generated.deepcopy.go

@@ -3,7 +3,6 @@ package build
 import (
 	"context"
 	"fmt"
-
 	"github.com/cnoe-io/idpbuilder/api/v1alpha1"
 	"github.com/cnoe-io/idpbuilder/globals"
 	"github.com/cnoe-io/idpbuilder/pkg/controllers"
@@ -16,6 +15,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
+	"time"
 )
 
 var (
@@ -142,15 +142,20 @@ func (b *Build) Run(ctx context.Context, recreateCluster bool) error {
 		return err
 	}
 
-	// Create localbuild resource
 	localBuild := v1alpha1.Localbuild{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: b.name,
 		},
 	}
 
+	cliStartTime := time.Now().Format(time.RFC3339Nano)
+
 	setupLog.Info("Creating localbuild resource")
 	_, err = controllerutil.CreateOrUpdate(ctx, kubeClient, &localBuild, func() error {
+		if localBuild.ObjectMeta.Annotations == nil {
+			localBuild.ObjectMeta.Annotations = map[string]string{}
+		}
+		localBuild.ObjectMeta.Annotations[v1alpha1.CliStartTimeAnnotation] = cliStartTime
 		localBuild.Spec = v1alpha1.LocalbuildSpec{
 			PackageConfigs: v1alpha1.PackageConfigsSpec{
 				Argo: v1alpha1.ArgoPackageConfigSpec{
@@ -160,7 +165,6 @@ func (b *Build) Run(ctx context.Context, recreateCluster bool) error {
 					Enabled: true,
 				},
 				GitConfig: v1alpha1.GitConfigSpec{
-					// hint: for the old behavior, replace Type value below with globals.GitServerResourcename()
 					Type: globals.GiteaResourceName(),
 				},
 				CustomPackageDirs: b.customPackageDirs,

pkg/build/build.go

@@ -3,6 +3,7 @@ package custompackage
 import (
 	"context"
 	"fmt"
+	"github.com/cnoe-io/idpbuilder/pkg/util"
 	"os"
 	"path/filepath"
 	"strings"
@@ -54,10 +55,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 func (r *Reconciler) postProcessReconcile(ctx context.Context, req ctrl.Request, pkg *v1alpha1.CustomPackage) {
 	logger := log.FromContext(ctx)
+
 	err := r.Status().Update(ctx, pkg)
 	if err != nil {
 		logger.Error(err, "failed updating repo status")
 	}
+
+	err = util.UpdateSyncAnnotation(ctx, r.Client, pkg)
+	if err != nil {
+		logger.Error(err, "failed updating repo annotation")
+	}
 }
 
 // create an in-cluster repository CR, update the application spec, then apply
@@ -173,21 +180,30 @@ func (r *Reconciler) reconcileGitRepo(ctx context.Context, resource *v1alpha1.Cu
 			Name:      repoName,
 			Namespace: resource.Namespace,
 		},
-		Spec: v1alpha1.GitRepositorySpec{
+	}
+
+	cliStartTime, _ := util.GetCLIStartTimeAnnotationValue(resource.ObjectMeta.Annotations)
+
+	_, err := controllerutil.CreateOrUpdate(ctx, r.Client, repo, func() error {
+		if err := controllerutil.SetControllerReference(resource, repo, r.Scheme); err != nil {
+			return err
+		}
+
+		if repo.ObjectMeta.Annotations == nil {
+			repo.ObjectMeta.Annotations = make(map[string]string)
+		}
+		util.SetCLIStartTimeAnnotationValue(repo.ObjectMeta.Annotations, cliStartTime)
+
+		repo.Spec = v1alpha1.GitRepositorySpec{
 			Source: v1alpha1.GitRepositorySource{
 				Type: "local",
 				Path: absPath,
 			},
 			GitURL:         resource.Spec.GitServerURL,
 			InternalGitURL: resource.Spec.InternalGitServeURL,
 			SecretRef:      resource.Spec.GitServerAuthSecretRef,
-		},
-	}
-
-	_, err := controllerutil.CreateOrUpdate(ctx, r.Client, repo, func() error {
-		if err := controllerutil.SetControllerReference(resource, repo, r.Scheme); err != nil {
-			return err
 		}
+
 		return nil
 	})
 	// it's possible for an application to specify the same directory multiple times in the spec.

pkg/controllers/custompackage/controller.go

@@ -97,13 +97,12 @@ func (r *RepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	defer r.postProcessReconcile(ctx, req, &gitRepo)
 	if !r.shouldProcess(gitRepo) {
 		return ctrl.Result{Requeue: false}, nil
 	}
 
 	logger.Info("reconciling GitRepository", "name", req.Name, "namespace", req.Namespace)
-	defer r.postProcessReconcile(ctx, req, &gitRepo)
-
 	result, err := r.reconcileGitRepo(ctx, &gitRepo)
 	if err != nil {
 		r.Recorder.Event(&gitRepo, "Warning", "reconcile error", err.Error())
@@ -116,16 +115,22 @@ func (r *RepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 func (r *RepositoryReconciler) postProcessReconcile(ctx context.Context, req ctrl.Request, repo *v1alpha1.GitRepository) {
 	logger := log.FromContext(ctx)
+
 	err := r.Status().Update(ctx, repo)
 	if err != nil {
 		logger.Error(err, "failed updating repo status")
 	}
+
+	err = util.UpdateSyncAnnotation(ctx, r.Client, repo)
+	if err != nil {
+		logger.Error(err, "failed updating repo annotation")
+	}
 }
 
 func (r *RepositoryReconciler) reconcileGitRepo(ctx context.Context, repo *v1alpha1.GitRepository) (ctrl.Result, error) {
 	logger := log.FromContext(ctx)
 	logger.Info("reconciling", "name", repo.Name, "dir", repo.Spec.Source)
-
+	repo.Status.Synced = false
 	tr := &http.Transport{
 		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
 	}
@@ -147,13 +152,14 @@ func (r *RepositoryReconciler) reconcileGitRepo(ctx context.Context, repo *v1alp
 	if err != nil {
 		return ctrl.Result{Requeue: true, RequeueAfter: requeueTime}, fmt.Errorf("failed to create or update repo %w", err)
 	}
-	repo.Status.ExternalGitRepositoryUrl = giteaRepo.CloneURL
-	repo.Status.InternalGitRepositoryUrl = getRepositoryURL(repo.Namespace, repo.Name, repo.Spec.InternalGitURL)
 
 	err = r.reconcileRepoContent(ctx, repo, giteaRepo)
 	if err != nil {
 		return ctrl.Result{Requeue: true, RequeueAfter: requeueTime}, fmt.Errorf("failed to reconcile repo content %w", err)
 	}
+
+	repo.Status.ExternalGitRepositoryUrl = giteaRepo.CloneURL
+	repo.Status.InternalGitRepositoryUrl = getRepositoryURL(repo.Namespace, repo.Name, repo.Spec.InternalGitURL)
 	repo.Status.Synced = true
 	return ctrl.Result{Requeue: true, RequeueAfter: requeueTime}, nil
 }

pkg/controllers/gitrepository/controller.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"testing"
 	"time"
 
@@ -60,6 +61,7 @@ type testCase struct {
 
 type fakeClient struct {
 	client.Client
+	patchObj client.Object
 }
 
 func (f *fakeClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error {
@@ -71,6 +73,23 @@ func (f *fakeClient) Get(ctx context.Context, key client.ObjectKey, obj client.O
 	return nil
 }
 
+func (f *fakeClient) Status() client.StatusWriter {
+	return fakeStatusWriter{}
+}
+
+func (f *fakeClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
+	f.patchObj = obj
+	return nil
+}
+
+type fakeStatusWriter struct {
+	client.StatusWriter
+}
+
+func (f fakeStatusWriter) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
+	return nil
+}
+
 func setUpLocalRepo() (string, string, error) {
 	repoDir, err := os.MkdirTemp("", fmt.Sprintf("test"))
 	if err != nil {
@@ -377,6 +396,41 @@ func TestGitRepositoryReconcile(t *testing.T) {
 			if !reflect.DeepEqual(v.input.Status, v.expect.resource) {
 				t.Fatalf("objects not equal")
 			}
+
 		})
 	}
 }
+
+func TestGitRepositoryPostReconcile(t *testing.T) {
+	c := fakeClient{}
+	reconciler := RepositoryReconciler{
+		Client: &c,
+	}
+	testTime := time.Now().Format(time.RFC3339Nano)
+	repo := v1alpha1.GitRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "test",
+			Annotations: map[string]string{
+				v1alpha1.CliStartTimeAnnotation: testTime,
+			},
+		},
+	}
+
+	reconciler.postProcessReconcile(context.Background(), ctrl.Request{}, &repo)
+	annotations := c.patchObj.GetAnnotations()
+	v, ok := annotations[v1alpha1.LastObservedCLIStartTimeAnnotation]
+	if !ok {
+		t.Fatalf("expected annotation not found: %s", v1alpha1.LastObservedCLIStartTimeAnnotation)
+	}
+	if v != testTime {
+		t.Fatalf("annotation values does not match")
+	}
+
+	repo.Annotations[v1alpha1.LastObservedCLIStartTimeAnnotation] = "abc"
+	reconciler.postProcessReconcile(context.Background(), ctrl.Request{}, &repo)
+	v = annotations[v1alpha1.LastObservedCLIStartTimeAnnotation]
+	if v != testTime {
+		t.Fatalf("annotation values does not match")
+	}
+}