chore: refactor knuu object to use options as argument (#436)

* chore: refactor knuu object to use options as argument * Update pkg/knuu/knuu.go Co-authored-by: Matthew Sevey <[email protected]> * chore: style space --------- Co-authored-by: Matthew Sevey <[email protected]>
celestiaorg · Jun 13, 2024 · df4999f · df4999f
1 parent 5b65065
commit df4999f
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 125 deletions.
diff --git a/e2e/bittwister/suite_setup_test.go b/e2e/bittwister/suite_setup_test.go
@@ -20,7 +20,7 @@ func (s *Suite) SetupSuite() {
 		err error
 		ctx = context.Background()
 	)
-	s.Knuu, err = knuu.New(ctx, knuu.WithProxyEnabled())
+	s.Knuu, err = knuu.New(ctx, knuu.Options{ProxyEnabled: true})
 	s.Require().NoError(err)
 	s.T().Logf("Scope: %s", s.Knuu.Scope())
 	s.Knuu.HandleStopSignal(ctx)

diff --git a/e2e/system/build_from_git_test.go b/e2e/system/build_from_git_test.go
@@ -18,7 +18,7 @@ func TestBuildFromGit(t *testing.T) {
 	ctx := context.Background()
 
 	// The default image builder is kaniko here
-	kn, err := knuu.New(ctx)
+	kn, err := knuu.New(ctx, knuu.Options{})
 	require.NoError(t, err, "Error creating knuu")
 
 	sampleInstance, err := kn.NewInstance("git-builder")

diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go
@@ -35,125 +35,48 @@ const (
 
 type Knuu struct {
 	system.SystemDependencies
-	timeout      time.Duration
-	proxyEnabled bool
+	timeout time.Duration
 }
 
-type Option func(*Knuu)
-
-func WithImageBuilder(builder builder.Builder) Option {
-	return func(k *Knuu) {
-		k.ImageBuilder = builder
-	}
-}
-
-func WithTestScope(scope string) Option {
-	return func(k *Knuu) {
-		k.TestScope = k8s.SanitizeName(scope)
-	}
-}
-
-// This timeout indicates how long the test will run before it is considered failed.
-func WithTimeout(timeout time.Duration) Option {
-	return func(k *Knuu) {
-		k.timeout = timeout
-	}
-}
-
-func WithMinio(minio *minio.Minio) Option {
-	return func(k *Knuu) {
-		k.MinioCli = minio
-	}
-}
-
-func WithK8s(k8s k8s.KubeManager) Option {
-	return func(k *Knuu) {
-		k.K8sCli = k8s
-	}
+type Options struct {
+	K8s          k8s.KubeManager
+	TestScope    string
+	ImageBuilder builder.Builder
+	Minio        *minio.Minio
+	Timeout      time.Duration
+	ProxyEnabled bool
+	Logger       *logrus.Logger
 }
 
-func WithLogger(logger *logrus.Logger) Option {
-	return func(k *Knuu) {
-		k.Logger = logger
-	}
-}
-
-func WithProxyEnabled() Option {
-	return func(k *Knuu) {
-		k.proxyEnabled = true
-	}
-}
-
-func New(ctx context.Context, opts ...Option) (*Knuu, error) {
-	if err := godotenv.Load(); err != nil {
-		if !os.IsNotExist(err) {
-			return nil, ErrCannotLoadEnv.Wrap(err)
-		}
-		logrus.Info("The .env file does not exist, continuing without loading environment variables.")
+func New(ctx context.Context, opts Options) (*Knuu, error) {
+	if err := loadEnvVariables(); err != nil {
+		return nil, err
 	}
 
-	k := &Knuu{}
-	for _, opt := range opts {
-		opt(k)
+	k := &Knuu{
+		SystemDependencies: system.SystemDependencies{
+			K8sCli:       opts.K8s,
+			MinioCli:     opts.Minio,
+			ImageBuilder: opts.ImageBuilder,
+			Logger:       opts.Logger,
+			TestScope:    opts.TestScope,
+			StartTime:    time.Now().UTC().Format(TimeFormat),
+		},
+		timeout: opts.Timeout,
 	}
 
-	k.StartTime = time.Now().UTC().Format(TimeFormat)
-
-	// handle default values
-	if k.Logger == nil {
-		k.Logger = log.DefaultLogger()
-	}
-
-	if k.TestScope == "" {
-		t := time.Now()
-		k.TestScope = fmt.Sprintf("%s-%03d", t.Format("20060102-150405"), t.Nanosecond()/1e6)
-	}
-
-	if k.timeout == 0 {
-		k.timeout = defaultTimeout
-	}
-
-	if k.K8sCli == nil {
-		var err error
-		k.K8sCli, err = k8s.New(ctx, k.TestScope)
-		if err != nil {
-			return nil, ErrCannotInitializeK8s.Wrap(err)
-		}
-	}
-
-	if k.MinioCli == nil {
-		// TODO: minio also needs a little refactor to accept k8s obj instead
-		k.MinioCli = &minio.Minio{
-			Clientset: k.K8sCli.Clientset(),
-			Namespace: k.K8sCli.Namespace(),
-		}
-	}
-
-	if k.ImageBuilder == nil {
-		// TODO: Also here for kaniko
-		k.ImageBuilder = &kaniko.Kaniko{
-			K8sClientset: k.K8sCli.Clientset(),
-			K8sNamespace: k.K8sCli.Namespace(),
-			Minio:        k.MinioCli,
-		}
+	if err := setDefaults(ctx, k); err != nil {
+		return nil, err
 	}
 
-	if k.proxyEnabled {
-		k.Proxy = &traefik.Traefik{
-			K8s: k.K8sCli,
-		}
-		if err := k.Proxy.Deploy(ctx); err != nil {
-			return nil, ErrCannotDeployTraefik.Wrap(err)
-		}
-		endpoint, err := k.Proxy.Endpoint(ctx)
-		if err != nil {
-			return nil, ErrCannotGetTraefikEndpoint.Wrap(err)
+	if opts.ProxyEnabled {
+		if err := setupProxy(ctx, k); err != nil {
+			return nil, err
 		}
-		k.Logger.Debugf("Proxy endpoint: %s", endpoint)
 	}
 
 	if err := k.handleTimeout(ctx); err != nil {
-		return nil, ErrCannotHandleTimeout.Wrap(err)
+		return nil, err
 	}
 
 	return k, nil
@@ -236,3 +159,69 @@ func (k *Knuu) handleTimeout(ctx context.Context) error {
 
 	return nil
 }
+
+func loadEnvVariables() error {
+	err := godotenv.Load()
+	if err != nil && !os.IsNotExist(err) {
+		return ErrCannotLoadEnv.Wrap(err)
+	}
+	if os.IsNotExist(err) {
+		logrus.Info("The .env file does not exist, continuing without loading environment variables.")
+	}
+	return nil
+}
+
+func setDefaults(ctx context.Context, k *Knuu) error {
+	if k.Logger == nil {
+		k.Logger = log.DefaultLogger()
+	}
+
+	if k.TestScope == "" {
+		t := time.Now()
+		k.TestScope = fmt.Sprintf("%s-%03d", t.Format("20060102-150405"), t.Nanosecond()/1e6)
+	}
+
+	if k.timeout == 0 {
+		k.timeout = defaultTimeout
+	}
+
+	if k.K8sCli == nil {
+		var err error
+		k.K8sCli, err = k8s.New(ctx, k.TestScope)
+		if err != nil {
+			return ErrCannotInitializeK8s.Wrap(err)
+		}
+	}
+
+	if k.MinioCli == nil {
+		k.MinioCli = &minio.Minio{
+			Clientset: k.K8sCli.Clientset(),
+			Namespace: k.K8sCli.Namespace(),
+		}
+	}
+
+	if k.ImageBuilder == nil {
+		k.ImageBuilder = &kaniko.Kaniko{
+			K8sClientset: k.K8sCli.Clientset(),
+			K8sNamespace: k.K8sCli.Namespace(),
+			Minio:        k.MinioCli,
+		}
+	}
+
+	return nil
+}
+
+func setupProxy(ctx context.Context, k *Knuu) error {
+	k.Proxy = &traefik.Traefik{
+		K8s: k.K8sCli,
+	}
+	if err := k.Proxy.Deploy(ctx); err != nil {
+		return ErrCannotDeployTraefik.Wrap(err)
+	}
+	endpoint, err := k.Proxy.Endpoint(ctx)
+	if err != nil {
+		return ErrCannotGetTraefikEndpoint.Wrap(err)
+	}
+	k.Logger.Debugf("Proxy endpoint: %s", endpoint)
+	return nil
+}
diff --git a/pkg/knuu/knuu_old.go b/pkg/knuu/knuu_old.go
@@ -69,11 +69,11 @@ func InitializeWithScope(testScope string) error {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
 	defer cancel()
 	var err error
-	tmpKnuu, err = New(ctx,
-		WithTestScope(testScope),
-		WithTimeout(timeout),
-		WithProxyEnabled(),
-	)
+	tmpKnuu, err = New(ctx, Options{
+		TestScope:    testScope,
+		Timeout:      timeout,
+		ProxyEnabled: true,
+	})
 	if err != nil {
 		return ErrCannotInitializeKnuu.Wrap(err)
 	}

diff --git a/pkg/knuu/knuu_test.go b/pkg/knuu/knuu_test.go
@@ -60,13 +60,13 @@ func TestNew(t *testing.T) {
 
 	tt := []struct {
 		name         string
-		options      []Option
+		options      Options
 		expectError  bool
 		validateFunc func(*testing.T, *Knuu)
 	}{
 		{
 			name:        "Default initialization",
-			options:     nil,
+			options:     Options{},
 			expectError: false,
 			validateFunc: func(t *testing.T, k *Knuu) {
 				assert.NotNil(t, k)
@@ -79,8 +79,8 @@ func TestNew(t *testing.T) {
 		},
 		{
 			name: "With custom Logger",
-			options: []Option{
-				WithLogger(&logrus.Logger{}),
+			options: Options{
+				Logger: &logrus.Logger{},
 			},
 			expectError: false,
 			validateFunc: func(t *testing.T, k *Knuu) {
@@ -90,8 +90,8 @@ func TestNew(t *testing.T) {
 		},
 		{
 			name: "With custom Timeout",
-			options: []Option{
-				WithTimeout(30 * time.Minute),
+			options: Options{
+				Timeout: 30 * time.Minute,
 			},
 			expectError: false,
 			validateFunc: func(t *testing.T, k *Knuu) {
@@ -101,8 +101,8 @@ func TestNew(t *testing.T) {
 		},
 		{
 			name: "With custom K8s client",
-			options: []Option{
-				WithK8s(&mockK8s{}),
+			options: Options{
+				K8s: &mockK8s{},
 			},
 			expectError: false,
 			validateFunc: func(t *testing.T, k *Knuu) {
@@ -112,8 +112,8 @@ func TestNew(t *testing.T) {
 		},
 		{
 			name: "With custom Minio client",
-			options: []Option{
-				WithMinio(&minio.Minio{}),
+			options: Options{
+				Minio: &minio.Minio{},
 			},
 			expectError: false,
 			validateFunc: func(t *testing.T, k *Knuu) {
@@ -123,8 +123,8 @@ func TestNew(t *testing.T) {
 		},
 		{
 			name: "With custom Image Builder",
-			options: []Option{
-				WithImageBuilder(&kaniko.Kaniko{}),
+			options: Options{
+				ImageBuilder: &kaniko.Kaniko{},
 			},
 			expectError: false,
 			validateFunc: func(t *testing.T, k *Knuu) {
@@ -136,7 +136,7 @@ func TestNew(t *testing.T) {
 
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
-			k, err := New(ctx, tc.options...)
+			k, err := New(ctx, tc.options)
 			if tc.expectError {
 				assert.Error(t, err)
 				return