Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: correctly assign apps to nested envs #389

Merged
merged 3 commits into from
Feb 2, 2025
Merged

fix: correctly assign apps to nested envs #389

merged 3 commits into from
Feb 2, 2025

Conversation

Zebradil
Copy link
Member

@Zebradil Zebradil commented Feb 1, 2025

Before, if myks is tasked to process several intersecting environments, the
application list from the last collected environment was winning.

For example, with the following search paths:

envs/foo: app1, app2
envs/foo/bar: app3

The environment envs/foo/bar was processed with app3 instead of app1,
app2, and app3.

Moreover, empty application lists were not handled correctly. If an application
list is empty, all applications in the environment should be processed.

@Zebradil
fix: correctly assign apps to nested envs
4ccbed6 
Before, if myks is tasked to process several intersecting environments, the
application list from the last collected environment was winning.

For example, with the following search paths:

```
envs/foo: app1, app2
envs/foo/bar: app3
```

The environment `envs/foo/bar` was processed with `app3` instead of `app1`,
`app2`, and `app3`.

Moreover, empty application lists were not handled correctly. If an application
list is empty, all applications in the environment should be processed.
@kbudde
Copy link
Member

kbudde commented Feb 2, 2025

@Zebradil I tried to reproduce the issue without any luck.
Can you elaborate a bit more what you were trying to do?

I tried the default repo from myks init where we have one environment mykso/dev and modified it a bit.

envs/mykso/env-data.mykso.yaml

#@data/values
---
environment:
  id: mykso
  applications:
    - proto: argocd

envs/mykso/dev/env-data.dev.yaml

#@data/values
---
environment:
  id: mykso-dev
  applications:
    - proto: httpbingo

For me old and new myks are doing the same things...

myks from this PR (with one additional log to log the search path in each loop):

❯ myks-new render mykso,mykso/dev
10:16AM INF Using config file: /home/kbudde/pprojects/myks-base/.myks.yaml
10:16AM INF Setting log level to: info
10:16AM ERR Ran git
git -C . remote get-url origin
10:16AM ERR Fehler: Remote-Repository 'origin' nicht gefunden

10:16AM WRN Unable to set git repo branch error="exit status 2"
looking into envs/mykso
looking into envs/mykso/dev
looking into envs/mykso/dev
10:16AM INF [mykso > argocd > render-helm] Helm chart rendered
10:16AM INF [mykso-dev > argocd > render-helm] Helm chart rendered
10:16AM INF [mykso-dev > httpbingo > render-helm] Helm chart rendered
10:16AM INF [mykso-dev > httpbingo > render-ytt] Local ytt rendered
10:16AM INF [mykso-dev > httpbingo > render] Completed
10:16AM INF [mykso-dev > argocd > render-ytt] Local ytt rendered
10:16AM INF [mykso > argocd > render-ytt] Local ytt rendered
10:16AM INF [mykso-dev > argocd > render] Completed
10:16AM INF [mykso > argocd > render] Completed

myks from main branch:

❯ myks-main render mykso,mykso/dev
10:17AM INF Using config file: /home/kbudde/pprojects/myks-base/.myks.yaml
10:17AM INF Setting log level to: info
10:17AM ERR Ran git
git -C . remote get-url origin
10:17AM ERR Fehler: Remote-Repository 'origin' nicht gefunden

10:17AM WRN Unable to set git repo branch error="exit status 2"
10:17AM INF [mykso > argocd > render-helm] Helm chart rendered
10:17AM INF [mykso-dev > argocd > render-helm] Helm chart rendered
10:17AM INF [mykso-dev > httpbingo > render-helm] Helm chart rendered
10:17AM INF [mykso-dev > httpbingo > render-ytt] Local ytt rendered
10:17AM INF [mykso-dev > httpbingo > render] Completed
10:17AM INF [mykso > argocd > render-ytt] Local ytt rendered
10:17AM INF [mykso-dev > argocd > render-ytt] Local ytt rendered
10:17AM INF [mykso-dev > argocd > render] Completed
10:17AM INF [mykso > argocd > render] Completed

@kbudde
Copy link
Member

kbudde commented Feb 2, 2025 •
edited
Loading

There are differences between both versions if I provide the list of applications to render

myks render mykso,mykso/dev argocd,httpbingo

