Code refactor on options #189

huanwei · 2018-07-21T00:58:35Z

1.Describe what this PR did

Refactor code structure on options.

2.Does this pull request fix one issue?

During the debug and review of the program, I found the following codes in mysql-operator/pkg used/called the struct type of MySQLOperator Options defined in mysql-operator/cmd, while the latter package should be recognized as the program’s entrypoint.

mysql-operator/pkg/controllers/cluster/controller.go 
mysql-operator/pkg/controllers/cluster/controller_test.go 
mysql-operator/pkg/resources/statefulsets/statefulset.go 
mysql-operator/pkg/resources/statefulsets/statefulset_test.go

It looked like the option packages in mysql-operator/cmd/mysql-operator/app/options and mysql-operator/cmd/mysql-agent/app/options here were not in a good place, so I would like to to tiny refactor the code structure.

3.Describe how you did it

I did the following changes:

Move `mysql-operator/cmd/mysql-operator/app/options` to `mysql-operator/pkg/options/operator`
Move `mysql-operator/cmd/mysql-agent/app/options` to `mysql-operator/pkg/options/agent`
Rename struct  type `MySQLOperatorServer` to `MySQLOperatorOpts`, with respect to the struct type of `MysqlAgentOpts`
Adjust the usage of struct type MySQLOperatorOpts passed to the codes in `mysql-operator/pkg` and `mysql-operator/cmd`.

4.Describe how to verify it

Compile the code, build the docker images, and verify the operator, agent and cluster on Kubernetes.

[root@k8s-master ~]# kubectl get pods -n mysql-operator
NAME                              READY     STATUS    RESTARTS   AGE
mysql-cluster-with-3-replicas-0   2/2       Running   0          7d
mysql-cluster-with-3-replicas-1   2/2       Running   0          7d
mysql-cluster-with-3-replicas-2   2/2       Running   0          7d
mysql-operator-78b7d9bfcf-nfcp8   1/1       Running   0          16d

Signed-off-by: huanwei <huan@harmonycloud.cn>

huanwei · 2018-07-23T03:09:38Z

@owainlewis
I have submitted the OCA this morning. Pls kindly check it. Thank you.

delabassee · 2018-07-23T06:05:24Z

@owainlewis FYI, @huanwei 's OCA has been reviewed and approved

Signed-off-by: huanwei <huan@harmonycloud.cn>

owainlewis · 2018-07-24T14:12:12Z

cmd/mysql-operator/app/mysql_operator.go

@@ -28,7 +28,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"

-	options "github.com/oracle/mysql-operator/cmd/mysql-operator/app/options"
+	opratoropts "github.com/oracle/mysql-operator/pkg/options/operator"


Minor typo here (opratoropts => operatoropts)

Thanks for the review.

owainlewis · 2018-07-24T14:12:19Z

cmd/mysql-operator/app/mysql_operator.go

