-
Notifications
You must be signed in to change notification settings - Fork 19
FDN-4119 dependency argocd migration #849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis 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
Sequence DiagramssequenceDiagram
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
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
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)
✅ Passed checks (2 passed)
Comment |
There was a problem hiding this 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=backgroundJenkinsfile (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 scmAlso 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
📒 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
|
Click here for code coverage report: https://jenkins.flo.pub/job/flowcommerce/job/dependency/job/PR-849/2/scoverage-report/ |
Summary by CodeRabbit