Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiple controlplane replicas #659

Merged
merged 1 commit into from
Jun 30, 2024

Conversation

orozery
Copy link
Collaborator

@orozery orozery commented Jun 24, 2024

This PR adds an option for deploying clusterlink with multiple controlplane replicas.

Copy link
Collaborator

@elevran elevran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider if we can use the unset replica to mean run only 1 instance, to maintain backward compatibility

@@ -73,7 +73,9 @@ type PeerOptions struct {
ContainerRegistry string
// Tag represents the tag of the project images.
Tag string
// Dataplanes is the number of dataplanes to create.
// ControlplaneReplicas is the number of controlplanes to create.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: at some point we may want to start grouping CP and DP configurations into a structure for each. That is:

type ControlPlane struct {
   Replicas uint16
   ...
}

type DataPlane struct {
   Type string
   Replicas uint16
   ...
}

@@ -197,6 +197,7 @@ func (s *TestSuite) RunOnAllDataplaneTypes(test func(cfg *util.PeerConfig)) {
s.RunSubTest(dataplaneType, func() {
test(&util.PeerConfig{
DataplaneType: dataplaneType,
Controlplanes: 1,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: is there a way to make the zero value (i.e., replica count is not set) useful? Specifically, can it be used to maintain backward compatibility?
For example, since 0 replicas is not meaningful, treat that as user wanting only a single replica.

@@ -185,6 +188,7 @@ func (o *PeerOptions) Run() error {
FabricCertificate: fabricCert,
PeerCertificate: peerCert,
CACertificate: caCert,
Controlplanes: o.ControlplaneReplicas,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we maintain backward compatibility by defining that if replicas is not set at all (i.e., the zero value for uint16 is set by default) we treat it as if it was set to 1 - we can save changes across multiple files.
Somewhere we can say:

   ...
   if controlplaneReplicas == 0 { // not set, use default replica count
      controlplaneReplicas = 1
   }

@orozery orozery force-pushed the multiple-controlplanes branch from 90f3a32 to f142b1b Compare June 30, 2024 08:07
This commit adds an option for deploying clusterlink with multiple controlplane replicas.

Signed-off-by: Or Ozeri <[email protected]>
@orozery orozery force-pushed the multiple-controlplanes branch from f142b1b to ca5864d Compare June 30, 2024 08:07
@orozery orozery merged commit 7a9b585 into clusterlink-net:main Jun 30, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants