feat: created decentralized topology, capability nats and nats super cluster #221
johnride
Reference in New Issue
Block a user
No description provided.
Delete Branch "feat/nats_capability"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Good step in the right direction. I made comments as I read the code, it will probably require some discussion, unless everything I am saying really makes sense.
@@ -523,3 +518,1 @@let ca_cert = String::from_utf8(ca_cert.0.clone()).map_err(|_| {ExecutorError::UnexpectedError("ca.crt is not valid UTF-8".into())})?;loop {This retry logic does not belong within the get_ca_cert function.
It is the caller's responsibility to decide what he wants to do when the cert does not exist.
It would be relevant to create a utility function in harmony such as
retry(FnOnce..., backoff: bool, max_attempts : usize, attempt_timeout: Option<Duration> , total_timeout: Option<Duration>)There is the retry crate that seems close enough https://docs.rs/retry/latest/retry/
But I did not verify if/how it supports async.
It is also possible that tokio itself provides a retry utility. Worth checking.
@@ -559,6 +575,64 @@ impl K8sAnywhereTopology {}}async fn build_ca_bundle_secret(This does not belong in this file or as part of an
impl K8sAnywhereTopology.It should be somewhere in a k8s cert manager module.
It will help with reusability, maintainability, testability and readability.
@@ -0,0 +23,4 @@};#[async_trait]impl Nats for K8sAnywhereTopology {Once again, this contains way too many implementation details. There should be a NatsK8s module that contains the nats specific logid, then k8s anywhere only does the dispatching depending on its own logic/state (which k8s is the anywhere pointing to right now). K8sAnywhere should not know how to deploy nats. It should tell a natsk8s module "deploy to okd" or something along those lines.
@@ -0,0 +77,4 @@.await.map_err(|e| format!("Failed to deploy K8s ingress: {e}"))?;}KubernetesDistribution::Default => {Don't duplicate an identical match arm, rust allows a single body for multiple match cases. Look up the syntax if needed.
@@ -0,0 +7,4 @@};#[async_trait]impl<T: Topology + Nats + TlsRouter + 'static> NatsSupercluster for DecentralizedTopology<T> {Delete this impl. These todo! are a huge smell. And I think it is fine that a decentralized topology does not know how to deploy nats asite of a NatsSupercluster. At least for the current use case. The end user deploying a postgresql will deploy it to a failovertopology, then the failovertopology will depend on a topology that supports a natssupercluster. At least for now.
I may be missing a few details though.
WIP: feat: created decentralized topology, capability nats and nats super clusterto feat: created decentralized topology, capability nats and nats super cluster