Gitops controller#56
Conversation
Signed-off-by: Ondra Machacek <omachace@redhat.com>
machacekondra
requested review from
Fale,
croadfeldt,
gabriel-farache,
gciavarrini,
jenniferubah,
pkliczewski and
ygalblum
as code owners
|
|
||
| The proposal introduces two resources and a controller: | ||
|
|
||
| 1. **`GitRepository` resource** -- a new DCM API object that configures a |
There was a problem hiding this comment.
I think PUT/PATCH and DELETE should be supported so an user can remove the git repo from the watch list or update the polling frequency and/or path to watch.
If an user deletes the git repo, I would not alter the already deployed resources
There was a problem hiding this comment.
nevermind the fisrt part, I see the endpoints are defined later
| #### Story 3 -- Application removed from Git | ||
|
|
||
| A developer removes an Application YAML file from the watched path and merges | ||
| the change. If `pruneEnabled` is `true`, the GitOps controller detects the |
There was a problem hiding this comment.
to reply to the open question about this: I think have the flag is enough to proceed with the automatic deletion. Maybe for deletion some sort of cooldown should be applied: marked for deletion then after some time, perform the deletion (with the timeout being configurable). This would give the user the opportunity to identify wrong deletion. WDYT?
| A developer modifies an Application YAML file in a feature branch, opens a pull | ||
| request, gets it reviewed and merged to `main`. Within the next polling | ||
| interval, the GitOps controller detects the new commit, diffs the Application | ||
| files, and submits the updated Application through the catalog/placement flow. |
There was a problem hiding this comment.
Why not having a flag to enable/disable automatic update as it is done for deletion?
My main concern is malicious attack when a malicious PR get merged and reach main. It may be useful for some specific, sensitive, repos
| end | ||
| ``` | ||
|
|
||
| ## Implementation History |
There was a problem hiding this comment.
do we want to keep this empty section?
|
|
||
| ## Open Questions | ||
|
|
||
| 1. Should the GitOps controller run as a separate process or as a package inside |
There was a problem hiding this comment.
my main concern about having it running inside DCM is the scalability: it will consume a bit of resources to watch git repos and to run the diffs on all resources hosted on each git repo
On a first iteration, I guess it's fine but we should be careful to not intricates the behaviours too much. Once we will gain experience on this, we can adapt
There was a problem hiding this comment.
I agree, first iteration let's keep it inside, but long term it should definitely be standalone process.
| An operator manually modifies an Application via the DCM API that was originally | ||
| submitted by the GitOps controller. On the next poll cycle, the controller | ||
| detects that DCM state has drifted from Git state, reports this in the | ||
| `GitRepository` status, and re-applies the Git version to restore consistency. |
There was a problem hiding this comment.
what is the update doing? Did we defined it already?
what if the new changes breaks DCM flow? Meaning for instance that the update re-evaluate the policies and that the policies reject the request, what would happen? Would the retryPolicy be used to stop retrying to fix the drift? But then when a new PR is merged to fix the issue, we must reset this
There was a problem hiding this comment.
+1 on this. I think we need to define update
|
|
||
| This enhancement adds Git-driven Application lifecycle management to DCM. A new | ||
| `GitRepository` API resource lets users point DCM at a Git repository containing | ||
| Application YAML definitions. A GitOps controller inside the control plane |
There was a problem hiding this comment.
Is it a CatalogItem or something else?
| them. | ||
|
|
||
| The declarative-api enhancement already identifies "User/GitOps" as a submission | ||
| source (see the architecture diagram in declarative-api.md), confirming that |
There was a problem hiding this comment.
nit: add a link if possible
| - Integrate with the existing catalog/placement flow -- the GitOps controller | ||
| submits Applications through the same internal path as API-driven submissions. | ||
| - Provide sync status reporting and drift detection (Git state vs DCM state). | ||
| - Define the behavior for create, update, and delete of Applications based on |
There was a problem hiding this comment.
IIRC, update was not in scope/implemented so far for the API. Do you think it's a problem? any extra work to do in dcm?
| An operator manually modifies an Application via the DCM API that was originally | ||
| submitted by the GitOps controller. On the next poll cycle, the controller | ||
| detects that DCM state has drifted from Git state, reports this in the | ||
| `GitRepository` status, and re-applies the Git version to restore consistency. |
There was a problem hiding this comment.
+1 on this. I think we need to define update
|
|
||
| ### Implementation Details/Notes/Constraints | ||
|
|
||
| #### GitRepository Resource Schema |
There was a problem hiding this comment.
How do we handle git authentication? I think users will use private gh repo so it's important to handle git credentials
|
|
||
| | Status | Meaning | | ||
| | ------------- | ----------------------------------------- | | ||
| | `Synced` | DCM state matches Git at lastSyncedCommit | |
There was a problem hiding this comment.
Can you clarify this status? When it's synced? What "entities" are we comparing?
| handles `POST /api/v1alpha1/catalog-item-instances`. Catalog resolves the | ||
| blueprint, placement builds the DAG, policy evaluates, and SPRM provisions. | ||
|
|
||
| 2. **Update**: Controller submits an update through catalog. |
There was a problem hiding this comment.
That's the missing point in DCM
| - `gitops.dcm.io/source-path: apps/production/my-app.yaml` | ||
| - `gitops.dcm.io/commit: <sha>` | ||
|
|
||
| This metadata enables drift detection and prevents conflicts with manually |
There was a problem hiding this comment.
How are Git files mapped to DCM instances? What happens on file rename or duplicate names? Is it possible?
| GC->>DB: Update lastSyncedCommit + status | ||
| else No new commit | ||
| GC->>DB: Load git-managed Applications | ||
| GC->>GC: Compare spec hashes |
There was a problem hiding this comment.
Which fields are in the hash?
| SPRM-->>GC: 202 Accepted | ||
| end | ||
| opt pruneEnabled and files deleted | ||
| loop Each deleted Application | ||
| GC->>Cat: Delete CatalogItemInstance | ||
| Cat->>PM: Delete flow | ||
| PM->>SPRM: Delete instance | ||
| end | ||
| end | ||
| GC->>DB: Update lastSyncedCommit + status |
There was a problem hiding this comment.
CAn you clarify this part?
The diagram sets sync complete after 202.
But, for n-tier applications, the status can be Synced while provisioning is still in progress in this way, no?
3 participants
No description provided.