refactor(controller): restructure controlplane commands #1658
Conversation
✅ Deploy Preview for docs-kargo-akuity-io ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1658 +/- ##
==========================================
- Coverage 44.04% 43.86% -0.19%
==========================================
Files 207 207
Lines 13133 13187 +54
==========================================
Hits 5784 5784
- Misses 7106 7160 +54
Partials 243 243 ☔ View full report in Codecov by Sentry. |
hiddeco
force-pushed
the
refactor-controlplane-cmd
branch
from
71e6424 to
400c73a
Compare
hiddeco
changed the title
hiddeco
force-pushed
the
refactor-controlplane-cmd
branch
from
400c73a to
7ff76c6
Compare
|
At a glance this looks fantastic, and I agree it's a step in the right direction with room for further improvement. I do want to put this off to v0.6.0 though. |
cmd/controlplane/controller.go
Outdated
| cacheOpts := cache.Options{} // Watches all namespaces by default | ||
| if o.AnalysisRunNamespace != "" { | ||
| // TODO: When NOT sharded, Kargo can simply create AnalysisRun | ||
| // resources in the project namespaces. When sharded, AnalysisRun | ||
| // resources must be created IN the shard clusters (not the Kargo | ||
| // control plane cluster) and project namespaces do not exist in the | ||
| // shard clusters. We need a place to put them, so for now we allow | ||
| // the user to specify a namespace that that exists on each shard for | ||
| // this purpose. Note that the namespace does not need to be the same | ||
| // on every shard. This may be one of the weaker points in our tenancy | ||
| // model and can stand to be improved. | ||
| cacheOpts.DefaultNamespaces = map[string]cache.Config{ | ||
| o.AnalysisRunNamespace: {}, | ||
| } | ||
| } |
cmd/controlplane/controller.go
Outdated
| RolloutsKubeConfig string | ||
| AnalysisRunNamespace string |
There was a problem hiding this comment.
Both of these went away in #1706.
AnalysisRuns now live exclusively in the Kargo control plane, so there's never a need for a separate client nor a need to fall back on using any namespace that isn't a Project's own namespace.
cmd/controlplane/controller.go
Outdated
| ) | ||
| } | ||
|
|
||
| func (o *controllerOptions) setupRolloutsManager(ctx context.Context) (manager.Manager, error) { |
|
Apart from conflicts with #1706, this looks good. |
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
Signed-off-by: Hidde Beydals <hidde@hhh.computer>
hiddeco
force-pushed
the
refactor-controlplane-cmd
branch
from
7ff76c6 to
1730061
Compare
|
Think I managed to rebase this correctly, but a careful look with a second pair of 👀 would be much appreciated. |
|
@hiddeco it looked good before rebase... I'm just going to run it. |
|
Works flawlessly. Startup logs look just like they did before. A few e2e cases all check out. |
This is the "control plane" version of #1547.
While it does move things into a better direction by creating a better separation of concerns, this continues to be a candidate to be revised again when we have time.