@@ -45,15 +45,15 @@ const (

 // resyncPeriod computes the time interval a shared informer waits before
 // resyncing with the api server.
-func resyncPeriod(s *options.MySQLOperatorServer) func() time.Duration {
+func resyncPeriod(s *opratoropts.MySQLOperatorOpts) func() time.Duration {


Minor typo here (opratoropts => operatoropts)

Thanks for the review.

owainlewis · 2018-07-24T14:12:24Z

cmd/mysql-operator/app/mysql_operator.go

 	return func() time.Duration {
 		factor := rand.Float64() + 1
 		return time.Duration(float64(s.MinResyncPeriod.Nanoseconds()) * factor)
 	}
 }

 // Run starts the mysql-operator controllers. This should never exit.
-func Run(s *options.MySQLOperatorServer) error {
+func Run(s *opratoropts.MySQLOperatorOpts) error {


Minor typo here (opratoropts => operatoropts)

Thanks for the review.

owainlewis · 2018-07-24T14:15:47Z

Thanks for this contribution @huanwei the changes seem reasonable to me in terms of code organisation. If you're happy to fix those typos then we'll get this merged. 👍

owainlewis

Changes look good. Just a few minor typos and then we can merge.

Signed-off-by: huanwei <huan@harmonycloud.cn>

Use --decode for cross OS compatibility (oracle#187)

Signed-off-by: huanwei <huan@harmonycloud.cn>

huanwei

@owainlewis I have fixed the typos. Pls Kindly review. Thanks.

huanwei · 2018-07-24T17:24:19Z

cmd/mysql-operator/app/mysql_operator.go

@@ -28,7 +28,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"

-	options "github.com/oracle/mysql-operator/cmd/mysql-operator/app/options"
+	opratoropts "github.com/oracle/mysql-operator/pkg/options/operator"


Thanks for the review.

huanwei · 2018-07-24T17:24:44Z

cmd/mysql-operator/app/mysql_operator.go

 	return func() time.Duration {
 		factor := rand.Float64() + 1
 		return time.Duration(float64(s.MinResyncPeriod.Nanoseconds()) * factor)
 	}
 }

 // Run starts the mysql-operator controllers. This should never exit.
-func Run(s *options.MySQLOperatorServer) error {
+func Run(s *opratoropts.MySQLOperatorOpts) error {


Thanks for the review.

huanwei · 2018-07-24T17:24:59Z

cmd/mysql-operator/app/mysql_operator.go

@@ -45,15 +45,15 @@ const (

 // resyncPeriod computes the time interval a shared informer waits before
 // resyncing with the api server.
-func resyncPeriod(s *options.MySQLOperatorServer) func() time.Duration {
+func resyncPeriod(s *opratoropts.MySQLOperatorOpts) func() time.Duration {


Thanks for the review.

huanwei

@owainlewis I have fixed the typos. Pls review the changes. Thanks.

huanwei · 2018-07-24T17:54:44Z

cmd/mysql-operator/app/mysql_operator.go

@@ -28,7 +28,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"

-	options "github.com/oracle/mysql-operator/cmd/mysql-operator/app/options"
+	operatoropts "github.com/oracle/mysql-operator/pkg/options/operator"


@owainlewis Pls review the change. Thanks.

huanwei · 2018-07-24T17:55:05Z

cmd/mysql-operator/app/mysql_operator.go

@@ -45,15 +45,15 @@ const (

 // resyncPeriod computes the time interval a shared informer waits before
 // resyncing with the api server.
-func resyncPeriod(s *options.MySQLOperatorServer) func() time.Duration {
+func resyncPeriod(s *operatoropts.MySQLOperatorOpts) func() time.Duration {


@owainlewis Pls review the change. Thanks.

huanwei · 2018-07-24T17:55:22Z

cmd/mysql-operator/app/mysql_operator.go

 	return func() time.Duration {
 		factor := rand.Float64() + 1
 		return time.Duration(float64(s.MinResyncPeriod.Nanoseconds()) * factor)
 	}
 }

 // Run starts the mysql-operator controllers. This should never exit.
-func Run(s *options.MySQLOperatorServer) error {
+func Run(s *operatoropts.MySQLOperatorOpts) error {


@owainlewis Pls review the change. Thanks.

Resolved

Code refactor on options (oracle#189)

huanwei added 3 commits July 21, 2018 08:29

tiny refactor code structure

b076468

Signed-off-by: huanwei <huan@harmonycloud.cn>

tiny refactor code structure

80a01a9

Signed-off-by: huanwei <huan@harmonycloud.cn>

tiny refactor code structure

3324a89

Signed-off-by: huanwei <huan@harmonycloud.cn>

huanwei closed this Jul 21, 2018

huanwei reopened this Jul 21, 2018

refactor on options

c7e5e53

Signed-off-by: huanwei <huan@harmonycloud.cn>

owainlewis added the oracle-cla: yes Contributor has signed the Oracle Contributor Licence Agreement label Jul 23, 2018

owainlewis reviewed Jul 24, 2018

View reviewed changes

owainlewis requested a review from prydie July 24, 2018 14:15

owainlewis removed the request for review from prydie July 24, 2018 14:21

owainlewis previously requested changes Jul 24, 2018

View reviewed changes

huanwei added 3 commits July 25, 2018 01:01

fix typo operatoropts

a5f06fd

Signed-off-by: huanwei <huan@harmonycloud.cn>

Merge pull request #4 from oracle/master

9e6a658

Use --decode for cross OS compatibility (oracle#187)

fix conflicts

c0f6e82

Signed-off-by: huanwei <huan@harmonycloud.cn>

huanwei commented Jul 24, 2018

View reviewed changes

prydie approved these changes Jul 25, 2018

View reviewed changes

prydie merged commit 23c5eb3 into oracle:master Jul 25, 2018

huanwei added a commit to huanwei/mysql-operator that referenced this pull request Jul 25, 2018

Merge pull request #6 from oracle/master

3e10cbc

Code refactor on options (oracle#189)

huanwei deleted the code-refactor branch July 25, 2018 16:22

Code refactor on options #189

Code refactor on options #189

Uh oh!

Conversation

huanwei commented Jul 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

huanwei commented Jul 23, 2018

Uh oh!

delabassee commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

owainlewis commented Jul 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

owainlewis left a comment

Choose a reason for hiding this comment

Uh oh!

huanwei left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huanwei left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

huanwei commented Jul 21, 2018 •

edited

Loading

delabassee commented Jul 23, 2018 •

edited

Loading

owainlewis commented Jul 24, 2018 •

edited

Loading