Skip to content

Conversation

@kyrylyuk-andriy
Copy link
Contributor

@kyrylyuk-andriy kyrylyuk-andriy commented Oct 24, 2025

Summary by CodeRabbit

  • Chores
    • Refactored continuous integration pipeline with simplified execution flow and improved error handling
    • Introduced centralized configuration management for pipeline operations
    • Added production deployment configurations with advanced resource synchronization policies
    • Enhanced build infrastructure reliability and workspace management procedures

@coderabbitai
Copy link

coderabbitai bot commented Oct 24, 2025

📝 Walkthrough

Walkthrough

This pull request refactors the primary Jenkinsfile to use a podTemplate-based execution model with shared library integration and centralized error handling, while introducing a backup Jenkinsfile with Kubernetes-based Kaniko builds. Additionally, two new ArgoCD Application manifests are added for dependency-api and dependency-www with comprehensive ignoreDifferences rules and production-grade sync policies.

Changes

Cohort / File(s) Summary
Jenkins Pipeline Refactor
Jenkinsfile
Replaces declarative pipeline blocks with podTemplate-based node execution; integrates @Library('lib-jenkins-pipeline') shared library; introduces top-level script constants (newTagEveryRunMainBranch, sbtOnMain, sbtCommand); adds try/catch error handling with explicit FAILURE marking; simplifies stages to checkout, tagging, component JSON construction, and mainJenkinsBuildArgo invocation; ensures workspace cleanup in finally block.
Backup Kubernetes Pipeline
Jenkinsfile-backup
New multi-stage pipeline configuration using Kubernetes Kaniko environment; includes checkout with semantic versioning via flowSemver; tagging stage for main branch; parallel Kaniko image builds for dependency-api and dependency-www (amd64/arm64); manifest tool orchestration; Helm-based deploy stage on main branch; SBT test stage with database readiness checks, linting, coverage, and post-report handling; sets ORG environment variable to flowcommerce.
ArgoCD Deployment Manifests
deploy/dependency-api/app.yaml, deploy/dependency-www/app.yaml
New ArgoCD Application resources targeting production namespace; define ignoreDifferences rules for VirtualService, DatadogMonitor, Deployment, Rollout, Service, and DestinationRule resources; specify Helm chart sources from flow-generic and git sources for dependency repositories; configure syncPolicy with RespectIgnoreDifferences, CreateNamespace, ApplyOutOfSyncOnly, ServerSideApply, and PruneLast options.

Sequence Diagrams

sequenceDiagram
    participant Dev as Developer
    participant Git as Git Repository
    participant Jenkins as Jenkins
    participant K8s as Kubernetes (podTemplate)
    participant Lib as Shared Library
    participant BuildArgo as Build/Argo System

    Dev->>Git: Push code
    Git->>Jenkins: Webhook trigger
    Jenkins->>K8s: Create podTemplate node
    
    K8s->>Lib: Load @Library('lib-jenkins-pipeline')
    K8s->>Jenkins: Checkout code
    
    rect rgba(200, 220, 255, 0.2)
        Note over K8s,Jenkins: Main Flow (try block)
        K8s->>K8s: Stage: Checkout
        K8s->>K8s: Stage: Tagging (invoke taggingv2)
        K8s->>K8s: Construct JSON payload for components
        K8s->>BuildArgo: mainJenkinsBuildArgo(credentials, parameters)
        BuildArgo-->>K8s: Build result
    end
    
    rect rgba(255, 200, 200, 0.2)
        Note over K8s,Jenkins: Error Handling
        K8s->>K8s: catch: Mark build FAILURE
        K8s->>K8s: finally: Cleanup workspace
    end
    
    K8s-->>Jenkins: Pipeline complete
Loading
sequenceDiagram
    participant Git as Git Push
    participant Jenkins as Backup Jenkins
    participant K8s as Kubernetes (Kaniko)
    participant Registry as Image Registry
    participant Helm as Helm/ArgoCD

    Git->>Jenkins: Webhook trigger
    Jenkins->>K8s: Create Kaniko pod
    
    rect rgba(200, 220, 255, 0.2)
        Note over Jenkins,K8s: Checkout & Version
        K8s->>K8s: flowSemver (compute version)
    end
    
    rect rgba(220, 255, 200, 0.2)
        Note over Jenkins,K8s: Build Stage (parallel)
        par amd64 builds
            K8s->>Registry: Build dependency-api (amd64)
            K8s->>Registry: Build dependency-www (amd64)
        and arm64 builds
            K8s->>Registry: Build dependency-api (arm64)
            K8s->>Registry: Build dependency-www (arm64)
        end
    end
    
    rect rgba(255, 250, 200, 0.2)
        Note over Jenkins,K8s: Post-Build & Deploy
        K8s->>K8s: Manifest tool (arch-specific)
        alt main branch
            K8s->>Helm: Deploy via Helm (dependency-api, dependency-www)
        end
    end
    
    rect rgba(200, 255, 220, 0.2)
        Note over Jenkins,K8s: Test Stage
        K8s->>K8s: Database wait
        K8s->>K8s: SBT: lint, coverage, tests
        K8s->>K8s: Post-report handling
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

The changes span multiple systems (Jenkins pipeline logic, Kubernetes pod orchestration, ArgoCD configuration) with significant heterogeneity. The Jenkinsfile refactor involves architectural shifts from declarative to imperative pod-based execution with new error handling patterns. The backup pipeline introduces parallel build orchestration and multi-architecture image management. The ArgoCD manifests, while structured declarations, require careful review of ignoreDifferences strategies and sync policies. The mix of logic-dense pipeline changes and configuration-heavy manifests demands separate reasoning for each cohort.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "FDN-4119 dependency argocd migration" is clear, specific, and directly related to the main changes in the changeset. The PR's primary focus is the introduction of ArgoCD Application manifests (deploy/dependency-api/app.yaml and deploy/dependency-www/app.yaml) along with supporting Jenkinsfile refactoring to enable ArgoCD-based deployments for the dependency services. The title accurately captures this core objective and is concise enough that a teammate reviewing the commit history would immediately understand the change's purpose.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (3)
deploy/dependency-api/app.yaml (3)

13-17: Scope VirtualService ignores to rollout-only fields (same as dependency-www).

Use jqPathExpressions for weights/subsets rather than hiding /spec/http/0.


45-53: Pinning and image tag parameter need pipeline confirmation.

Ensure CI updates deployments.live.version per release or adjust values flow.


60-69: Add sync retries and prune propagation for robustness.

Same retry/prune suggestions as in dependency-www.

🧹 Nitpick comments (8)
deploy/dependency-www/app.yaml (3)

13-17: VirtualService ignore scope is broad; consider narrowing to rollout-only fields.

Ignoring the entire first http block can hide real drift. Prefer targeting weights/subsets changed by rollouts.

Example (replace jsonPointers with jq paths):

-  - /spec/http/0
+jqPathExpressions:
+  - .spec.http[].route[].weight
+  - .spec.http[].route[].destination.subset

45-53: Chart/version pinning and image version parameter.

  • targetRevision v2.0.0 is pinned: good for reproducibility.

  • deployments.live.version="0.10.33" is static; ensure your pipeline updates image tags at deploy time or you’ll drift.

  • Does mainJenkinsBuildArgo override this value per release?

  • If not, consider removing the parameter here and relying on values.yaml + CI patches, or wire this to a placeholder that CI sets.


60-69: Add sync retries and prune propagation for resilience.

Transient API errors are common; add a retry block and explicit prune propagation.

Apply:

   syncPolicy:
     syncOptions:
       - RespectIgnoreDifferences=true
       - CreateNamespace=false
       - ApplyOutOfSyncOnly=true
       - ServerSideApply=true
       - PruneLast=true
+    retry:
+      limit: 5
+      backoff:
+        duration: 5s
+        factor: 2
+        maxDuration: 3m
+    managedNamespaceMetadata:
+      annotations: {}
+      labels: {}

(Optional) If you rely on background object deletion:

     syncOptions:
+      - PrunePropagationPolicy=background
Jenkinsfile (2)

24-27: Remove redundant checkouts.

You run checkoutWithTags before and after Checkout; keep it inside the Checkout stage only.

