mysql-operator icon indicating copy to clipboard operation
mysql-operator copied to clipboard

Allow additional service configuration for MysqlCluster

Open ynnt opened this issue 3 years ago • 12 comments

This pull request adds capability to set ServiceSpec for MysqlCluster.

  • [x] I've made sure the Changelog.md will remain up-to-date after this PR is merged.

ynnt avatar Oct 29 '21 12:10 ynnt

Or we can create some exporter services instead of making changes to the original services.

cndoit18 avatar Oct 31 '21 11:10 cndoit18

WDYT, cc @calind

cndoit18 avatar Nov 02 '21 03:11 cndoit18

The MySQLCluster controller/resource should not be responsible for creating any additional services (as is the case of the k8s StatefulSet controller for example). The extra services should be created by the cluster operator.

The place where extra services are created might be within the mysql-cluster helm chart.

calind avatar Nov 04 '21 12:11 calind

The MySQLCluster controller/resource should not be responsible for creating any additional services (as is the case of the k8s StatefulSet controller for example). The extra services should be created by the cluster operator.

The place where extra services are created might be within the mysql-cluster helm chart.

Thank you for your reply, but could you elaborate more on this? This pull request does not add any additional services. Instead, it allows changing the type of existing ones.

ynnt avatar Nov 04 '21 12:11 ynnt

The MySQLCluster controller/resource should not be responsible for creating any additional services (as is the case of the k8s StatefulSet controller for example). The extra services should be created by the cluster operator. The place where extra services are created might be within the mysql-cluster helm chart.

Thank you for your reply, but could you elaborate more on this? This pull request does not add any additional services. Instead, it allows changing the type of existing ones.

You can go in the mysql-cluster helm chart and add two additional services as extensions, and with the extensibility of the chart, a new loadbalancer can be added without change the code

cndoit18 avatar Nov 05 '21 11:11 cndoit18

You can go in the mysql-cluster helm chart and add two additional services as extensions, and with the extensibility of the chart, a new loadbalancer can be added without change the code

I see that. The only issue with this approach is that I am forced to use the Helm chart for creating a cluster. Services will not be created when a cluster is created using a plain yaml file or kustomize.

ynnt avatar Nov 05 '21 13:11 ynnt

You can go in the mysql-cluster helm chart and add two additional services as extensions, and with the extensibility of the chart, a new loadbalancer can be added without change the code

I see that. The only issue with this approach is that I am forced to use the Helm chart for creating a cluster. Services will not be created when a cluster is created using a plain yaml file or kustomize.

right

cndoit18 avatar Nov 05 '21 14:11 cndoit18

right

That's the point of my PR. ;)

ynnt avatar Nov 05 '21 15:11 ynnt

You can go in the mysql-cluster helm chart and add two additional services as extensions, and with the extensibility of the chart, a new loadbalancer can be added without change the code

I see that. The only issue with this approach is that I am forced to use the Helm chart for creating a cluster. Services will not be created when a cluster is created using a plain yaml file or kustomize.

It's probably about design, we need to be careful about extending mysqlcluster in order to keep it easy to use, maybe we can start with helm support? WDYT @ynnt

cndoit18 avatar Nov 15 '21 11:11 cndoit18

I do agree that this PR changes API layout a little bit (though, I do not think it changes UX or introduces any significant changes to the overall application design).

Unfortunately, the Helm chart approach simply does not work for my use case (creating mysql clusters programmatically). :(

So, feel free to close the PR if you believe it adds unnecessary complexity, I will just stick to my fork.

ynnt avatar Nov 15 '21 19:11 ynnt

WDYT @calind

cndoit18 avatar Nov 16 '21 03:11 cndoit18

@cndoit18 @calind Any plans to merge it? We really need this feature

mglants avatar Dec 30 '21 09:12 mglants