[marcadamsge]

Implements #1739 . Some design choices:

• the dss-core instances are now stateful and better served by a k8s statefulset, so we have a new dss-raft template instead of extending the dss-core-service.yaml one
• the common parameters to the dss-core are factored out in a template (no duplication)
• rendering fails unless exactly one of cockroachdb.enabled / yugabyte.enabled / raft.enabled is true

This PR was tested by deploying the helm chart to 2 local k3s clusters, each containing 3 dss instances all belonging to the same raft pool. The test configs looked like:

raft:
  # instances to connect to in the current cluster, this also creates the loadbalancers
  nodes:
    - id: 1
      endpoint: "dss-c1-n1.dss.internal:9000"
      ip: "172.30.0.200"
    - id: 2
      endpoint: "dss-c1-n2.dss.internal:9000"
      ip: "172.30.0.201"
    - id: 3
      endpoint: "dss-c1-n3.dss.internal:9000"
      ip: "172.30.0.202"
  # remote instances to connect to
  peers:
    - id: 4
      endpoint: "dss-c2-n1.dss.internal:9000"
    - id: 5
      endpoint: "dss-c2-n2.dss.internal:9000"
    - id: 6
      endpoint: "dss-c2-n3.dss.internal:9000"

[the-glu]

This only allow a local cluster to be run, correct? Do you plan to make it "clusteriable" with others clusters in a flowup PR ?

[ahmadelrouby]

FYI, after merging #1810 + https://github.com/Orbitalize/dss/pull/17/changes You'll need to either use insecure or specify the mTLS args.

[marcadamsge]

@ahmadelrouby thanks for the pointer, I rebased on master and updated the helm chart to support the tsl certificate for mTLS
@the-glu I've tried it with two cluster, and I also updated the format of the values. Here is what the configuration looked like in my little test:

test cluster 1

raft:
  enabled: true
  nodes:
    - name: dss1
      id: 1
      url: "172.18.0.2:30211"
    - name: dss2
      id: 2
      url: "172.18.0.2:30212"
    - name: dss3
      id: 3
      url: "172.18.0.2:30213"
  peers:
    - name: dss4
      id: 4
      url: "172.18.0.4:30211"
    - name: dss5
      id: 5
      url: "172.18.0.4:30212"
    - name: dss6
      id: 6
      url: "172.18.0.4:30213"

test cluster 2:

raft:
  enabled: true
  nodes:
    - name: dss4
      id: 4
      url: "172.18.0.4:30211"
    - name: dss5
      id: 5
      url: "172.18.0.4:30212"
    - name: dss6
      id: 6
      url: "172.18.0.4:30213"
  peers:
    - name: dss1
      id: 1
      url: "172.18.0.2:30211"
    - name: dss2
      id: 2
      url: "172.18.0.2:30212"
    - name: dss3
      id: 3
      url: "172.18.0.2:30213"

[the-glu]

Thanks, second pass of comments

[the-glu]

loadBalancers part should go bellow in the section, to follow existing conventions expect if there is a specific reason no?

[marcadamsge]

Currently it would look something like:

raft:
  nodes:
    - id: 1
      endpoint: "endpoint1:30211"
      awsLB:
        eipAllocationId: eipalloc-0123456789abcdef0
        subnet: subnet-0123456789abcdef0

How else would you have it? We could put the load balancers in another section, how it was done for yugabyte and cockroachDB, but don't you think it's more convenient to have everything here in the same place now that each dss-core has to have it's own ip?

[the-glu]

That was more for normalization - it's actually more convenient there. That probably fine with a TODO to cleanup the old block in the future and have a more convenient way.

[the-glu]

After discussions, I would make it like the rest (so list in loadBalancers, same parameters as now, matching done via position in the list) and introduce the change in a separate PR.

[marcadamsge]

Ok I changed it. Now it looks like:

loadBalancers:
  raft:
    - ip: "172.30.0.200"

raft:
  nodes:
    - id: 1
      endpoint: "dss-c1-n1.dss.internal:9000"
  peers:
    - id: 4
      endpoint: "dss-c2-n1.dss.internal:9000"

The port is still defined on the "endpoint", so that the format is the same as for the peers. I see also that the ports where hardcoded for yugabyte and cockroach and it couldn't be defined in the loadBalancers section, so this is consistent with the current format

[the-glu]

Successfully tested (without TLS and a on master's build) in AWS and GKE :)