diff --git a/api/v1alpha1/localbuild_types.go b/api/v1alpha1/localbuild_types.go index 5d734ad32..6e1ce71bc 100644 --- a/api/v1alpha1/localbuild_types.go +++ b/api/v1alpha1/localbuild_types.go @@ -12,7 +12,11 @@ const ( LastObservedCLIStartTimeAnnotation = "cnoe.io/last-observed-cli-start-time" // CliStartTimeAnnotation indicates when the CLI was invoked. CliStartTimeAnnotation = "cnoe.io/cli-start-time" - FieldManager = "idpbuilder" + // PackagePriorityAnnotation indicates the priority of a package (higher = wins conflicts). + PackagePriorityAnnotation = "cnoe.io/package-priority" + // PackageSourcePathAnnotation indicates the source path of a package. + PackageSourcePathAnnotation = "cnoe.io/package-source-path" + FieldManager = "idpbuilder" // If GetSecretLabelKey is set to GetSecretLabelValue on a kubernetes secret, secret key and values can be used by the get command. CLISecretLabelKey = "cnoe.io/cli-secret" CLISecretLabelValue = "true" diff --git a/pkg/controllers/custompackage/controller.go b/pkg/controllers/custompackage/controller.go index 72d1bffb2..51f206403 100644 --- a/pkg/controllers/custompackage/controller.go +++ b/pkg/controllers/custompackage/controller.go @@ -72,8 +72,173 @@ func (r *Reconciler) postProcessReconcile(ctx context.Context, req ctrl.Request, } } +// shouldTakeOverGitRepository checks if this CustomPackage should take over an existing GitRepository. +// Returns true if this package has higher priority than the current owner. +func (r *Reconciler) shouldTakeOverGitRepository(ctx context.Context, resource *v1alpha1.CustomPackage, existingRepo *v1alpha1.GitRepository) (bool, error) { + logger := log.FromContext(ctx) + + // Get this package's priority + thisPriority, err := getPackagePriority(resource) + if err != nil { + // If no priority annotation, assume it's a legacy package - allow takeover for backward compat + logger.V(1).Info("no priority on this package, allowing takeover", "package", resource.Name) + return true, nil + } + + // Check the existing repo's owner references + for _, ownerRef := range existingRepo.GetOwnerReferences() { + if ownerRef.Kind == "CustomPackage" { + // Fetch the owner CustomPackage to get its priority + ownerPkg := &v1alpha1.CustomPackage{} + err := r.Client.Get(ctx, client.ObjectKey{ + Name: ownerRef.Name, + Namespace: existingRepo.Namespace, + }, ownerPkg) + + if err != nil { + if errors.IsNotFound(err) { + // Owner doesn't exist anymore, we can take over + logger.Info("GitRepository owner not found, taking over", "repo", existingRepo.Name) + return true, nil + } + return false, fmt.Errorf("getting owner package: %w", err) + } + + // Get owner's priority + ownerPriority, err := getPackagePriority(ownerPkg) + if err != nil { + // Owner has no priority, we can take over + logger.V(1).Info("GitRepository owner has no priority, taking over", "repo", existingRepo.Name) + return true, nil + } + + // Only take over if we have HIGHER priority (higher number wins) + if thisPriority > ownerPriority { + logger.Info("Taking over GitRepository from lower priority owner", + "repo", existingRepo.Name, + "ourPriority", thisPriority, + "ownerPriority", ownerPriority, + "owner", ownerPkg.Name) + return true, nil + } else { + logger.Info("Not taking over GitRepository owned by higher/equal priority package", + "repo", existingRepo.Name, + "ourPriority", thisPriority, + "ownerPriority", ownerPriority, + "owner", ownerPkg.Name) + return false, nil + } + } + } + + // No CustomPackage owner found, we can take over + logger.V(1).Info("GitRepository has no CustomPackage owner, taking over", "repo", existingRepo.Name) + return true, nil +} + +// shouldReconcile checks if this CustomPackage should proceed with reconciliation. +// It returns true only if this is the highest priority CustomPackage for the same app. +func (r *Reconciler) shouldReconcile(ctx context.Context, resource *v1alpha1.CustomPackage) (bool, error) { + logger := log.FromContext(ctx) + + // Get this package's priority + thisPriority, err := getPackagePriority(resource) + if err != nil { + // If no priority annotation, assume it's a legacy package and allow it + logger.V(1).Info("no priority annotation found, assuming legacy package", "name", resource.Name) + return true, nil + } + + // List all CustomPackages in the same namespace + pkgList := &v1alpha1.CustomPackageList{} + err = r.Client.List(ctx, pkgList, client.InNamespace(resource.Namespace)) + if err != nil { + return false, fmt.Errorf("listing custom packages: %w", err) + } + + // Check if any other CustomPackage has the same app name with higher priority + for i := range pkgList.Items { + pkg := &pkgList.Items[i] + + // Skip self + if pkg.Name == resource.Name { + continue + } + + // Skip if different app name + if pkg.Spec.ArgoCD.Name != resource.Spec.ArgoCD.Name { + continue + } + + // Get the other package's priority + otherPriority, err := getPackagePriority(pkg) + if err != nil { + // If the other package has no priority, this one wins + continue + } + + // If another package has higher priority, this one should not reconcile + if otherPriority > thisPriority { + thisSource := resource.ObjectMeta.Annotations[v1alpha1.PackageSourcePathAnnotation] + otherSource := pkg.ObjectMeta.Annotations[v1alpha1.PackageSourcePathAnnotation] + logger.Info("Yielding to higher priority package - skipping all reconciliation", + "appName", resource.Spec.ArgoCD.Name, + "yieldingPackage", thisSource, + "yieldingPriority", thisPriority, + "activePackage", otherSource, + "activePriority", otherPriority) + return false, nil + } + } + + return true, nil +} + +// getPackagePriority extracts the priority from a CustomPackage's annotations +func getPackagePriority(pkg *v1alpha1.CustomPackage) (int, error) { + if pkg.ObjectMeta.Annotations == nil { + return 0, fmt.Errorf("no annotations") + } + + priorityStr, ok := pkg.ObjectMeta.Annotations[v1alpha1.PackagePriorityAnnotation] + if !ok { + return 0, fmt.Errorf("no priority annotation") + } + + var priority int + _, err := fmt.Sscanf(priorityStr, "%d", &priority) + if err != nil { + return 0, fmt.Errorf("invalid priority format: %w", err) + } + + return priority, nil +} + // create an in-cluster repository CR, update the application spec, then apply func (r *Reconciler) reconcileCustomPackage(ctx context.Context, resource *v1alpha1.CustomPackage) (ctrl.Result, error) { + logger := log.FromContext(ctx) + + // Check if this package should reconcile + shouldReconcile, err := r.shouldReconcile(ctx, resource) + if err != nil { + return ctrl.Result{}, fmt.Errorf("checking if should reconcile: %w", err) + } + + if !shouldReconcile { + // This package has been superseded by a higher priority package + // Mark as not synced and don't update resources, and don't requeue + logger.Info("package superseded by higher priority, skipping reconciliation", + "name", resource.Name, + "appName", resource.Spec.ArgoCD.Name, + "sourcePath", resource.ObjectMeta.Annotations[v1alpha1.PackageSourcePathAnnotation]) + resource.Status.Synced = false + return ctrl.Result{}, nil + } + + logger.V(1).Info("proceeding with reconciliation as highest priority package", + "name", resource.Name, + "appName", resource.Spec.ArgoCD.Name) + b, err := r.getArgoCDAppFile(ctx, resource) if err != nil { return ctrl.Result{}, fmt.Errorf("reading file %s: %w", resource.Spec.ArgoCD.ApplicationFile, err) @@ -219,7 +384,12 @@ func (r *Reconciler) reconcileArgoCDApp(ctx context.Context, resource *v1alpha1. } resource.Status.GitRepositoryRefs = repoRefs resource.Status.Synced = appSourcesSynced - return ctrl.Result{RequeueAfter: requeueTime}, nil + + // Only requeue if not synced yet to avoid continuous reconciliation + if !appSourcesSynced { + return ctrl.Result{RequeueAfter: requeueTime}, nil + } + return ctrl.Result{}, nil } func (r *Reconciler) reconcileArgoCDAppSet(ctx context.Context, resource *v1alpha1.CustomPackage, appSet *argov1alpha1.ApplicationSet) (ctrl.Result, error) { @@ -270,7 +440,11 @@ func (r *Reconciler) reconcileArgoCDAppSet(ctx context.Context, resource *v1alph resource.Status.Synced = resource.Status.Synced && gitGeneratorsSynced - return ctrl.Result{RequeueAfter: requeueTime}, nil + // Only requeue if not synced yet to avoid continuous reconciliation + if !resource.Status.Synced { + return ctrl.Result{RequeueAfter: requeueTime}, nil + } + return ctrl.Result{}, nil } // create a gitrepository custom resource, then let the git repository controller take care of the rest @@ -285,6 +459,7 @@ func (r *Reconciler) reconcileArgoCDSource(ctx context.Context, resource *v1alph } func (r *Reconciler) reconcileArgoCDSourceFromRemote(ctx context.Context, resource *v1alpha1.CustomPackage, appName, repoURL string) (ctrl.Result, *v1alpha1.GitRepository, error) { + logger := log.FromContext(ctx) relativePath := strings.TrimPrefix(repoURL, v1alpha1.CNOEURIScheme) // no guarantee that this path exists dirPath := filepath.Join(resource.Spec.RemoteRepository.Path, relativePath) @@ -296,9 +471,34 @@ func (r *Reconciler) reconcileArgoCDSourceFromRemote(ctx context.Context, resour }, } + // Check if GitRepository already exists + existingRepo := &v1alpha1.GitRepository{} + getErr := r.Client.Get(ctx, client.ObjectKeyFromObject(repo), existingRepo) + + if getErr == nil { + // GitRepository exists - check if we should take it over + shouldTakeOver, checkErr := r.shouldTakeOverGitRepository(ctx, resource, existingRepo) + if checkErr != nil { + return ctrl.Result{}, nil, fmt.Errorf("checking if should take over git repository: %w", checkErr) + } + + if !shouldTakeOver { + // A higher/equal priority package owns this, just return it without updating + logger.V(1).Info("Using existing GitRepository owned by higher priority package", + "repo", repo.Name, + "ourPriority", resource.ObjectMeta.Annotations[v1alpha1.PackagePriorityAnnotation]) + return ctrl.Result{}, existingRepo, nil + } + // We should take it over - proceed with CreateOrUpdate below + } else if !errors.IsNotFound(getErr) { + return ctrl.Result{}, nil, getErr + } + // GitRepository doesn't exist or we should take it over + cliStartTime, _ := util.GetCLIStartTimeAnnotationValue(resource.ObjectMeta.Annotations) - _, err := controllerutil.CreateOrUpdate(ctx, r.Client, repo, func() error { + var err error + _, err = controllerutil.CreateOrUpdate(ctx, r.Client, repo, func() error { if err := controllerutil.SetControllerReference(resource, repo, r.Scheme); err != nil { return err } @@ -349,6 +549,30 @@ func (r *Reconciler) reconcileArgoCDSourceFromLocal(ctx context.Context, resourc }, } + // Check if GitRepository already exists + existingRepo := &v1alpha1.GitRepository{} + getErr := r.Client.Get(ctx, client.ObjectKeyFromObject(repo), existingRepo) + + if getErr == nil { + // GitRepository exists - check if we should take it over + shouldTakeOver, checkErr := r.shouldTakeOverGitRepository(ctx, resource, existingRepo) + if checkErr != nil { + return ctrl.Result{}, nil, fmt.Errorf("checking if should take over git repository: %w", checkErr) + } + + if !shouldTakeOver { + // A higher/equal priority package owns this, just return it without updating + logger.V(1).Info("Using existing GitRepository owned by higher priority package", + "repo", repo.Name, + "ourPriority", resource.ObjectMeta.Annotations[v1alpha1.PackagePriorityAnnotation]) + return ctrl.Result{}, existingRepo, nil + } + // We should take it over - proceed with CreateOrUpdate below + } else if !errors.IsNotFound(getErr) { + return ctrl.Result{}, nil, getErr + } + // GitRepository doesn't exist or we should take it over + cliStartTime, _ := util.GetCLIStartTimeAnnotationValue(resource.ObjectMeta.Annotations) _, err = controllerutil.CreateOrUpdate(ctx, r.Client, repo, func() error { @@ -469,7 +693,8 @@ func (r *Reconciler) reconcileHelmValueObjectSource(ctx context.Context, val[k] = v } } - return ctrl.Result{RequeueAfter: requeueTime}, nil + // No need to requeue for helm value processing + return ctrl.Result{}, nil } func localRepoName(appName, dir string) string { diff --git a/pkg/controllers/custompackage/controller_test.go b/pkg/controllers/custompackage/controller_test.go index 1ff725828..9762c2041 100644 --- a/pkg/controllers/custompackage/controller_test.go +++ b/pkg/controllers/custompackage/controller_test.go @@ -709,3 +709,220 @@ func TestReconcileHelmValueObject(t *testing.T) { expectJson := `{"arrayMap":[{"nested":{"test":""},"test":""}],"arrayString":["abc",""],"bool":false,"int":456,"nested":{"bool":true,"int":123,"repoURLGit":""},"repoURLGit":""}` assert.JSONEq(t, expectJson, string(source.Helm.ValuesObject.Raw)) } + +func TestPackagePriority(t *testing.T) { + s := k8sruntime.NewScheme() + sb := k8sruntime.NewSchemeBuilder( + v1.AddToScheme, + argov1alpha1.AddToScheme, + v1alpha1.AddToScheme, + ) + sb.AddToScheme(s) + testEnv := &envtest.Environment{ + CRDDirectoryPaths: []string{ + filepath.Join("..", "resources"), + "../localbuild/resources/argo/install.yaml", + }, + ErrorIfCRDPathMissing: true, + Scheme: s, + BinaryAssetsDirectory: filepath.Join("..", "..", "..", "bin", "k8s", + fmt.Sprintf("1.29.1-%s-%s", runtime.GOOS, runtime.GOARCH)), + } + + cfg, err := testEnv.Start() + require.NoError(t, err) + defer testEnv.Stop() + + mgr, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: s, + }) + require.NoError(t, err) + + ctx, ctxCancel := context.WithCancel(context.Background()) + stoppedCh := make(chan error) + go func() { + err := mgr.Start(ctx) + stoppedCh <- err + }() + + defer func() { + ctxCancel() + err := <-stoppedCh + if err != nil { + t.Errorf("Starting controller manager: %v", err) + t.FailNow() + } + }() + + r := &Reconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("test-custompkg-controller"), + } + cwd, err := os.Getwd() + require.NoError(t, err) + + // Create namespaces + for _, n := range []string{"argocd", "test"} { + ns := v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: n, + }, + } + err = mgr.GetClient().Create(context.Background(), &ns) + if err != nil { + t.Fatalf("creating test ns: %v", err) + } + } + + // Create two CustomPackages with the same app name but different priorities + // Package 1 has priority 0 (from first --package argument) + pkg1 := v1alpha1.CustomPackage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pkg1-my-app", + Namespace: "test", + UID: "pkg1", + Annotations: map[string]string{ + v1alpha1.PackagePriorityAnnotation: "0", + v1alpha1.PackageSourcePathAnnotation: "/path/to/package1", + }, + }, + Spec: v1alpha1.CustomPackageSpec{ + Replicate: true, + GitServerURL: "https://cnoe.io", + InternalGitServeURL: "http://internal.cnoe.io", + ArgoCD: v1alpha1.ArgoCDPackageSpec{ + ApplicationFile: filepath.Join(cwd, "test/resources/customPackages/testDir/app.yaml"), + Name: "my-app", + Namespace: "argocd", + Type: "Application", + }, + }, + } + + // Package 2 has priority 1 (from second --package argument, should win) + pkg2 := v1alpha1.CustomPackage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pkg2-my-app", + Namespace: "test", + UID: "pkg2", + Annotations: map[string]string{ + v1alpha1.PackagePriorityAnnotation: "1", + v1alpha1.PackageSourcePathAnnotation: "/path/to/package2", + }, + }, + Spec: v1alpha1.CustomPackageSpec{ + Replicate: true, + GitServerURL: "https://cnoe.io", + InternalGitServeURL: "http://internal.cnoe.io", + ArgoCD: v1alpha1.ArgoCDPackageSpec{ + ApplicationFile: filepath.Join(cwd, "test/resources/customPackages/testDir/app.yaml"), + Name: "my-app", + Namespace: "argocd", + Type: "Application", + }, + }, + } + + // Create the CustomPackages in the cluster + err = mgr.GetClient().Create(context.Background(), &pkg1) + require.NoError(t, err) + err = mgr.GetClient().Create(context.Background(), &pkg2) + require.NoError(t, err) + + // Test priority resolution + t.Run("lower priority package should not reconcile", func(t *testing.T) { + shouldReconcile, err := r.shouldReconcile(context.Background(), &pkg1) + assert.NoError(t, err) + assert.False(t, shouldReconcile, "pkg1 (priority 0) should not reconcile when pkg2 (priority 1) exists") + }) + + t.Run("higher priority package should reconcile", func(t *testing.T) { + shouldReconcile, err := r.shouldReconcile(context.Background(), &pkg2) + assert.NoError(t, err) + assert.True(t, shouldReconcile, "pkg2 (priority 1) should reconcile as it has highest priority") + }) + + t.Run("getPackagePriority should extract priority correctly", func(t *testing.T) { + priority1, err := getPackagePriority(&pkg1) + assert.NoError(t, err) + assert.Equal(t, 0, priority1) + + priority2, err := getPackagePriority(&pkg2) + assert.NoError(t, err) + assert.Equal(t, 1, priority2) + }) +} + +func TestGetPackagePriority(t *testing.T) { + t.Run("valid priority annotation", func(t *testing.T) { + pkg := &v1alpha1.CustomPackage{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + v1alpha1.PackagePriorityAnnotation: "5", + }, + }, + } + priority, err := getPackagePriority(pkg) + assert.NoError(t, err) + assert.Equal(t, 5, priority) + }) + + t.Run("missing annotations", func(t *testing.T) { + pkg := &v1alpha1.CustomPackage{ + ObjectMeta: metav1.ObjectMeta{}, + } + _, err := getPackagePriority(pkg) + assert.Error(t, err) + }) + + t.Run("missing priority annotation", func(t *testing.T) { + pkg := &v1alpha1.CustomPackage{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "other": "value", + }, + }, + } + _, err := getPackagePriority(pkg) + assert.Error(t, err) + }) + + t.Run("invalid priority format", func(t *testing.T) { + pkg := &v1alpha1.CustomPackage{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + v1alpha1.PackagePriorityAnnotation: "invalid", + }, + }, + } + _, err := getPackagePriority(pkg) + assert.Error(t, err) + }) + + t.Run("zero priority", func(t *testing.T) { + pkg := &v1alpha1.CustomPackage{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + v1alpha1.PackagePriorityAnnotation: "0", + }, + }, + } + priority, err := getPackagePriority(pkg) + assert.NoError(t, err) + assert.Equal(t, 0, priority) + }) + + t.Run("large priority value", func(t *testing.T) { + pkg := &v1alpha1.CustomPackage{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + v1alpha1.PackagePriorityAnnotation: "1000", + }, + }, + } + priority, err := getPackagePriority(pkg) + assert.NoError(t, err) + assert.Equal(t, 1000, priority) + }) +} diff --git a/pkg/controllers/gitrepository/controller.go b/pkg/controllers/gitrepository/controller.go index 93baedefc..b2b9cda63 100644 --- a/pkg/controllers/gitrepository/controller.go +++ b/pkg/controllers/gitrepository/controller.go @@ -176,6 +176,9 @@ func (r *RepositoryReconciler) reconcileGitRepo(ctx context.Context, repo *v1alp repo.Status.ExternalGitRepositoryUrl = providerRepo.cloneUrl repo.Status.InternalGitRepositoryUrl = providerRepo.internalGitRepositoryUrl repo.Status.Synced = true + + // Keep requeueing to detect source file changes, but addAllAndCommit + // already checks if there are changes before pushing return ctrl.Result{Requeue: true, RequeueAfter: requeueTime}, nil } diff --git a/pkg/controllers/localbuild/controller.go b/pkg/controllers/localbuild/controller.go index d36532aaf..ce27bf2b5 100644 --- a/pkg/controllers/localbuild/controller.go +++ b/pkg/controllers/localbuild/controller.go @@ -248,22 +248,27 @@ func (r *LocalbuildReconciler) ReconcileArgoAppsWithGitea(ctx context.Context, r } } - for _, s := range resource.Spec.PackageConfigs.CustomPackageDirs { - result, err := r.reconcileCustomPkgDir(ctx, resource, s) + // Process packages in REVERSE order (highest priority first) to avoid creating + // lower priority packages first then having to delete them + for i := len(resource.Spec.PackageConfigs.CustomPackageDirs) - 1; i >= 0; i-- { + s := resource.Spec.PackageConfigs.CustomPackageDirs[i] + result, err := r.reconcileCustomPkgDir(ctx, resource, s, i) if err != nil { return result, err } } - for _, s := range resource.Spec.PackageConfigs.CustomPackageFiles { - result, err := r.reconcileCustomPkgFile(ctx, resource, s) + for i := len(resource.Spec.PackageConfigs.CustomPackageFiles) - 1; i >= 0; i-- { + s := resource.Spec.PackageConfigs.CustomPackageFiles[i] + result, err := r.reconcileCustomPkgFile(ctx, resource, s, i) if err != nil { return result, err } } - for _, s := range resource.Spec.PackageConfigs.CustomPackageUrls { - result, err := r.reconcileCustomPkgUrl(ctx, resource, s) + for i := len(resource.Spec.PackageConfigs.CustomPackageUrls) - 1; i >= 0; i-- { + s := resource.Spec.PackageConfigs.CustomPackageUrls[i] + result, err := r.reconcileCustomPkgUrl(ctx, resource, s, i) if err != nil { return result, err } @@ -436,6 +441,8 @@ func (r *LocalbuildReconciler) reconcileCustomPkg( b []byte, filePath string, remote *util.KustomizeRemote, + priority int, + sourcePath string, ) error { o := &unstructured.Unstructured{} _, gvk, fErr := scheme.Codecs.UniversalDeserializer().Decode(b, nil, o) @@ -447,10 +454,55 @@ func (r *LocalbuildReconciler) reconcileCustomPkg( kind := o.GetKind() appName := o.GetName() appNS := o.GetNamespace() + + // Check if a higher-priority CustomPackage already exists for this app + projectNS := globals.GetProjectNamespace(resource.Name) + existingPkgs := &v1alpha1.CustomPackageList{} + if err := r.Client.List(ctx, existingPkgs, client.InNamespace(projectNS)); err != nil { + return fmt.Errorf("listing existing custom packages: %w", err) + } + + for i := range existingPkgs.Items { + existingPkg := &existingPkgs.Items[i] + // Check if this package is for the same ArgoCD app + if existingPkg.Spec.ArgoCD.Name == appName { + // Get existing package's priority + existingPriorityStr, exists := existingPkg.ObjectMeta.Annotations[v1alpha1.PackagePriorityAnnotation] + if exists { + var existingPriority int + if _, err := fmt.Sscanf(existingPriorityStr, "%d", &existingPriority); err == nil { + if existingPriority > priority { + // A higher priority package already exists, skip this one + existingSourcePath := existingPkg.ObjectMeta.Annotations[v1alpha1.PackageSourcePathAnnotation] + log.FromContext(ctx).Info("Skipping CustomPackage creation - higher priority package already exists", + "appName", appName, + "skippingPackage", sourcePath, + "skippingPriority", priority, + "keepingPackage", existingSourcePath, + "keepingPriority", existingPriority) + return nil + } else if existingPriority < priority { + // We have higher priority, delete the existing lower-priority package + existingSourcePath := existingPkg.ObjectMeta.Annotations[v1alpha1.PackageSourcePathAnnotation] + log.FromContext(ctx).Info("Deleting lower priority CustomPackage", + "appName", appName, + "deletingPackage", existingSourcePath, + "deletingPriority", existingPriority, + "usingPackage", sourcePath, + "usingPriority", priority) + if err := r.Client.Delete(ctx, existingPkg); err != nil && !k8serrors.IsNotFound(err) { + return fmt.Errorf("deleting lower priority package: %w", err) + } + } + } + } + } + } + customPkg := &v1alpha1.CustomPackage{ ObjectMeta: metav1.ObjectMeta{ Name: getCustomPackageName(filepath.Base(filePath), appName), - Namespace: globals.GetProjectNamespace(resource.Name), + Namespace: projectNS, }, } @@ -465,6 +517,8 @@ func (r *LocalbuildReconciler) reconcileCustomPkg( } util.SetCLIStartTimeAnnotationValue(customPkg.ObjectMeta.Annotations, cliStartTime) + customPkg.ObjectMeta.Annotations[v1alpha1.PackagePriorityAnnotation] = fmt.Sprintf("%d", priority) + customPkg.ObjectMeta.Annotations[v1alpha1.PackageSourcePathAnnotation] = sourcePath customPkg.Spec = v1alpha1.CustomPackageSpec{ Replicate: true, @@ -498,7 +552,7 @@ func (r *LocalbuildReconciler) reconcileCustomPkg( return nil } -func (r *LocalbuildReconciler) reconcileCustomPkgUrl(ctx context.Context, resource *v1alpha1.Localbuild, pkgUrl string) (ctrl.Result, error) { +func (r *LocalbuildReconciler) reconcileCustomPkgUrl(ctx context.Context, resource *v1alpha1.Localbuild, pkgUrl string, priority int) (ctrl.Result, error) { logger := log.FromContext(ctx) remote, err := util.NewKustomizeRemote(pkgUrl) @@ -533,7 +587,7 @@ func (r *LocalbuildReconciler) reconcileCustomPkgUrl(ctx context.Context, resour continue } - rErr := r.reconcileCustomPkg(ctx, resource, b, yamlFile, remote) + rErr := r.reconcileCustomPkg(ctx, resource, b, yamlFile, remote, priority, pkgUrl) if rErr != nil { logger.Error(rErr, "reconciling custom pkg", "file", yamlFile, "pkgUrl", pkgUrl) } @@ -541,7 +595,7 @@ func (r *LocalbuildReconciler) reconcileCustomPkgUrl(ctx context.Context, resour return ctrl.Result{}, nil } -func (r *LocalbuildReconciler) reconcileCustomPkgDir(ctx context.Context, resource *v1alpha1.Localbuild, pkgDir string) (ctrl.Result, error) { +func (r *LocalbuildReconciler) reconcileCustomPkgDir(ctx context.Context, resource *v1alpha1.Localbuild, pkgDir string, priority int) (ctrl.Result, error) { logger := log.FromContext(ctx) files, err := os.ReadDir(pkgDir) @@ -562,7 +616,7 @@ func (r *LocalbuildReconciler) reconcileCustomPkgDir(ctx context.Context, resour continue } - rErr := r.reconcileCustomPkg(ctx, resource, b, filePath, nil) + rErr := r.reconcileCustomPkg(ctx, resource, b, filePath, nil, priority, pkgDir) if rErr != nil { logger.Error(rErr, "reconciling custom pkg", "file", filePath, "pkgDir", pkgDir) } @@ -571,7 +625,7 @@ func (r *LocalbuildReconciler) reconcileCustomPkgDir(ctx context.Context, resour return ctrl.Result{}, nil } -func (r *LocalbuildReconciler) reconcileCustomPkgFile(ctx context.Context, resource *v1alpha1.Localbuild, pkgFile string) (ctrl.Result, error) { +func (r *LocalbuildReconciler) reconcileCustomPkgFile(ctx context.Context, resource *v1alpha1.Localbuild, pkgFile string, priority int) (ctrl.Result, error) { logger := log.FromContext(ctx) file, err := os.Open(pkgFile) @@ -598,7 +652,7 @@ func (r *LocalbuildReconciler) reconcileCustomPkgFile(ctx context.Context, resou return ctrl.Result{}, fmt.Errorf("reading file, %s: %w", pkgFile, err) } - rErr := r.reconcileCustomPkg(ctx, resource, b, pkgFile, nil) + rErr := r.reconcileCustomPkg(ctx, resource, b, pkgFile, nil, priority, pkgFile) if rErr != nil { logger.Error(rErr, "reconciling custom pkg", "file", pkgFile) } diff --git a/tests/e2e/docker/docker_test.go b/tests/e2e/docker/docker_test.go index 91e185231..3a643fd94 100644 --- a/tests/e2e/docker/docker_test.go +++ b/tests/e2e/docker/docker_test.go @@ -48,6 +48,7 @@ func Test_CreateDocker(t *testing.T) { testCreatePath(t) testCreatePort(t) testCustomPkg(t) + testPackagePriority(t) } // test idpbuilder create @@ -162,3 +163,65 @@ func testCustomPkg(t *testing.T) { } assert.Empty(t, expectedRepoNames) } + +// testPackagePriority tests the priority-based package reconciliation feature +// where multiple packages for the same app can be specified, and only the highest priority wins +func testPackagePriority(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 8*time.Minute) + defer cancel() + defer CleanUpDocker(t) + + // Create with multiple package directories + // The packages will be assigned priorities based on their order (0, 1, 2, ...) + cmdString := "create --package ../../../pkg/controllers/custompackage/test/resources/customPackages/testDir --package ../../../pkg/controllers/custompackage/test/resources/customPackages/testDir2" + + t.Log(fmt.Sprintf("running %s to test package priority", cmdString)) + cmd := exec.CommandContext(ctx, e2e.IdpbuilderBinaryLocation, strings.Split(cmdString, " ")...) + b, err := cmd.CombinedOutput() + assert.NoError(t, err, fmt.Sprintf("error while running create: %s, %s", err, b)) + + kubeClient, err := e2e.GetKubeClient() + assert.NoError(t, err, fmt.Sprintf("error while getting client: %s", err)) + if err != nil { + assert.FailNow(t, "failed creating cluster") + } + + // Wait for core packages to be ready + e2e.TestArgoCDApps(ctx, t, kubeClient, e2e.CorePackages) + + // Verify CustomPackages have priority annotations + t.Log("Verifying CustomPackages have correct priority annotations") + + customPkgList := &e2e.CustomPackageList{} + err = kubeClient.List(ctx, customPkgList, &e2e.ListOptions{Namespace: "idpbuilder-localdev"}) + assert.NoError(t, err, "failed to list custom packages") + + // Verify that packages have priority annotations + foundPriorities := make(map[string]string) + for _, pkg := range customPkgList.Items { + if pkg.ObjectMeta.Annotations != nil { + if priority, exists := pkg.ObjectMeta.Annotations["cnoe.io/package-priority"]; exists { + t.Logf("Package %s has priority: %s", pkg.Name, priority) + foundPriorities[pkg.Name] = priority + } + if sourcePath, exists := pkg.ObjectMeta.Annotations["cnoe.io/package-source-path"]; exists { + t.Logf("Package %s has source path: %s", pkg.Name, sourcePath) + } + } + } + + // At least some packages should have priority annotations + assert.NotEmpty(t, foundPriorities, "expected custom packages to have priority annotations") + + // Wait for custom packages to reconcile + time.Sleep(10 * time.Second) + + // Verify apps are deployed + expectedApps := map[string]string{ + "my-app": "argocd", + "guestbook": "argocd", + } + e2e.TestArgoCDApps(ctx, t, kubeClient, expectedApps) + + t.Log("Package priority test completed successfully") +} diff --git a/tests/e2e/e2e.go b/tests/e2e/e2e.go index 195cd19e6..97725252b 100644 --- a/tests/e2e/e2e.go +++ b/tests/e2e/e2e.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/sdk/gitea" argov1alpha1 "github.com/cnoe-io/argocd-api/api/argo/application/v1alpha1" + "github.com/cnoe-io/idpbuilder/api/v1alpha1" "github.com/cnoe-io/idpbuilder/pkg/k8s" "github.com/cnoe-io/idpbuilder/pkg/printer/types" corev1 "k8s.io/api/core/v1" @@ -31,6 +32,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +// Type aliases for convenience +type CustomPackageList = v1alpha1.CustomPackageList +type ListOptions = client.ListOptions + const ( IdpbuilderBinaryLocation = "../../../idpbuilder" DefaultPort = "8443"