-
Notifications
You must be signed in to change notification settings - Fork 305
Add stone-prod-p02 cluster vector-k9e #7966
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
Add stone-prod-p02 cluster vector-k9e #7966
Conversation
Code Review by Gemini--- a/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
+++ b/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
@@ -16,6 +16,8 @@
elements:
# Staging
- nameNormalized: stone-stg-rh01
values.clusterDir: stone-stg-rh01
+ # Private
- nameNormalized: stone-prod-p02
values.clusterDir: stone-prod-p02 The changes in this file are consistent with the commit message and appear correct. --- a/components/mintmaker/staging/base/kustomization.yaml
+++ b/components/mintmaker/staging/base/kustomization.yaml
@@ -14,4 +14,4 @@
- name: quay.io/konflux-ci/mintmaker-renovate-image
newName: quay.io/konflux-ci/mintmaker-renovate-image
- newTag: e5a6e37a42c9d6ca64793c6b125e7082879d121c
+ newTag: 4b5d5fc2a0dec1344969e24d57dd765f2b58f9c2 The change to Suggestion: - newTag: 4b5d5fc2a0dec1344969e24d57dd765f2b58f9c2 And revert the line: + newTag: e5a6e37a42c9d6ca64793c6b125e7082879d121c This change should be part of a separate commit or pull request if it is indeed necessary. |
98891ae
to
533f208
Compare
Code Review by Geminiargo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml Improvement: For better readability and visual separation between different environment groups, consider adding a blank line between the --- a/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
+++ b/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
@@ -17,6 +17,7 @@
elements:
# Staging
- nameNormalized: stone-stg-rh01
values.clusterDir: stone-stg-rh01
+
# Private
- nameNormalized: stone-prod-p02
values.clusterDir: stone-prod-p02 |
/lgtm |
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.
This is not enough to deploy to a single prod cluster.
For internal prod, we also need to remove this block, and manage the other internal clusters too.
Usually we deploy an empty-base
to them. Doing something like this will deploy the empty-base
folder to any enabled but not matched clusters. We need to create the empty-base
folder under production
too.
The empty-base
folder needs to contain at least one resource. We usually deploy an empty namespace.
533f208
to
4a13a89
Compare
Code Review by Gemini```diff
--- a/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
+++ b/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
@@ -13,20 +13,8 @@
- list:
elements:
# Staging
- # - nameNormalized: stone-stage-p01
- # values.clusterDir: stone-stage-p01
- nameNormalized: stone-stg-rh01
values.clusterDir: stone-stg-rh01
# Private
- # - nameNormalized: kflux-ocp-p01
- # values.clusterDir: kflux-ocp-p01
- # - nameNormalized: stone-prod-p01
- # values.clusterDir: stone-prod-p01
- nameNormalized: stone-prod-p02
values.clusterDir: stone-prod-p02
- # - nameNormalized: pentest-p01
- # values.clusterDir: pentest-p01
- # Public
- # - nameNormalized: stone-prd-rh01
- # values.clusterDir: stone-prd-rh01
- # - nameNormalized: kflux-prd-rh02
- # values.clusterDir: kflux-prd-rh02
- # - nameNormalized: kflux-prd-rh03
- # values.clusterDir: kflux-prd-rh03
- # - nameNormalized: kflux-rhel-p01
- # values.clusterDir: kflux-rhel-p01 Explanation: The commented-out lines add unnecessary clutter to the configuration file. It's best to keep the configuration clean and only include active entries. If these are placeholders for future clusters, they should be added when those clusters are actually onboarded. General Improvement: The changes in |
Thanks for the review! I totally missed the deletion of the application. So I removed that block but with regards to the generator I didn't do any changes. I was comparing with what we have under the |
4a13a89
to
ad79080
Compare
Code Review by Gemini### argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
**Improvements:**
* **Remove commented-out clusters:** The file contains several commented-out cluster configurations. While these might be for historical reference or future use, they add clutter and reduce readability. It's generally better to remove inactive configurations unless they are temporarily disabled for a specific, short-term reason.
```diff
--- a/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
+++ b/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
@@ -13,22 +13,12 @@
- list:
elements:
# Staging
- # - nameNormalized: stone-stage-p01
- # values.clusterDir: stone-stage-p01
- nameNormalized: stone-stg-rh01
values.clusterDir: stone-stg-rh01
# Private
- # - nameNormalized: kflux-ocp-p01
- # values.clusterDir: kflux-ocp-p01
- # - nameNormalized: stone-prod-p01
- # values.clusterDir: stone-prod-p01
- nameNormalized: stone-prod-p02
values.clusterDir: stone-prod-p02
- # - nameNormalized: pentest-p01
- # values.clusterDir: pentest-p01
- # Public
- # - nameNormalized: stone-prd-rh01
- # values.clusterDir: stone-prd-rh01
- # - nameNormalized: kflux-prd-rh02
- # values.clusterDir: kflux-prd-rh02
- # - nameNormalized: kflux-prd-rh03
- # values.clusterDir: kflux-prd-rh03
- # - nameNormalized: kflux-rhel-p01
- # values.clusterDir: kflux-rhel-p01
template:
metadata:
name: vector-kubearchive-log-collector-{{nameNormalized}}
```
### argo-cd-apps/overlays/production-downstream/delete-applications.yaml
No issues or improvements found. The removal of the `$patch: delete` for `vector-kubearchive-log-collector` is consistent with the goal of deploying it to a new production cluster. |
ad79080
to
67e37cd
Compare
Code Review by Gemini## Code Review
### `argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml`
**Improvements:**
* **Readability of commented lines:** The addition of several commented-out lines (lines 11-13, 15-17, 19-24) around the active cluster definitions, while potentially for context, adds visual noise to the configuration.
Consider if these commented blocks are truly necessary in the active configuration file. If they are for reference or examples, moving them to a separate documentation or example file could improve the readability of the main configuration.
### `argo-cd-apps/overlays/production-downstream/delete-applications.yaml`
**No issues found.**
The removal of the `vector-kubearchive-log-collector` ApplicationSet from the `delete-applications.yaml` is consistent with the goal of deploying this application, as stated in the commit message. This change correctly ensures the application will not be deleted in the `production-downstream` environment. |
...ber/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
Show resolved
Hide resolved
67e37cd
to
0e84caa
Compare
Code Review by Gemini### argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
**Improvements:**
* **Remove commented-out clusters:** The file contains several commented-out cluster configurations. While these might be for historical reference or future use, they add clutter and reduce readability. It's generally better to remove inactive configurations unless they are temporarily disabled for a specific, short-term reason.
```diff
--- a/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
+++ b/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
@@ -13,22 +13,12 @@
- list:
elements:
# Staging
- # - nameNormalized: stone-stage-p01
- # values.clusterDir: stone-stage-p01
- nameNormalized: stone-stg-rh01
values.clusterDir: stone-stg-rh01
# Private
- # - nameNormalized: kflux-ocp-p01
- # values.clusterDir: kflux-ocp-p01
- # - nameNormalized: stone-prod-p01
- # values.clusterDir: stone-prod-p01
- nameNormalized: stone-prod-p02
values.clusterDir: stone-prod-p02
- # - nameNormalized: pentest-p01
- # values.clusterDir: pentest-p01
- # Public
- # - nameNormalized: stone-prd-rh01
- # values.clusterDir: stone-prd-rh01
- # - nameNormalized: kflux-prd-rh02
- # values.clusterDir: kflux-prd-rh02
- # - nameNormalized: kflux-prd-rh03
- # values.clusterDir: kflux-prd-rh03
- # - nameNormalized: kflux-rhel-p01
- # values.clusterDir: kflux-rhel-p01
template:
metadata:
name: vector-kubearchive-log-collector-{{nameNormalized}}
```
### argo-cd-apps/overlays/production-downstream/delete-applications.yaml
No issues or improvements found. The removal of the `$patch: delete` for `vector-kubearchive-log-collector` is consistent with the goal of deploying it to a new production cluster. |
0e84caa
to
df91f80
Compare
Code Review by Gemini--- a/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
+++ b/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
@@ -13,21 +13,10 @@
sourceRoot: components/vector-kubearchive-log-collector
environment: staging
- clusterDir: ""
+ clusterDir: empty-base
- list:
elements:
- # Staging
- # - nameNormalized: stone-stage-p01
- # values.clusterDir: stone-stage-p01
- nameNormalized: stone-stg-rh01
values.clusterDir: stone-stg-rh01
- # Private
- # - nameNormalized: kflux-ocp-p01
- # values.clusterDir: kflux-ocp-p01
- # - nameNormalized: stone-prod-p01
- # values.clusterDir: stone-prod-p01
- nameNormalized: stone-prod-p02
values.clusterDir: stone-prod-p02
- # - nameNormalized: pentest-p01
- # values.clusterDir: pentest-p01
- # Public
- # - nameNormalized: stone-prd-rh01
- # values.clusterDir: stone-prd-rh01
- # - nameNormalized: kflux-prd-rh02
- # values.clusterDir: kflux-prd-rh02
- # - nameNormalized: kflux-prd-rh03
- # values.clusterDir: kflux-prd-rh03
- # - nameNormalized: kflux-rhel-p01
- # values.clusterDir: kflux-rhel-p01 Improvement: --- a/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
+++ b/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
@@ -13,21 +13,10 @@
sourceRoot: components/vector-kubearchive-log-collector
environment: staging
- clusterDir: ""
+ clusterDir: empty-base
- list:
elements:
- # Staging
- # - nameNormalized: stone-stage-p01
- # values.clusterDir: stone-stage-p01
- nameNormalized: stone-stg-rh01
values.clusterDir: stone-stg-rh01
- # Private
- # - nameNormalized: kflux-ocp-p01
- # values.clusterDir: kflux-ocp-p01
- # - nameNormalized: stone-prod-p01
- # values.clusterDir: stone-prod-p01
- nameNormalized: stone-prod-p02
values.clusterDir: stone-prod-p02
- # - nameNormalized: pentest-p01
- # values.clusterDir: pentest-p01
- # Public
- # - nameNormalized: stone-prd-rh01
- # values.clusterDir: stone-prd-rh01
- # - nameNormalized: kflux-prd-rh02
- # values.clusterDir: kflux-prd-rh02
- # - nameNormalized: kflux-prd-rh03
- # values.clusterDir: kflux-prd-rh03
- # - nameNormalized: kflux-rhel-p01
- # values.clusterDir: kflux-rhel-p01 |
df91f80
to
702abc9
Compare
Code Review by Gemini--- a/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
+++ b/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
@@ -13,21 +13,10 @@
sourceRoot: components/vector-kubearchive-log-collector
environment: staging
- clusterDir: ""
+ clusterDir: empty-base
- list:
elements:
- # Staging
- # - nameNormalized: stone-stage-p01
- # values.clusterDir: stone-stage-p01
- nameNormalized: stone-stg-rh01
values.clusterDir: stone-stg-rh01
- # Private
- # - nameNormalized: kflux-ocp-p01
- # values.clusterDir: kflux-ocp-p01
- # - nameNormalized: stone-prod-p01
- # values.clusterDir: stone-prod-p01
- nameNormalized: stone-prod-p02
values.clusterDir: stone-prod-p02
- # - nameNormalized: pentest-p01
- # values.clusterDir: pentest-p01
- # Public
- # - nameNormalized: stone-prd-rh01
- # values.clusterDir: stone-prd-rh01
- # - nameNormalized: kflux-prd-rh02
- # values.clusterDir: kflux-prd-rh02
- # - nameNormalized: kflux-prd-rh03
- # values.clusterDir: kflux-prd-rh03
- # - nameNormalized: kflux-rhel-p01
- # values.clusterDir: kflux-rhel-p01 Improvement: --- a/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
+++ b/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
@@ -13,21 +13,10 @@
sourceRoot: components/vector-kubearchive-log-collector
environment: staging
- clusterDir: ""
+ clusterDir: empty-base
- list:
elements:
- # Staging
- # - nameNormalized: stone-stage-p01
- # values.clusterDir: stone-stage-p01
- nameNormalized: stone-stg-rh01
values.clusterDir: stone-stg-rh01
- # Private
- # - nameNormalized: kflux-ocp-p01
- # values.clusterDir: kflux-ocp-p01
- # - nameNormalized: stone-prod-p01
- # values.clusterDir: stone-prod-p01
- nameNormalized: stone-prod-p02
values.clusterDir: stone-prod-p02
- # - nameNormalized: pentest-p01
- # values.clusterDir: pentest-p01
- # Public
- # - nameNormalized: stone-prd-rh01
- # values.clusterDir: stone-prd-rh01
- # - nameNormalized: kflux-prd-rh02
- # values.clusterDir: kflux-prd-rh02
- # - nameNormalized: kflux-prd-rh03
- # values.clusterDir: kflux-prd-rh03
- # - nameNormalized: kflux-rhel-p01
- # values.clusterDir: kflux-rhel-p01 |
components/vector-kubearchive-log-collector/empty-base/kustomization.yaml
Outdated
Show resolved
Hide resolved
702abc9
to
25c944f
Compare
Code Review by Gemini### Review of Changes
The changes aim to add the `stone-prod-p02` cluster to the `vector-kubearchive-log-collector` Argo CD ApplicationSet and enable its deployment in the `production-downstream` environment. The introduction of `empty-base` also seems to refactor how generic staging clusters are handled for this application.
Here are the identified issues and suggested improvements:
---
**File: `components/vector-kubearchive-log-collector/empty-base/kustomization.yaml`**
**Issue:**
The `empty-base/kustomization.yaml` file sets the `namespace` to `product-kubearchive-logging`. However, this `empty-base` is used as the `clusterDir` for the `clusters` generator in `vector-kubearchive-log-collector.yaml`, which is configured with `environment: staging`. This means that any staging cluster discovered by this generator (and not explicitly overridden by the `list` generator) will attempt to deploy `vector-kubearchive-log-collector` into a namespace typically reserved for production environments. This can lead to incorrect environment isolation and potential conflicts.
**Suggested Change:**
Change the namespace in `empty-base/kustomization.yaml` to a staging-appropriate namespace. If a specific staging namespace for this application exists, use that. Otherwise, a generic staging namespace or removing the namespace definition to be set by higher-level overlays would be more appropriate. Assuming `staging-kubearchive-logging` is a suitable staging namespace:
```diff
--- a/components/vector-kubearchive-log-collector/empty-base/kustomization.yaml
+++ b/components/vector-kubearchive-log-collector/empty-base/kustomization.yaml
@@ -0,0 +1,3 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
-namespace: product-kubearchive-logging
+namespace: staging-kubearchive-logging |
Signed-off-by: Marta Anon <[email protected]>
25c944f
to
d4f3499
Compare
Code Review by Gemini```diff
--- a/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
+++ b/argo-cd-apps/base/member/infra-deployments/vector-kubearchive-log-collector/vector-kubearchive-log-collector.yaml
@@ -5,25 +5,46 @@ metadata:
spec:
generators:
- merge:
mergeKeys:
- nameNormalized
generators:
- clusters:
values:
sourceRoot: components/vector-kubearchive-log-collector
environment: staging
- clusterDir: ""
+ clusterDir: empty-base
- list:
elements:
+ # Staging
+ # - nameNormalized: stone-stage-p01
+ # values.clusterDir: stone-stage-p01
- nameNormalized: stone-stg-rh01
values.clusterDir: stone-stg-rh01
+ # Private
+ # - nameNormalized: kflux-ocp-p01
+ # values.clusterDir: kflux-ocp-p01
+ # - nameNormalized: stone-prod-p01
+ # values.clusterDir: stone-prod-p01
+ - nameNormalized: stone-prod-p02
+ values.clusterDir: stone-prod-p02
+ # - nameNormalized: pentest-p01
+ # values.clusterDir: pentest-p01
+ # Public
+ # - nameNormalized: stone-prd-rh01
+ # values.clusterDir: stone-prd-rh01
+ # - nameNormalized: kflux-prd-rh02
+ # values.clusterDir: kflux-prd-rh02
+ # - nameNormalized: kflux-prd-rh03
+ # values.clusterDir: kflux-prd-rh03
+ # - nameNormalized: kflux-rhel-p01
+ # values.clusterDir: kflux-rhel-p01
template:
metadata:
name: vector-kubearchive-log-collector-{{nameNormalized}}
annotations:
argocd.argoproj.io/sync-wave: "100"
argocd.argoproj.io/refresh: "hard"
spec:
ignoreDifferences:
# Ignore Helm-generated dynamic content that causes drift
- group: apps No issues found. The change correctly adds --- a/argo-cd-apps/overlays/production-downstream/delete-applications.yaml
+++ b/argo-cd-apps/overlays/production-downstream/delete-applications.yaml
@@ -28,17 +28,10 @@ apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
name: quality-dashboard
$patch: delete
---
apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
name: konflux-kite
$patch: delete
----
-apiVersion: argoproj.io/v1alpha1
-kind: ApplicationSet
-metadata:
- name: vector-kubearchive-log-collector
-$patch: delete
- No issues found. The removal of the --- a/argo-cd-apps/overlays/production-downstream/kustomization.yaml
+++ b/argo-cd-apps/overlays/production-downstream/kustomization.yaml
@@ -195,20 +195,25 @@ patches:
- path: production-overlay-patch.yaml
target:
kind: ApplicationSet
version: v1alpha1
name: crossplane-config
- path: production-overlay-patch.yaml
target:
kind: ApplicationSet
version: v1alpha1
name: konflux-info
+ - path: production-overlay-patch.yaml
+ target:
+ kind: ApplicationSet
+ version: v1alpha1
+ name: vector-kubearchive-log-collector
- path: production-overlay-patch.yaml
target:
kind: ApplicationSet
version: v1alpha1
name: vector-tekton-logs-collector
- path: production-overlay-patch.yaml
target:
kind: ApplicationSet
version: v1alpha1
name: cost-management No issues found. The addition of the patch for --- /dev/null
+++ b/components/vector-kubearchive-log-collector/production/empty-base/kustomization.yaml
@@ -0,0 +1,3 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+resources: []
\ No newline at end of file Issue: Missing newline at the end of the file. Suggested change: --- a/components/vector-kubearchive-log-collector/production/empty-base/kustomization.yaml
+++ b/components/vector-kubearchive-log-collector/production/empty-base/kustomization.yaml
@@ -0,0 +1,3 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources: []
+ |
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.
/lgtm
/approve |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: filariow, hugares, maruiz93 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
074847a
into
redhat-appstudio:main
The changes added in #7895 weren't applied because we miss adding the cluster in the Argo App