options: expose command line options structs

This allows to use local state in unit tests instead relying on global value.
scylladb · Feb 3, 2021 · f5ed310 · f5ed310
1 parent 4520f59
commit f5ed310
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 28 deletions.
diff --git a/pkg/cmd/options/common.go b/pkg/cmd/options/common.go
@@ -11,19 +11,19 @@ import (
 )
 
 // Singleton
-var commonOpts = &commonOptions{}
+var commonOpts = &CommonOptions{}
 
-type commonOptions struct {
+type CommonOptions struct {
 	Name      string
 	Namespace string
 	LogLevel  string
 }
 
-func GetCommonOptions() *commonOptions {
+func GetCommonOptions() *CommonOptions {
 	return commonOpts
 }
 
-func (o *commonOptions) AddFlags(cmd *cobra.Command) {
+func (o *CommonOptions) AddFlags(cmd *cobra.Command) {
 	cmd.Flags().StringVar(&o.Name, "pod-name", os.Getenv(naming.EnvVarEnvVarPodName), "name of the pod")
 	cmd.Flags().StringVar(&o.Namespace, "pod-namespace", os.Getenv(naming.EnvVarPodNamespace), "namespace of the pod")
 	cmd.Flags().StringVar(&o.LogLevel,
@@ -34,7 +34,7 @@ func (o *commonOptions) AddFlags(cmd *cobra.Command) {
 
 }
 
-func (o *commonOptions) Validate() error {
+func (o *CommonOptions) Validate() error {
 	if o.Name == "" {
 		return errors.New("pod-name not set")
 	}

diff --git a/pkg/cmd/options/manager.go b/pkg/cmd/options/manager.go
@@ -6,19 +6,19 @@ import (
 
 // Singleton
 var managerOpts = &managerOptions{
-	commonOptions: GetCommonOptions(),
+	CommonOptions: GetCommonOptions(),
 }
 
 type managerOptions struct {
-	*commonOptions
+	*CommonOptions
 }
 
 func GetManagerOptions() *managerOptions {
 	return managerOpts
 }
 
 func (o *managerOptions) AddFlags(cmd *cobra.Command) {
-	o.commonOptions.AddFlags(cmd)
+	o.CommonOptions.AddFlags(cmd)
 }
 
 func (o *managerOptions) Validate() error {

diff --git a/pkg/cmd/options/operator.go b/pkg/cmd/options/operator.go
@@ -6,29 +6,29 @@ import (
 )
 
 // Singleton
-var operatorOpts = &operatorOptions{
-	commonOptions: GetCommonOptions(),
+var operatorOpts = &OperatorOptions{
+	CommonOptions: GetCommonOptions(),
 }
 
-type operatorOptions struct {
-	*commonOptions
+type OperatorOptions struct {
+	*CommonOptions
 	Image                  string
 	EnableAdmissionWebhook bool
 }
 
-func GetOperatorOptions() *operatorOptions {
+func GetOperatorOptions() *OperatorOptions {
 	return operatorOpts
 }
 
-func (o *operatorOptions) AddFlags(cmd *cobra.Command) {
-	o.commonOptions.AddFlags(cmd)
+func (o *OperatorOptions) AddFlags(cmd *cobra.Command) {
+	o.CommonOptions.AddFlags(cmd)
 	cmd.Flags().StringVar(&o.Image, "image", "", "image of the operator used")
 	cmd.Flags().BoolVar(&o.EnableAdmissionWebhook, "enable-admission-webhook", true, "enable the admission webhook")
 }
 
-func (o *operatorOptions) Validate() error {
+func (o *OperatorOptions) Validate() error {
 
-	if o.Image == "" && o.commonOptions.Validate() != nil {
+	if o.Image == "" && o.CommonOptions.Validate() != nil {
 		return errors.New("image not set - you must set either image or namespace and name")
 	}
 	return nil

diff --git a/pkg/cmd/options/sidecar.go b/pkg/cmd/options/sidecar.go
@@ -10,11 +10,11 @@ import (
 
 // Singleton
 var sidecarOpts = &sidecarOptions{
-	commonOptions: GetCommonOptions(),
+	CommonOptions: GetCommonOptions(),
 }
 
 type sidecarOptions struct {
-	*commonOptions
+	*CommonOptions
 	CPU string
 }
 
@@ -23,12 +23,12 @@ func GetSidecarOptions() *sidecarOptions {
 }
 
 func (o *sidecarOptions) AddFlags(cmd *cobra.Command) {
-	o.commonOptions.AddFlags(cmd)
+	o.CommonOptions.AddFlags(cmd)
 	cmd.Flags().StringVar(&o.CPU, "cpu", os.Getenv(naming.EnvVarCPU), "number of cpus to use")
 }
 
 func (o *sidecarOptions) Validate() error {
-	if err := o.commonOptions.Validate(); err != nil {
+	if err := o.CommonOptions.Validate(); err != nil {
 		return errors.WithStack(err)
 	}
 	if o.CPU == "" {

diff --git a/pkg/controllers/cluster/cluster_controller.go b/pkg/controllers/cluster/cluster_controller.go
@@ -73,7 +73,7 @@ func New(ctx context.Context, mgr ctrl.Manager, logger log.Logger) (*ClusterReco
 		return nil, errors.Wrap(err, "get dynamic uncached client")
 	}
 
-	operatorImage, err := getOperatorImage(ctx, kubeClient)
+	operatorImage, err := getOperatorImage(ctx, kubeClient, options.GetOperatorOptions())
 	if err != nil {
 		return nil, errors.Wrap(err, "get operator image")
 	}
@@ -211,8 +211,7 @@ func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return nil
 }
 
-func getOperatorImage(ctx context.Context, kubeClient kubernetes.Interface) (string, error) {
-	opts := options.GetOperatorOptions()
+func getOperatorImage(ctx context.Context, kubeClient kubernetes.Interface, opts *options.OperatorOptions) (string, error) {
 	if opts.Image != "" {
 		return opts.Image, nil
 	}

diff --git a/pkg/controllers/cluster/cluster_controller_test.go b/pkg/controllers/cluster/cluster_controller_test.go
@@ -21,9 +21,12 @@ var _ = Describe("Cluster controller", func() {
 			namespace = "ns"
 			name      = "name"
 		)
-		opts := options.GetOperatorOptions()
-		opts.Namespace = namespace
-		opts.Name = name
+		opts := &options.OperatorOptions{
+			CommonOptions: &options.CommonOptions{
+				Name:      name,
+				Namespace: namespace,
+			},
+		}
 
 		pod := &corev1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
@@ -38,7 +41,7 @@ var _ = Describe("Cluster controller", func() {
 		}
 
 		kubeClientFake := kubefake.NewSimpleClientset(pod)
-		image, err := getOperatorImage(context.Background(), kubeClientFake)
+		image, err := getOperatorImage(context.Background(), kubeClientFake, opts)
 		if expectedImage == "" {
 			Expect(err).To(HaveOccurred())
 		} else {