❯ myks-main render mykso,mykso/dev argocd,httpbingo
10:25AM INF Using config file: /home/kbudde/pprojects/myks-base/.myks.yaml
10:25AM INF Setting log level to: info
10:25AM ERR Ran git
git -C . remote get-url origin
10:25AM ERR Fehler: Remote-Repository 'origin' nicht gefunden

10:25AM WRN Unable to set git repo branch error="exit status 2"
10:25AM WRN [mykso > init] Application not found app=httpbingo dir=envs/mykso
10:25AM INF [mykso > argocd > render-helm] Helm chart rendered
10:25AM INF [mykso-dev > argocd > render-helm] Helm chart rendered
10:25AM INF [mykso-dev > httpbingo > render-helm] Helm chart rendered
10:25AM INF [mykso-dev > httpbingo > render-ytt] Local ytt rendered
10:25AM INF [mykso-dev > httpbingo > render] Completed
10:25AM INF [mykso-dev > argocd > render-ytt] Local ytt rendered
10:25AM INF [mykso > argocd > render-ytt] Local ytt rendered
10:25AM INF [mykso > argocd > render] Completed
10:25AM INF [mykso-dev > argocd > render] Completed
❯ myks-new render mykso,mykso/dev argocd,httpbingo
10:25AM INF Using config file: /home/kbudde/pprojects/myks-base/.myks.yaml
10:25AM INF Setting log level to: info
10:25AM ERR Ran git
git -C . remote get-url origin
10:25AM ERR Fehler: Remote-Repository 'origin' nicht gefunden

10:25AM WRN Unable to set git repo branch error="exit status 2"
looking into envs/mykso
looking into envs/mykso/dev
looking into envs/mykso/dev
10:25AM WRN [mykso > init] Application not found app=httpbingo dir=envs/mykso
10:25AM INF [mykso > argocd > render-helm] Helm chart rendered
10:25AM INF [mykso-dev > argocd > render-helm] Helm chart rendered
10:25AM INF [mykso-dev > argocd > render-helm] Helm chart rendered
10:25AM INF [mykso-dev > httpbingo > render-helm] Helm chart rendered
10:25AM INF [mykso-dev > httpbingo > render-helm] Helm chart rendered
10:25AM INF [mykso-dev > httpbingo > render-ytt] Local ytt rendered
10:25AM INF [mykso-dev > httpbingo > render-ytt] Local ytt rendered
10:25AM INF [mykso-dev > httpbingo > render] Completed
10:25AM INF [mykso-dev > httpbingo > render] Completed
10:25AM INF [mykso > argocd > render-ytt] Local ytt rendered
10:25AM INF [mykso-dev > argocd > render-ytt] Local ytt rendered
10:25AM INF [mykso-dev > argocd > render-ytt] Local ytt rendered
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/customresourcedefinition-applications.argoproj.io.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/customresourcedefinition-applicationsets.argoproj.io.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/customresourcedefinition-appprojects.argoproj.io.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/serviceaccount-argocd-application-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/serviceaccount-argocd-applicationset-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/serviceaccount-argocd-dex-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/serviceaccount-argocd-notifications-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/serviceaccount-argocd-redis-ha.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/serviceaccount-argocd-redis-ha-haproxy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/serviceaccount-argocd-repo-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/serviceaccount-argocd-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/role-argocd-application-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/role-argocd-applicationset-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/role-argocd-dex-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/role-argocd-notifications-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/role-argocd-redis-ha.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/role-argocd-redis-ha-haproxy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/role-argocd-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/clusterrole-argocd-application-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/clusterrole-argocd-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/rolebinding-argocd-application-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/rolebinding-argocd-applicationset-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/rolebinding-argocd-dex-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/rolebinding-argocd-notifications-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/rolebinding-argocd-redis-ha.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/rolebinding-argocd-redis-ha-haproxy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/rolebinding-argocd-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/clusterrolebinding-argocd-application-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/clusterrolebinding-argocd-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/configmap-argocd-cm.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/configmap-argocd-cmd-params-cm.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/configmap-argocd-gpg-keys-cm.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/configmap-argocd-notifications-cm.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/configmap-argocd-rbac-cm.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/configmap-argocd-redis-ha-configmap.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/configmap-argocd-redis-ha-health-configmap.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/configmap-argocd-ssh-known-hosts-cm.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/configmap-argocd-tls-certs-cm.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/secret-argocd-notifications-secret.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/secret-argocd-secret.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-applicationset-controller.yaml
10:25AM INF [mykso-dev > argocd > render] Completed
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-dex-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-metrics.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-notifications-controller-metrics.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-redis-ha.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-redis-ha-announce-0.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-redis-ha-announce-1.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-redis-ha-announce-2.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-redis-ha-haproxy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-repo-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/service-argocd-server-metrics.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/deployment-argocd-applicationset-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/deployment-argocd-dex-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/deployment-argocd-notifications-controller.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/deployment-argocd-redis-ha-haproxy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/deployment-argocd-repo-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/deployment-argocd-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/statefulset-argocd-application-controller.yaml
10:25AM INF [mykso > argocd > render] Completed
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/statefulset-argocd-redis-ha-server.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/networkpolicy-argocd-application-controller-network-policy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/networkpolicy-argocd-applicationset-controller-network-policy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/networkpolicy-argocd-dex-server-network-policy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/networkpolicy-argocd-notifications-controller-network-policy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/networkpolicy-argocd-redis-ha-proxy-network-policy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/networkpolicy-argocd-redis-ha-server-network-policy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/networkpolicy-argocd-repo-server-network-policy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/networkpolicy-argocd-server-network-policy.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/configmap-cmp-plugin.yaml
10:25AM WRN [mykso-dev > argocd > slice] File already exists. Consider enabling render.includeNamespace file=rendered/envs/mykso-dev/argocd/namespace-argocd.yaml
10:25AM INF [mykso-dev > argocd > render] Completed

@Zebradil
Copy link
Member Author

Zebradil commented Feb 2, 2025

Hey @kbudde, thank you for looking into this.

The key thing here is to use the smart mode, i.e. you need to run myks sync without any arguments.

I've prepared a reproduction to better demonstrate the issue.

The change we'll be using for the test is an addition of two files to two different applications on different levels of the environment hierarchy:

A   envs/_apps/httpbingo/ytt/smart-mode.test
A   envs/mykso/dev/_apps/argocd/ytt/smart-mode.test

I expect both httpbingo and argocd to be detected by the smart mode and processed.

cd "$(mktemp -d)"
git clone https://github.com/Zebradil/myks-389-repro.git
git checkout smart-mode-test
myks render --smart-mode.base-revision=smart-mode-base --log-level=debug 2>&1 | rg '(Detected changes|Collected environments)'

Output before the fix:

10:41AM DBG [global] Collected environments envToAppMap={"envs/mykso/dev":[]}
10:41AM DBG [global] Detected changes apps=["httpbingo"] env=envs
10:41AM DBG [global] Detected changes apps=["argocd"] env=envs/mykso/dev
10:41AM DBG [global] Collected environments envToAppMap={"envs/mykso/dev":["argocd"]}

Note the last line: only argocd app was collected for rendering, even though httpbingo was detected correctly by the smart mode.

Output after the fix:

10:42AM DBG [global] Collected environments envToAppMap={"envs/mykso/dev":[]}
10:42AM DBG [global] Detected changes apps=["httpbingo"] env=envs
10:42AM DBG [global] Detected changes apps=["argocd"] env=envs/mykso/dev
10:42AM DBG [global] Collected environments envToAppMap={"envs/mykso/dev":["httpbingo","argocd"]}

Here in the last line we now see both argocd and httpbingo.

@Zebradil
Copy link
Member Author

Zebradil commented Feb 2, 2025

I'll take a look into the duplicated rendering from this comment: #389 (comment)

@Zebradil
Copy link
Member Author

Zebradil commented Feb 2, 2025 •
edited
Loading

Yeah, it's a new bug:

myks render mykso,mykso/dev argocd --log-level=debug 2>&1 | rg '(Detected changes|Collected environments)'
10:53AM DBG [global] Collected environments envToAppMap={"envs/mykso/dev":["argocd","argocd"]}

Note duplicated argocd applications collected.

@Zebradil
Copy link
Member Author

Zebradil commented Feb 2, 2025

Fixed. Also added tests.

@Zebradil Zebradil requested a review from kbudde February 2, 2025 15:49
@Zebradil
Copy link
Member Author

Zebradil commented Feb 2, 2025

@kbudde I'll merge this for now, because I need this fix asap. But I'd appreciate if you take another look and provide a feedback if you see something.

@Zebradil Zebradil merged commit 39f185d into main Feb 2, 2025
7 checks passed
@Zebradil Zebradil deleted the fix-apps-on-env-collection branch February 2, 2025 15:51
@mykso-bot mykso-bot mentioned this pull request Feb 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kbudde kbudde kbudde approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Zebradil @kbudde