Split fleet-agent #1905
Split fleet-agent #1905
Conversation
manno
force-pushed
the
split-agent
branch
9 times, most recently
from
57892b5
to
77a8eca
Compare
| func (cs *ClusterStatus) Run(cmd *cobra.Command, args []string) error { | ||
| // provide a logger in the context to be compatible with controller-runtime | ||
| zopts := zap.Options{ | ||
| Development: true, |
There was a problem hiding this comment.
This enables stacktraces on warnings (instead of errors) and disables sampling; are we sure we want this level of verbosity in production?
Same question about register.go and root.go.
There was a problem hiding this comment.
Yes, I would say we will have to come up with logging configuration later. I was hoping we could switch to https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/log/zap#Options.BindFlags
| ctx, cancel := context.WithCancel(ctx) | ||
| // try to register with upstream fleet controller by obtaining | ||
| // a kubeconfig for the upstream cluster | ||
| agentInfo, err := register.Register(ctx, r.Namespace, kc) | ||
| if err != nil { | ||
| logrus.Fatal(err) | ||
| } | ||
|
|
||
| ns, _, err := agentInfo.ClientConfig.Namespace() | ||
| if err != nil { | ||
| logrus.Fatal(err) | ||
| } | ||
|
|
||
| _, err = agentInfo.ClientConfig.ClientConfig() | ||
| if err != nil { | ||
| logrus.Fatal(err) | ||
| } | ||
|
|
||
| setupLog.Info("successfully registered with upstream cluster", "namespace", ns) | ||
| cancel() |
There was a problem hiding this comment.
Why not using defer?
ctx, cancel := context.WithCancel(ctx)
defer cancel()
|
|
||
| agentInfo, err := register.Get(ctx, namespace, kc) | ||
| if err != nil { | ||
| logrus.Fatal(err) |
There was a problem hiding this comment.
Probably just moved from somewhere else, but why all these logrus.Fatal if this method already returns an error?
There was a problem hiding this comment.
Yes, all these logrus calls should be removed in the future https://github.com/rancher/fleet/pull/1772/files#diff-a08a0ecbbab37eaea426b6b8bc55a0d5107c75cac3db92153ef7d3329dc47650R79. But the second point you make is interesting. Why do we exit instead of return... I copied that from start.go 🤷
I'll change it in the next PR :)
Refers to #1772
To prepare the agent for controller-runtime, split the code into three processes:
Only the last part is an actual controller. Cluster status updates work on a timer and registration is done when the agent starts up. A new function "registration.Get" was added, to just return an existing registration to the other processes.
This PR switches the agent to a statefulset, to avoid leader election. I don't expect the agent to scale horizontally, as a controller.
Updating the cluster status with information about the cluster nodes seems unrelated to Fleet's purpose and we might discuss this in the future.
The code of the handlers in the bundledeployment controller is currently spread around the "handler", a deploy "manager" and the helm deployer. The PR takes first steps to consolidate the functionality into the deployer package, making it more obvious which client is used when during reconciliation.
"Trigger" was renamed to "driftdetect" as it uses the helm history to detect drift for deployments. Which is similar to the "monitor", which updates the bundle deployments status.