(onboarding) Remove staging repository-validator apps from AppSRE ArgoCD' - #13784
(onboarding) Remove staging repository-validator apps from AppSRE ArgoCD'#13784celeztyne wants to merge 4 commits into
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: celeztyne The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Kustomize Render DiffComparing
Total: 19 components, +1170 -177 lines 📋 Full diff available in the workflow summary and as a downloadable artifact. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13784 +/- ##
=======================================
Coverage 60.38% 60.38%
=======================================
Files 24 24
Lines 1628 1628
=======================================
Hits 983 983
Misses 563 563
Partials 82 82
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
7853ccc to
91a4d13
Compare
PR Summary by QodoRetire legacy repository-validator staging ApplicationSet
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Misplaced YAML document separator
|
91a4d13 to
3aed377
Compare
|
Code review by qodo was updated up to the latest commit 3aed377 |
🤖 Pipeline Failure AnalysisCategory: Build The pipeline failed because an automatic merge of PR #13784 encountered a content conflict in 📋 Technical DetailsImmediate CauseThe direct cause of the failure was a content merge conflict detected in the Contributing FactorsNo additional contributing factors were identified from the provided failed steps analysis or additional context. The conflict appears to be a direct issue with the changes introduced by PR #13784 conflicting with the target branch's ImpactThe merge conflict prevented the successful completion of the 🔍 Evidenceappstudio-e2e-tests/konflux-ci-install-konfluxCategory: Logs:
|
enkeefe00
left a comment
There was a problem hiding this comment.
You'll need to add and ApplicationSet folder to the rd-dev/ overlay for this component if you're going to delete the dev app here.
3aed377 to
b277454
Compare
|
Code review by qodo was updated up to the latest commit b277454 |
|
Code review by qodo was updated up to the latest commit a72515d |
| # automated: | ||
| # prune: true | ||
| # selfHeal: true |
|
Code review by qodo was updated up to the latest commit c982c3b |
c982c3b to
1bd7fd7
Compare
| $patch: delete | ||
| --- | ||
| apiVersion: argoproj.io/v1alpha1 | ||
| --- |
There was a problem hiding this comment.
1. Misplaced yaml document separator 🐞 Bug ≡ Correctness
The added separator now follows notification-controller's apiVersion, leaving the preceding document with duplicate apiVersion keys and the notification-controller document without one. Kustomize cannot reliably parse this patch file, blocking the konflux-public-staging overlay build.
Agent Prompt
## Issue description
The YAML document separator was moved after the next resource's `apiVersion`, producing malformed strategic-merge patch documents.
## Issue Context
The `etcd-defrag` document receives a duplicate `apiVersion`, while the `notification-controller` document begins without one.
## Fix Focus Areas
- argo-cd-apps/overlays/konflux-public-staging/delete-applications.yaml[38-44]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| name: repository-validator | ||
| $patch: delete |
There was a problem hiding this comment.
2. Deletion target is absent 🐞 Bug ≡ Correctness
The new repository-validator strategic-merge deletion targets an ApplicationSet that is not present anywhere in the konflux-public-staging resource graph. After correcting the YAML separator, Kustomize will fail because it cannot find a unique resource matching this delete patch.
Agent Prompt
## Issue description
The new strategic-merge delete patch targets `ApplicationSet/repository-validator`, but that resource is not included by the konflux-public-staging overlay.
## Issue Context
Unlike staging-downstream, konflux-public-staging does not include `../../base/repository-validator`; unmatched strategic-merge patches make Kustomize builds fail.
## Fix Focus Areas
- argo-cd-apps/overlays/konflux-public-staging/delete-applications.yaml[44-50]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 1bd7fd7 |
Summary of Changes
As part of the Migration/reorganization of the infra-deployments repo components to comply with new component standards, applying step 3 of the Migration SOP that consist in deleting the old staging of the component repository-validator and ApplicationSet from konflux-public-staging and staging-downstream.
Tracked in KFLUXINFRA-4289
Argo-cd RD link from parts 1 and 2 fo the SOP:
https://argocd-infra-deployments-server-argocd-infra-deployments.apps.rosa.kflux-c-stg-i01.qfla.p3.openshiftapps.com/applications?proj=&sync=&autoSync=&health=&namespace=&cluster=&labels=&annotations=&operation=&search=repository-validator
Verification
oc kustomize has been performed on the following directories to render config files and the results:
metadata:
labels:
appstudio.redhat.com/deployment-clusters: tenant
appstudio.redhat.com/internal-only: "true"
name: repository-validator
namespace: argocd-infra-deployments
spec:
...
sourceRoot: components/repository-validator-rd
WRT Staging this components is only deployed on internal clusters
Risk Level
Risk Level: Medium
Reasoning: Changes will be performed in staging, in case would be an error only would affect only one cluster stone-stage-p01 (+ the lightwell-dev).
Rollback Strategy: Revert this PR + sync in ArgoCD should restore previous ApplicationSet
Validation
For non-production environments this component is only in Staging, so this PR will be the first step before Production. This has been already done in other components like kueue #13631