-        checkoutWithTags scm
 ...
         stage('Checkout') {
             echo "Checking out branch: ${env.BRANCH_NAME}"
-            checkout scm
+            checkoutWithTags scm
         }
 ...
-        checkoutWithTags scm

Also applies to: 22-22, 36-36


31-33: Type semversion to avoid binding surprises.

Scripted Pipeline globals can leak; declare it explicitly.

-            semversion = taggingv2(newTagEveryMainRun: "${newTagEveryRunMainBranch}")
+            String semversion = taggingv2(newTagEveryMainRun: "${newTagEveryRunMainBranch}")
Jenkinsfile-backup (3)

4-8: Enable fail‑fast for parallel stages.

Stop other branches when one fails to save time.

   options {
     disableConcurrentBuilds()
     buildDiscarder(logRotator(numToKeepStr: '10'))
     timeout(time: 30, unit: 'MINUTES')
+    parallelsAlwaysFailFast()
   }

62-64: Fix typos in stage names.

Minor polish.

-        stage('Build dependency-api and depencdency-www services') {
+        stage('Build dependency-api and dependency-www services') {
...
-        stage('Deploy dependency servcies') {
+        stage('Deploy dependency services') {

Also applies to: 188-189


216-230: DB wait loop: good; consider explicit host/port and shorter poll.

Network namespace is shared in a Pod, but be explicit to reduce flakes.

-                    until pg_isready -h localhost
+                    until pg_isready -h 127.0.0.1 -p 5432
                     do
-                      sleep 10
+                      sleep 3
                     done
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23bf43f and 4b000d1.

📒 Files selected for processing (4)
  • Jenkinsfile (1 hunks)
  • Jenkinsfile-backup (1 hunks)
  • deploy/dependency-api/app.yaml (1 hunks)
  • deploy/dependency-www/app.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Jenkins
🔇 Additional comments (7)
deploy/dependency-www/app.yaml (3)

23-31: Replicas ignore rules look correct for KEDA/HPA.

Good use of replicas ignores on Deployment and Rollout to avoid controller fights.

Confirm KEDA/HPA is the only scaler; if a custom scaler also mutates other fields (e.g., annotations), consider adding targeted ignores for those, too.


32-42: Service clusterIP and DestinationRule hash ignores: LGTM.

These are common noisy diffs with Rollouts/Istio; rules are precise and safe.


9-11: Project/namespace wiring check.

Destination namespace is production and project is production. Verify the AppProject allows that namespace and cluster.

If AppProject constraints differ, this Application will be OutOfSync/Forbidden until updated.

Also applies to: 58-59

deploy/dependency-api/app.yaml (1)

23-31: Replicas ignores for Deployment/Rollout are appropriate.

Confirm only scaler-managed fields are ignored.

Jenkinsfile (2)

38-45: Dockerfile paths: absolute vs repo-relative.

Leading slashes imply absolute FS paths inside the agent. Most library build steps expect repo-relative paths (e.g., api/Dockerfile).

If required to be relative, apply:

-        "dockerFilePath" : "/api/Dockerfile"},
+        "dockerFilePath" : "api/Dockerfile"},
 ...
-        "dockerFilePath" : "/www/Dockerfile"}]
+        "dockerFilePath" : "www/Dockerfile"}]

47-53: Credential handling LGTM; consider passing token via withEnv only within the call.

Current scope is fine; ensure mainJenkinsBuildArgo only reads ARGOCD_AUTH_TOKEN and does not log it.

Confirm the shared library masks this env in logs.

Jenkinsfile-backup (1)

45-59: Verify helm container availability.

You call container('helm') but only inheritFrom 'kaniko-slim' and add 'postgres'/'play' templates.

Confirm 'kaniko-slim' includes a 'helm' container image; otherwise add a helm containerTemplate here.

Also applies to: 196-207

@flow-tech
Copy link
Contributor

@kyrylyuk-andriy kyrylyuk-andriy merged commit 1feb62e into main Oct 27, 2025
7 checks passed
@kyrylyuk-andriy kyrylyuk-andriy deleted the argocd-migration branch October 27, 2025 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants