search

kestra-io / plugin-git

Apache License 2.0
3 stars 4 forks source link

If git.Sync includes child namespaces, ensure that flows from that child namespace not present in Git are removed #44

Closed kriko closed 8 months ago

kriko commented 9 months ago

Expected Behavior

Running io.kestra.plugin.git.Sync in a parent namespace should sync all flows from Git to Kestra, overwriting any changes in Kestra, adding new flows and removing any flows in Kestra that are not present in the Git repository.

This seems to work for the namespace where git.Sync is run, however it does not apply directly to child (sub) namespaces.

Actual Behaviour

Kestra namespace structure: ns: test ns: test.child ns: test.child.grandchild

When running git.Sync from ns "test" then:

  1. ns: test flows that are not present in git are removed - as expected
  2. ns: test.child and ns.test.grandchild get flows applied from git, but any existing flows not present in git are not removed.

Steps To Reproduce

No response

Environment Information

Example flow

No response

anna-geller commented 9 months ago

This is by design as namespaces are independent and are not meant to share code by default.

Changed the label to enhancement as this is a feature request. Thanks for the issue!

kriko commented 9 months ago

This is by design as namespaces are independent and are not meant to share code by default.

Changed the label to enhancement as this is a feature request. Thanks for the issue!

If namespaces, including child namespaces should be independent, then git.Sync should not deploy to child namespaces either. Thus the current functionality might be a bit confusing.

However the current behaviour for child namespaces is preferred, because then it would be easy to set up GitOps flows to the root namespace (the parent of the children), have that flow syncing that root namespace and all children (and grandchildren) namespaces - so no additional flows would be necessary.

anna-geller commented 9 months ago

definitely more convenient and intuitive that way, I agree

brian-mulier-p commented 8 months ago

This looks like a scary feature so maybe we should add some boolean property for it as one could want not to manage children namespaces but I understand the point.

The deploy to children namespaces was done for convenience purpose when promoting one environment to the other and it looked non-destructive and unharmful as users who want could deploy children namespaces but ones who don't want would not do anything to children namespaces.

@anna-geller this definitely needs to be discussed or taken with warnings as I believe it could ruin a lot of users

kriko commented 8 months ago

@brian-mulier-p , having an additional boolean property would be okay and maybe necessary to make it explicitly clear how git.Sync behaves.

However, the current behaviour is inconsistent - git.Sync for the direct namespace will overwrite, delete, create flows; but for child namespaces, it only creates or overwrites. This behaviour can lead to orphan flows.

The general idea is not to have too many syncing flows, but one per parent namespace which would handle the sync for all child namespaces as well.

anna-geller commented 8 months ago

The issue will be addressed in the new task PushFlows 🎉 You can follow the progress and updates here https://github.com/kestra-io/plugin-git/issues/56