From b442dfa26632a7f82a47e9c1bf8b4a3f74d7c6d2 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Mon, 10 Jun 2024 12:36:57 +0200 Subject: [PATCH 01/14] chore: optimize minio cli to be reused --- pkg/minio/minio.go | 65 +++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/pkg/minio/minio.go b/pkg/minio/minio.go index 3f5a829f..46d65613 100644 --- a/pkg/minio/minio.go +++ b/pkg/minio/minio.go @@ -44,6 +44,7 @@ const ( type Minio struct { Clientset kubernetes.Interface Namespace string + cli *miniogo.Client } func (m *Minio) DeployMinio(ctx context.Context) error { @@ -160,14 +161,17 @@ func (m *Minio) IsMinioDeployed(ctx context.Context) (bool, error) { return true, nil } -// PushToMinio pushes data (i.e. a reader) to Minio -func (m *Minio) PushToMinio(ctx context.Context, localReader io.Reader, minioFilePath, bucketName string) error { +func (m *Minio) init(ctx context.Context) error { + if m.cli != nil { + return nil + } + endpoint, err := m.getEndpoint(ctx) if err != nil { return ErrMinioFailedToGetEndpoint.Wrap(err) } - cli, err := miniogo.New(endpoint, &miniogo.Options{ + m.cli, err = miniogo.New(endpoint, &miniogo.Options{ Creds: credentials.NewStaticV4(rootUser, rootPassword, ""), Secure: false, }) @@ -175,11 +179,20 @@ func (m *Minio) PushToMinio(ctx context.Context, localReader io.Reader, minioFil return ErrMinioFailedToInitializeClient.Wrap(err) } - if err := m.createBucketIfNotExists(ctx, cli, bucketName); err != nil { + return nil +} + +// PushToMinio pushes data (i.e. a reader) to Minio +func (m *Minio) PushToMinio(ctx context.Context, localReader io.Reader, minioFilePath, bucketName string) error { + if err := m.init(ctx); err != nil { + return err + } + + if err := m.createBucketIfNotExists(ctx, bucketName); err != nil { return ErrMinioFailedToCreateBucket.Wrap(err) } - uploadInfo, err := cli.PutObject(ctx, bucketName, minioFilePath, localReader, -1, miniogo.PutObjectOptions{}) + uploadInfo, err := m.cli.PutObject(ctx, bucketName, minioFilePath, localReader, -1, miniogo.PutObjectOptions{}) if err != nil { return ErrMinioFailedToUploadData.Wrap(err) } @@ -190,26 +203,17 @@ func (m *Minio) PushToMinio(ctx context.Context, localReader io.Reader, minioFil // DeleteFromMinio deletes a file from Minio and fails if the content does not exist func (m *Minio) DeleteFromMinio(ctx context.Context, minioFilePath, bucketName string) error { - endpoint, err := m.getEndpoint(ctx) - if err != nil { - return ErrMinioFailedToGetPresignedURL.Wrap(err) - } - - cli, err := miniogo.New(endpoint, &miniogo.Options{ - Creds: credentials.NewStaticV4(rootUser, rootPassword, ""), - Secure: false, - }) - if err != nil { - return ErrMinioFailedToUpdateService.Wrap(err) + if err := m.init(ctx); err != nil { + return err } // Check if the object exists before attempting to delete - _, err = cli.StatObject(ctx, bucketName, minioFilePath, miniogo.StatObjectOptions{}) + _, err := m.cli.StatObject(ctx, bucketName, minioFilePath, miniogo.StatObjectOptions{}) if err != nil { return ErrMinioFailedToFindFileBeforeDeletion.Wrap(err) } - err = cli.RemoveObject(ctx, bucketName, minioFilePath, miniogo.RemoveObjectOptions{}) + err = m.cli.RemoveObject(ctx, bucketName, minioFilePath, miniogo.RemoveObjectOptions{}) if err != nil { return ErrMinioFailedToDeleteFile.Wrap(err) } @@ -220,24 +224,15 @@ func (m *Minio) DeleteFromMinio(ctx context.Context, minioFilePath, bucketName s // GetMinioURL returns an S3-compatible URL for a Minio file func (m *Minio) GetMinioURL(ctx context.Context, minioFilePath, bucketName string) (string, error) { - minioEndpoint, err := m.getEndpoint(ctx) - if err != nil { - return "", ErrMinioFailedToGetMinioEndpoint.Wrap(err) - } - // Initialize Minio client - minioClient, err := miniogo.New(minioEndpoint, &miniogo.Options{ - Creds: credentials.NewStaticV4(rootUser, rootPassword, ""), - Secure: false, - }) - if err != nil { - return "", ErrMinioFailedToInitializeClient.Wrap(err) + if err := m.init(ctx); err != nil { + return "", err } // Set the expiration time for the URL (e.g., 24h from now) expiration := 24 * time.Hour // Generate a presigned URL for the object - presignedURL, err := minioClient.PresignedGetObject(ctx, bucketName, minioFilePath, expiration, nil) + presignedURL, err := m.cli.PresignedGetObject(ctx, bucketName, minioFilePath, expiration, nil) if err != nil { return "", ErrMinioFailedToGeneratePresignedURL.Wrap(err) } @@ -295,8 +290,12 @@ func (m *Minio) createOrUpdateService(ctx context.Context) error { return nil } -func (m *Minio) createBucketIfNotExists(ctx context.Context, cli *miniogo.Client, bucketName string) error { - exists, err := cli.BucketExists(ctx, bucketName) +func (m *Minio) createBucketIfNotExists(ctx context.Context, bucketName string) error { + if err := m.init(ctx); err != nil { + return err + } + + exists, err := m.cli.BucketExists(ctx, bucketName) if err != nil { return ErrMinioFailedToCheckBucket.Wrap(err) } @@ -304,7 +303,7 @@ func (m *Minio) createBucketIfNotExists(ctx context.Context, cli *miniogo.Client return nil } - if err := cli.MakeBucket(ctx, bucketName, miniogo.MakeBucketOptions{}); err != nil { + if err := m.cli.MakeBucket(ctx, bucketName, miniogo.MakeBucketOptions{}); err != nil { return ErrMinioFailedToCreateBucket.Wrap(err) } logrus.Debugf("Bucket `%s` created successfully.", bucketName) From e14dc59be323491774190e2c0707b5467d82631d Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Fri, 14 Jun 2024 16:56:10 +0200 Subject: [PATCH 02/14] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jose Ramon Mañes <32740567+jrmanes@users.noreply.github.com> --- pkg/minio/minio.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/minio/minio.go b/pkg/minio/minio.go index 46d65613..8cc2d8da 100644 --- a/pkg/minio/minio.go +++ b/pkg/minio/minio.go @@ -44,7 +44,7 @@ const ( type Minio struct { Clientset kubernetes.Interface Namespace string - cli *miniogo.Client + client *miniogo.Client } func (m *Minio) DeployMinio(ctx context.Context) error { @@ -162,7 +162,7 @@ func (m *Minio) IsMinioDeployed(ctx context.Context) (bool, error) { } func (m *Minio) init(ctx context.Context) error { - if m.cli != nil { + if m.client != nil { return nil } @@ -171,7 +171,7 @@ func (m *Minio) init(ctx context.Context) error { return ErrMinioFailedToGetEndpoint.Wrap(err) } - m.cli, err = miniogo.New(endpoint, &miniogo.Options{ + m.client, err = miniogo.New(endpoint, &miniogo.Options{ Creds: credentials.NewStaticV4(rootUser, rootPassword, ""), Secure: false, }) @@ -192,7 +192,7 @@ func (m *Minio) PushToMinio(ctx context.Context, localReader io.Reader, minioFil return ErrMinioFailedToCreateBucket.Wrap(err) } - uploadInfo, err := m.cli.PutObject(ctx, bucketName, minioFilePath, localReader, -1, miniogo.PutObjectOptions{}) + uploadInfo, err := m.client.PutObject(ctx, bucketName, minioFilePath, localReader, -1, miniogo.PutObjectOptions{}) if err != nil { return ErrMinioFailedToUploadData.Wrap(err) } @@ -208,12 +208,12 @@ func (m *Minio) DeleteFromMinio(ctx context.Context, minioFilePath, bucketName s } // Check if the object exists before attempting to delete - _, err := m.cli.StatObject(ctx, bucketName, minioFilePath, miniogo.StatObjectOptions{}) + _, err := m.client.StatObject(ctx, bucketName, minioFilePath, miniogo.StatObjectOptions{}) if err != nil { return ErrMinioFailedToFindFileBeforeDeletion.Wrap(err) } - err = m.cli.RemoveObject(ctx, bucketName, minioFilePath, miniogo.RemoveObjectOptions{}) + err = m.client.RemoveObject(ctx, bucketName, minioFilePath, miniogo.RemoveObjectOptions{}) if err != nil { return ErrMinioFailedToDeleteFile.Wrap(err) } @@ -232,7 +232,7 @@ func (m *Minio) GetMinioURL(ctx context.Context, minioFilePath, bucketName strin expiration := 24 * time.Hour // Generate a presigned URL for the object - presignedURL, err := m.cli.PresignedGetObject(ctx, bucketName, minioFilePath, expiration, nil) + presignedURL, err := m.client.PresignedGetObject(ctx, bucketName, minioFilePath, expiration, nil) if err != nil { return "", ErrMinioFailedToGeneratePresignedURL.Wrap(err) } @@ -295,7 +295,7 @@ func (m *Minio) createBucketIfNotExists(ctx context.Context, bucketName string) return err } - exists, err := m.cli.BucketExists(ctx, bucketName) + exists, err := m.client.BucketExists(ctx, bucketName) if err != nil { return ErrMinioFailedToCheckBucket.Wrap(err) } @@ -303,7 +303,7 @@ func (m *Minio) createBucketIfNotExists(ctx context.Context, bucketName string) return nil } - if err := m.cli.MakeBucket(ctx, bucketName, miniogo.MakeBucketOptions{}); err != nil { + if err := m.client.MakeBucket(ctx, bucketName, miniogo.MakeBucketOptions{}); err != nil { return ErrMinioFailedToCreateBucket.Wrap(err) } logrus.Debugf("Bucket `%s` created successfully.", bucketName) From b2ed2f8be63ff6df496291794ca353e8ea678f5a Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Mon, 17 Jun 2024 17:35:47 +0200 Subject: [PATCH 03/14] fix: add a New func to the Minio pkh --- e2e/tshark/tshark_test.go | 3 +- pkg/builder/kaniko/errors.go | 1 + pkg/builder/kaniko/kaniko.go | 43 ++++--- pkg/builder/kaniko/kaniko_test.go | 2 +- pkg/knuu/knuu.go | 10 +- pkg/knuu/knuu_old.go | 4 +- pkg/knuu/minio.go | 20 +-- pkg/minio/errors.go | 1 + pkg/minio/minio.go | 206 ++++++++++++++++-------------- 9 files changed, 155 insertions(+), 135 deletions(-) diff --git a/e2e/tshark/tshark_test.go b/e2e/tshark/tshark_test.go index 8b0709a9..2ca9c961 100644 --- a/e2e/tshark/tshark_test.go +++ b/e2e/tshark/tshark_test.go @@ -13,6 +13,7 @@ import ( "github.com/celestiaorg/knuu/pkg/instance" "github.com/celestiaorg/knuu/pkg/knuu" + "github.com/celestiaorg/knuu/pkg/minio" ) const ( @@ -48,7 +49,7 @@ func TestTshark(t *testing.T) { require.NoError(t, err, "error setting command") t.Log("deploying minio as s3 backend") - err = kn.MinioClient.DeployMinio(ctx) + kn.MinioClient, err = minio.New(ctx, kn.K8sClient) require.NoError(t, err, "error deploying minio") t.Log("getting minio configs") diff --git a/pkg/builder/kaniko/errors.go b/pkg/builder/kaniko/errors.go index f993ef1a..2505f5f1 100644 --- a/pkg/builder/kaniko/errors.go +++ b/pkg/builder/kaniko/errors.go @@ -30,4 +30,5 @@ var ( ErrMinioDeploymentFailed = errors.New("MinioDeploymentFailed", "Minio deployment failed") ErrDeletingMinioContent = errors.New("DeletingMinioContent", "error deleting Minio content") ErrParsingQuantity = errors.New("ParsingQuantity", "error parsing quantity") + ErrMinioFailedToGetDeployment = errors.New("MinioFailedToGetDeployment", "Minio failed to get deployment") ) diff --git a/pkg/builder/kaniko/kaniko.go b/pkg/builder/kaniko/kaniko.go index 721707c2..e4d5e6b0 100644 --- a/pkg/builder/kaniko/kaniko.go +++ b/pkg/builder/kaniko/kaniko.go @@ -31,8 +31,8 @@ const ( ) type Kaniko struct { - K8s k8s.KubeManager - Minio *minio.Minio // Minio service to store the build context if it's a directory + K8sClient k8s.KubeManager + MinioClient *minio.Minio // Minio service to store the build context if it's a directory ContentName string // Name of the content pushed to Minio } @@ -44,7 +44,7 @@ func (k *Kaniko) Build(ctx context.Context, b *builder.BuilderOptions) (logs str return "", ErrPreparingJob.Wrap(err) } - cJob, err := k.K8s.Clientset().BatchV1().Jobs(k.K8s.Namespace()).Create(ctx, job, metav1.CreateOptions{}) + cJob, err := k.K8sClient.Clientset().BatchV1().Jobs(k.K8sClient.Namespace()).Create(ctx, job, metav1.CreateOptions{}) if err != nil { return "", ErrCreatingJob.Wrap(err) } @@ -76,7 +76,7 @@ func (k *Kaniko) Build(ctx context.Context, b *builder.BuilderOptions) (logs str } func (k *Kaniko) waitForJobCompletion(ctx context.Context, job *batchv1.Job) (*batchv1.Job, error) { - watcher, err := k.K8s.Clientset().BatchV1().Jobs(k.K8s.Namespace()).Watch(ctx, metav1.ListOptions{ + watcher, err := k.K8sClient.Clientset().BatchV1().Jobs(k.K8sClient.Namespace()).Watch(ctx, metav1.ListOptions{ FieldSelector: fmt.Sprintf("metadata.name=%s", job.Name), }) if err != nil { @@ -107,7 +107,7 @@ func (k *Kaniko) waitForJobCompletion(ctx context.Context, job *batchv1.Job) (*b } func (k *Kaniko) firstPodFromJob(ctx context.Context, job *batchv1.Job) (*v1.Pod, error) { - podList, err := k.K8s.Clientset().CoreV1().Pods(k.K8s.Namespace()).List(ctx, metav1.ListOptions{ + podList, err := k.K8sClient.Clientset().CoreV1().Pods(k.K8sClient.Namespace()).List(ctx, metav1.ListOptions{ LabelSelector: fmt.Sprintf("job-name=%s", job.Name), }) if err != nil { @@ -130,7 +130,7 @@ func (k *Kaniko) containerLogs(ctx context.Context, pod *v1.Pod) (string, error) Container: pod.Spec.Containers[0].Name, } - req := k.K8s.Clientset().CoreV1().Pods(k.K8s.Namespace()).GetLogs(pod.Name, &logOptions) + req := k.K8sClient.Clientset().CoreV1().Pods(k.K8sClient.Namespace()).GetLogs(pod.Name, &logOptions) logs, err := req.DoRaw(ctx) if err != nil { return "", err @@ -140,7 +140,7 @@ func (k *Kaniko) containerLogs(ctx context.Context, pod *v1.Pod) (string, error) } func (k *Kaniko) cleanup(ctx context.Context, job *batchv1.Job) error { - err := k.K8s.Clientset().BatchV1().Jobs(k.K8s.Namespace()). + err := k.K8sClient.Clientset().BatchV1().Jobs(k.K8sClient.Namespace()). Delete(ctx, job.Name, metav1.DeleteOptions{ PropagationPolicy: &[]metav1.DeletionPropagation{metav1.DeletePropagationBackground}[0], }) @@ -149,7 +149,7 @@ func (k *Kaniko) cleanup(ctx context.Context, job *batchv1.Job) error { } // Delete the associated Pods - err = k.K8s.Clientset().CoreV1().Pods(k.K8s.Namespace()). + err = k.K8sClient.Clientset().CoreV1().Pods(k.K8sClient.Namespace()). DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{ LabelSelector: fmt.Sprintf("job-name=%s", job.Name), }) @@ -159,7 +159,10 @@ func (k *Kaniko) cleanup(ctx context.Context, job *batchv1.Job) error { // Delete the content pushed to Minio if k.ContentName != "" { - if err := k.Minio.DeleteFromMinio(ctx, k.ContentName, MinioBucketName); err != nil { + if err := k.initMinio(ctx); err != nil { + return ErrMinioFailedToGetDeployment.Wrap(err) + } + if err := k.MinioClient.DeleteFromMinio(ctx, k.ContentName, MinioBucketName); err != nil { return ErrDeletingMinioContent.Wrap(err) } } @@ -167,6 +170,16 @@ func (k *Kaniko) cleanup(ctx context.Context, job *batchv1.Job) error { return nil } +func (k *Kaniko) initMinio(ctx context.Context) error { + if k.MinioClient != nil { + return nil + } + + var err error + k.MinioClient, err = minio.New(ctx, k.K8sClient) + return err +} + func (k *Kaniko) prepareJob(ctx context.Context, b *builder.BuilderOptions) (*batchv1.Job, error) { jobName, err := names.NewRandomK8(kanikoJobNamePrefix) if err != nil { @@ -248,8 +261,8 @@ func (k *Kaniko) prepareJob(ctx context.Context, b *builder.BuilderOptions) (*ba // As kaniko also supports directly tar.gz archives, no need to extract it, // we just need to set the context to tar:// func (k *Kaniko) mountDir(ctx context.Context, bCtx string, job *batchv1.Job) (*batchv1.Job, error) { - if k.Minio == nil { - return nil, ErrMinioNotConfigured + if err := k.initMinio(ctx); err != nil { + return nil, ErrMinioFailedToGetDeployment.Wrap(err) } // Create the tar.gz archive @@ -263,15 +276,11 @@ func (k *Kaniko) mountDir(ctx context.Context, bCtx string, job *batchv1.Job) (* hash.Write(archiveData) k.ContentName = hex.EncodeToString(hash.Sum(nil)) - if err := k.Minio.DeployMinio(ctx); err != nil { - return nil, ErrMinioDeploymentFailed.Wrap(err) - } - - if err := k.Minio.PushToMinio(ctx, bytes.NewReader(archiveData), k.ContentName, MinioBucketName); err != nil { + if err := k.MinioClient.PushToMinio(ctx, bytes.NewReader(archiveData), k.ContentName, MinioBucketName); err != nil { return nil, err } - s3URL, err := k.Minio.GetMinioURL(ctx, k.ContentName, MinioBucketName) + s3URL, err := k.MinioClient.GetMinioURL(ctx, k.ContentName, MinioBucketName) if err != nil { return nil, err } diff --git a/pkg/builder/kaniko/kaniko_test.go b/pkg/builder/kaniko/kaniko_test.go index e138ca9b..17e63f62 100644 --- a/pkg/builder/kaniko/kaniko_test.go +++ b/pkg/builder/kaniko/kaniko_test.go @@ -28,7 +28,7 @@ func TestKanikoBuilder(t *testing.T) { k8sClient, err := k8s.NewClientCustom(context.Background(), k8sCS, k8sCS.Discovery(), nil, k8sNamespace) require.NoError(t, err) kb := &Kaniko{ - K8s: k8sClient, + K8sClient: k8sClient, } ctx := context.Background() diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go index 01c3a303..1a10e9a2 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -193,16 +193,10 @@ func setDefaults(ctx context.Context, k *Knuu) error { } } - if k.MinioClient == nil { - k.MinioClient = &minio.Minio{ - K8s: k.K8sClient, - } - } - if k.ImageBuilder == nil { k.ImageBuilder = &kaniko.Kaniko{ - K8s: k.K8sClient, - Minio: k.MinioClient, + K8sClient: k.K8sClient, + MinioClient: k.MinioClient, } } diff --git a/pkg/knuu/knuu_old.go b/pkg/knuu/knuu_old.go index f431fe61..cb3d62af 100644 --- a/pkg/knuu/knuu_old.go +++ b/pkg/knuu/knuu_old.go @@ -82,8 +82,8 @@ func InitializeWithScope(testScope string) error { switch builderType { case "kubernetes": tmpKnuu.ImageBuilder = &kaniko.Kaniko{ - K8s: tmpKnuu.K8sClient, - Minio: tmpKnuu.MinioClient, + K8sClient: tmpKnuu.K8sClient, + MinioClient: tmpKnuu.MinioClient, } case "docker", "": tmpKnuu.ImageBuilder = &docker.Docker{ diff --git a/pkg/knuu/minio.go b/pkg/knuu/minio.go index 49f4169d..f12da202 100644 --- a/pkg/knuu/minio.go +++ b/pkg/knuu/minio.go @@ -3,23 +3,23 @@ package knuu import ( "context" "io" + + "github.com/celestiaorg/knuu/pkg/minio" ) const minioBucketName = "knuu" +// initMinio initializes the Minio client +// Since not always we need minio to be deployed and ready, +// We deploy it on the first use. i.e. minio.New deploys it func (k *Knuu) initMinio(ctx context.Context) error { - if k.MinioClient == nil { - return ErrMinioNotInitialized - } - - ok, err := k.MinioClient.IsMinioDeployed(ctx) - if err != nil { - return err - } - if ok { + if k.MinioClient != nil { return nil } - return k.MinioClient.DeployMinio(ctx) + + var err error + k.MinioClient, err = minio.New(ctx, k.K8sClient) + return err } // contentName is a unique string to identify the content in Minio diff --git a/pkg/minio/errors.go b/pkg/minio/errors.go index cb4e7ad6..0dd896db 100644 --- a/pkg/minio/errors.go +++ b/pkg/minio/errors.go @@ -40,4 +40,5 @@ var ( ErrMinioFailedToListPersistentVolumes = errors.New("MinioFailedToListPersistentVolumes", "failed to list PersistentVolumes") ErrMinioFailedToCreatePersistentVolume = errors.New("MinioFailedToCreatePersistentVolume", "failed to create PersistentVolume") ErrMinioFailedToCreatePersistentVolumeClaim = errors.New("MinioFailedToCreatePersistentVolumeClaim", "failed to create PersistentVolumeClaim") + ErrMinioClientNotInitialized = errors.New("MinioClientNotInitialized", "Minio client not initialized") ) diff --git a/pkg/minio/minio.go b/pkg/minio/minio.go index facb7d5b..0aef6853 100644 --- a/pkg/minio/minio.go +++ b/pkg/minio/minio.go @@ -52,7 +52,113 @@ type Config struct { SecretAccessKey string } -func (m *Minio) DeployMinio(ctx context.Context) error { +func New(ctx context.Context, k8sClient k8s.KubeManager) (*Minio, error) { + m := &Minio{ + K8s: k8sClient, + } + + fmt.Println("minio.New is called") + + // [optimization] since it might be called from different places, + // we need to check if minio is already deployed + isMinioDeployed, err := m.isMinioDeployed(ctx) + if err != nil { + return nil, ErrMinioFailedToGetDeployment.Wrap(err) + } + if !isMinioDeployed { + if err := m.deployMinio(ctx); err != nil { + return nil, err + } + } + + endpoint, err := m.getEndpoint(ctx) + if err != nil { + return nil, ErrMinioFailedToGetEndpoint.Wrap(err) + } + + m.client, err = miniogo.New(endpoint, &miniogo.Options{ + Creds: credentials.NewStaticV4(rootUser, rootPassword, ""), + Secure: false, + }) + if err != nil { + return nil, ErrMinioFailedToInitializeClient.Wrap(err) + } + + return m, nil +} + +// PushToMinio pushes data (i.e. a reader) to Minio +func (m *Minio) PushToMinio(ctx context.Context, localReader io.Reader, minioFilePath, bucketName string) error { + if m.client == nil { + return ErrMinioClientNotInitialized + } + + if err := m.createBucketIfNotExists(ctx, bucketName); err != nil { + return ErrMinioFailedToCreateBucket.Wrap(err) + } + + uploadInfo, err := m.client.PutObject(ctx, bucketName, minioFilePath, localReader, -1, miniogo.PutObjectOptions{}) + if err != nil { + return ErrMinioFailedToUploadData.Wrap(err) + } + + logrus.Debugf("Data uploaded successfully to %s in bucket %s", uploadInfo.Key, bucketName) + return nil +} + +// DeleteFromMinio deletes a file from Minio and fails if the content does not exist +func (m *Minio) DeleteFromMinio(ctx context.Context, minioFilePath, bucketName string) error { + if m.client == nil { + return ErrMinioClientNotInitialized + } + + // Check if the object exists before attempting to delete + _, err := m.client.StatObject(ctx, bucketName, minioFilePath, miniogo.StatObjectOptions{}) + if err != nil { + return ErrMinioFailedToFindFileBeforeDeletion.Wrap(err) + } + + err = m.client.RemoveObject(ctx, bucketName, minioFilePath, miniogo.RemoveObjectOptions{}) + if err != nil { + return ErrMinioFailedToDeleteFile.Wrap(err) + } + + logrus.Debugf("File %s deleted successfully from bucket %s", minioFilePath, bucketName) + return nil +} + +// GetMinioURL returns an S3-compatible URL for a Minio file +func (m *Minio) GetMinioURL(ctx context.Context, minioFilePath, bucketName string) (string, error) { + if m.client == nil { + return "", ErrMinioClientNotInitialized + } + + // Set the expiration time for the URL (e.g., 24h from now) + expiration := 24 * time.Hour + + // Generate a presigned URL for the object + presignedURL, err := m.client.PresignedGetObject(ctx, bucketName, minioFilePath, expiration, nil) + if err != nil { + return "", ErrMinioFailedToGeneratePresignedURL.Wrap(err) + } + + return presignedURL.String(), nil +} + +func (m *Minio) GetConfigs(ctx context.Context) (*Config, error) { + endpoint, err := m.getEndpoint(ctx) + if err != nil { + return nil, ErrMinioFailedToGetEndpoint.Wrap(err) + } + + return &Config{ + Endpoint: endpoint, + AccessKeyID: rootUser, + SecretAccessKey: rootPassword, + }, nil +} + +func (m *Minio) deployMinio(ctx context.Context) error { if err := m.createOrUpdateDeployment(ctx); err != nil { return ErrMinioFailedToStart.Wrap(err) } @@ -152,7 +258,7 @@ func (m *Minio) createOrUpdateDeployment(ctx context.Context) error { return nil } -func (m *Minio) IsMinioDeployed(ctx context.Context) (bool, error) { +func (m *Minio) isMinioDeployed(ctx context.Context) (bool, error) { deploymentClient := m.K8s.Clientset().AppsV1().Deployments(m.K8s.Namespace()) _, err := deploymentClient.Get(ctx, DeploymentName, metav1.GetOptions{}) @@ -166,98 +272,6 @@ func (m *Minio) IsMinioDeployed(ctx context.Context) (bool, error) { return true, nil } -func (m *Minio) init(ctx context.Context) error { - if m.client != nil { - return nil - } - - endpoint, err := m.getEndpoint(ctx) - if err != nil { - return ErrMinioFailedToGetEndpoint.Wrap(err) - } - - m.client, err = miniogo.New(endpoint, &miniogo.Options{ - Creds: credentials.NewStaticV4(rootUser, rootPassword, ""), - Secure: false, - }) - if err != nil { - return ErrMinioFailedToInitializeClient.Wrap(err) - } - - return nil -} - -// PushToMinio pushes data (i.e. a reader) to Minio -func (m *Minio) PushToMinio(ctx context.Context, localReader io.Reader, minioFilePath, bucketName string) error { - if err := m.init(ctx); err != nil { - return err - } - - if err := m.createBucketIfNotExists(ctx, bucketName); err != nil { - return ErrMinioFailedToCreateBucket.Wrap(err) - } - - uploadInfo, err := m.client.PutObject(ctx, bucketName, minioFilePath, localReader, -1, miniogo.PutObjectOptions{}) - if err != nil { - return ErrMinioFailedToUploadData.Wrap(err) - } - - logrus.Debugf("Data uploaded successfully to %s in bucket %s", uploadInfo.Key, bucketName) - return nil -} - -// DeleteFromMinio deletes a file from Minio and fails if the content does not exist -func (m *Minio) DeleteFromMinio(ctx context.Context, minioFilePath, bucketName string) error { - if err := m.init(ctx); err != nil { - return err - } - - // Check if the object exists before attempting to delete - _, err := m.client.StatObject(ctx, bucketName, minioFilePath, miniogo.StatObjectOptions{}) - if err != nil { - return ErrMinioFailedToFindFileBeforeDeletion.Wrap(err) - } - - err = m.client.RemoveObject(ctx, bucketName, minioFilePath, miniogo.RemoveObjectOptions{}) - if err != nil { - return ErrMinioFailedToDeleteFile.Wrap(err) - } - - logrus.Debugf("File %s deleted successfully from bucket %s", minioFilePath, bucketName) - return nil -} - -// GetMinioURL returns an S3-compatible URL for a Minio file -func (m *Minio) GetMinioURL(ctx context.Context, minioFilePath, bucketName string) (string, error) { - if err := m.init(ctx); err != nil { - return "", err - } - - // Set the expiration time for the URL (e.g., 24h from now) - expiration := 24 * time.Hour - - // Generate a presigned URL for the object - presignedURL, err := m.client.PresignedGetObject(ctx, bucketName, minioFilePath, expiration, nil) - if err != nil { - return "", ErrMinioFailedToGeneratePresignedURL.Wrap(err) - } - - return presignedURL.String(), nil -} - -func (m *Minio) GetConfigs(ctx context.Context) (*Config, error) { - endpoint, err := m.getEndpoint(ctx) - if err != nil { - return nil, ErrMinioFailedToGetEndpoint.Wrap(err) - } - - return &Config{ - Endpoint: endpoint, - AccessKeyID: rootUser, - SecretAccessKey: rootPassword, - }, nil -} - func (m *Minio) createOrUpdateService(ctx context.Context) error { serviceClient := m.K8s.Clientset().CoreV1().Services(m.K8s.Namespace()) @@ -309,8 +323,8 @@ func (m *Minio) createOrUpdateService(ctx context.Context) error { } func (m *Minio) createBucketIfNotExists(ctx context.Context, bucketName string) error { - if err := m.init(ctx); err != nil { - return err + if m.client == nil { + return ErrMinioClientNotInitialized } exists, err := m.client.BucketExists(ctx, bucketName) From efef1a6cc961d062e69882935cc34d0b53065050 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Tue, 18 Jun 2024 09:45:18 +0200 Subject: [PATCH 04/14] fix: remove the extra println --- pkg/minio/minio.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/minio/minio.go b/pkg/minio/minio.go index 0aef6853..0f749325 100644 --- a/pkg/minio/minio.go +++ b/pkg/minio/minio.go @@ -57,8 +57,6 @@ func New(ctx context.Context, k8sClient k8s.KubeManager) (*Minio, error) { K8s: k8sClient, } - fmt.Println("minio.New is called") - // [optimization] since it might be called from different places, // we need to check if minio is already deployed isMinioDeployed, err := m.isMinioDeployed(ctx) From b214e856d112c38093e4237b890ab69212663f5b Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Tue, 18 Jun 2024 17:21:26 +0200 Subject: [PATCH 05/14] chore: refactor minio to be loaded as an option --- e2e/tshark/tshark_test.go | 4 +- pkg/builder/kaniko/kaniko.go | 31 +++--------- pkg/builder/kaniko/kaniko_test.go | 5 +- pkg/knuu/knuu.go | 19 +++++-- pkg/knuu/knuu_old.go | 10 ++-- pkg/knuu/knuu_test.go | 24 ++++----- pkg/knuu/minio.go | 38 -------------- pkg/minio/errors.go | 1 + pkg/minio/minio.go | 84 ++++++++++++------------------- 9 files changed, 78 insertions(+), 138 deletions(-) delete mode 100644 pkg/knuu/minio.go diff --git a/e2e/tshark/tshark_test.go b/e2e/tshark/tshark_test.go index 2ca9c961..e85baf79 100644 --- a/e2e/tshark/tshark_test.go +++ b/e2e/tshark/tshark_test.go @@ -27,7 +27,7 @@ func TestTshark(t *testing.T) { ctx := context.Background() - kn, err := knuu.New(ctx, knuu.Options{}) + kn, err := knuu.New(ctx, knuu.Options{MinioEnabled: true}) require.NoError(t, err, "error creating knuu") defer func() { @@ -99,7 +99,7 @@ func TestTshark(t *testing.T) { _, err = target.ExecuteCommand(ctx, "ping", "-c", "4", "google.com") require.NoError(t, err, "error executing command") - url, err := kn.MinioClient.GetMinioURL(ctx, fileKey, s3BucketName) + url, err := kn.MinioClient.GetURL(ctx, fileKey, s3BucketName) require.NoError(t, err, "error getting minio url") resp, err := http.Get(url) diff --git a/pkg/builder/kaniko/kaniko.go b/pkg/builder/kaniko/kaniko.go index e4d5e6b0..b2adaa17 100644 --- a/pkg/builder/kaniko/kaniko.go +++ b/pkg/builder/kaniko/kaniko.go @@ -13,9 +13,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/celestiaorg/knuu/pkg/builder" - "github.com/celestiaorg/knuu/pkg/k8s" - "github.com/celestiaorg/knuu/pkg/minio" "github.com/celestiaorg/knuu/pkg/names" + "github.com/celestiaorg/knuu/pkg/system" ) const ( @@ -31,9 +30,8 @@ const ( ) type Kaniko struct { - K8sClient k8s.KubeManager - MinioClient *minio.Minio // Minio service to store the build context if it's a directory - ContentName string // Name of the content pushed to Minio + system.SystemDependencies + ContentName string // Name of the content pushed to Minio } var _ builder.Builder = &Kaniko{} @@ -159,10 +157,7 @@ func (k *Kaniko) cleanup(ctx context.Context, job *batchv1.Job) error { // Delete the content pushed to Minio if k.ContentName != "" { - if err := k.initMinio(ctx); err != nil { - return ErrMinioFailedToGetDeployment.Wrap(err) - } - if err := k.MinioClient.DeleteFromMinio(ctx, k.ContentName, MinioBucketName); err != nil { + if err := k.MinioClient.Delete(ctx, k.ContentName, MinioBucketName); err != nil { return ErrDeletingMinioContent.Wrap(err) } } @@ -170,16 +165,6 @@ func (k *Kaniko) cleanup(ctx context.Context, job *batchv1.Job) error { return nil } -func (k *Kaniko) initMinio(ctx context.Context) error { - if k.MinioClient != nil { - return nil - } - - var err error - k.MinioClient, err = minio.New(ctx, k.K8sClient) - return err -} - func (k *Kaniko) prepareJob(ctx context.Context, b *builder.BuilderOptions) (*batchv1.Job, error) { jobName, err := names.NewRandomK8(kanikoJobNamePrefix) if err != nil { @@ -261,10 +246,6 @@ func (k *Kaniko) prepareJob(ctx context.Context, b *builder.BuilderOptions) (*ba // As kaniko also supports directly tar.gz archives, no need to extract it, // we just need to set the context to tar:// func (k *Kaniko) mountDir(ctx context.Context, bCtx string, job *batchv1.Job) (*batchv1.Job, error) { - if err := k.initMinio(ctx); err != nil { - return nil, ErrMinioFailedToGetDeployment.Wrap(err) - } - // Create the tar.gz archive archiveData, err := createTarGz(builder.GetDirFromBuildContext(bCtx)) if err != nil { @@ -276,11 +257,11 @@ func (k *Kaniko) mountDir(ctx context.Context, bCtx string, job *batchv1.Job) (* hash.Write(archiveData) k.ContentName = hex.EncodeToString(hash.Sum(nil)) - if err := k.MinioClient.PushToMinio(ctx, bytes.NewReader(archiveData), k.ContentName, MinioBucketName); err != nil { + if err := k.MinioClient.Push(ctx, bytes.NewReader(archiveData), k.ContentName, MinioBucketName); err != nil { return nil, err } - s3URL, err := k.MinioClient.GetMinioURL(ctx, k.ContentName, MinioBucketName) + s3URL, err := k.MinioClient.GetURL(ctx, k.ContentName, MinioBucketName) if err != nil { return nil, err } diff --git a/pkg/builder/kaniko/kaniko_test.go b/pkg/builder/kaniko/kaniko_test.go index 17e63f62..2f3dc87f 100644 --- a/pkg/builder/kaniko/kaniko_test.go +++ b/pkg/builder/kaniko/kaniko_test.go @@ -15,6 +15,7 @@ import ( "github.com/celestiaorg/knuu/pkg/builder" "github.com/celestiaorg/knuu/pkg/k8s" + "github.com/celestiaorg/knuu/pkg/system" ) const ( @@ -28,7 +29,9 @@ func TestKanikoBuilder(t *testing.T) { k8sClient, err := k8s.NewClientCustom(context.Background(), k8sCS, k8sCS.Discovery(), nil, k8sNamespace) require.NoError(t, err) kb := &Kaniko{ - K8sClient: k8sClient, + SystemDependencies: system.SystemDependencies{ + K8sClient: k8sClient, + }, } ctx := context.Background() diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go index 1a10e9a2..39713530 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -42,9 +42,9 @@ type Options struct { K8s k8s.KubeManager TestScope string ImageBuilder builder.Builder - Minio *minio.Minio - Timeout time.Duration + MinioEnabled bool ProxyEnabled bool + Timeout time.Duration Logger *logrus.Logger } @@ -56,7 +56,6 @@ func New(ctx context.Context, opts Options) (*Knuu, error) { k := &Knuu{ SystemDependencies: system.SystemDependencies{ K8sClient: opts.K8s, - MinioClient: opts.Minio, ImageBuilder: opts.ImageBuilder, Logger: opts.Logger, TestScope: opts.TestScope, @@ -69,6 +68,12 @@ func New(ctx context.Context, opts Options) (*Knuu, error) { return nil, err } + if opts.MinioEnabled { + if err := setupMinio(ctx, k); err != nil { + return nil, err + } + } + if opts.ProxyEnabled { if err := setupProxy(ctx, k); err != nil { return nil, err @@ -195,8 +200,7 @@ func setDefaults(ctx context.Context, k *Knuu) error { if k.ImageBuilder == nil { k.ImageBuilder = &kaniko.Kaniko{ - K8sClient: k.K8sClient, - MinioClient: k.MinioClient, + SystemDependencies: k.SystemDependencies, } } @@ -217,3 +221,8 @@ func setupProxy(ctx context.Context, k *Knuu) error { k.Logger.Debugf("Proxy endpoint: %s", endpoint) return nil } + +func setupMinio(ctx context.Context, k *Knuu) (err error) { + k.MinioClient, err = minio.New(ctx, k.K8sClient) + return err +} diff --git a/pkg/knuu/knuu_old.go b/pkg/knuu/knuu_old.go index cb3d62af..26a84d1d 100644 --- a/pkg/knuu/knuu_old.go +++ b/pkg/knuu/knuu_old.go @@ -21,6 +21,8 @@ import ( "github.com/celestiaorg/knuu/pkg/builder/kaniko" ) +const minioBucketName = "knuu" + // This is a temporary variable to hold the knuu instance until we refactor knuu pkg // TODO: remove this temporary variable var tmpKnuu *Knuu @@ -73,6 +75,7 @@ func InitializeWithScope(testScope string) error { TestScope: testScope, Timeout: timeout, ProxyEnabled: true, + MinioEnabled: true, }) if err != nil { return ErrCannotInitializeKnuu.Wrap(err) @@ -82,8 +85,7 @@ func InitializeWithScope(testScope string) error { switch builderType { case "kubernetes": tmpKnuu.ImageBuilder = &kaniko.Kaniko{ - K8sClient: tmpKnuu.K8sClient, - MinioClient: tmpKnuu.MinioClient, + SystemDependencies: tmpKnuu.SystemDependencies, } case "docker", "": tmpKnuu.ImageBuilder = &docker.Docker{ @@ -135,10 +137,10 @@ func CleanUp() error { // Deprecated: Use the new package knuu instead. func PushFileToMinio(ctx context.Context, contentName string, reader io.Reader) error { - return tmpKnuu.PushFileToMinio(ctx, contentName, reader) + return tmpKnuu.MinioClient.Push(ctx, reader, contentName, minioBucketName) } // Deprecated: Use the new package knuu instead. func GetMinioURL(ctx context.Context, contentName string) (string, error) { - return tmpKnuu.GetMinioURL(ctx, contentName) + return tmpKnuu.MinioClient.GetURL(ctx, contentName, minioBucketName) } diff --git a/pkg/knuu/knuu_test.go b/pkg/knuu/knuu_test.go index eb11633f..6c9892da 100644 --- a/pkg/knuu/knuu_test.go +++ b/pkg/knuu/knuu_test.go @@ -14,7 +14,6 @@ import ( "github.com/celestiaorg/knuu/pkg/builder/kaniko" "github.com/celestiaorg/knuu/pkg/k8s" - "github.com/celestiaorg/knuu/pkg/minio" ) const ( @@ -68,6 +67,18 @@ func TestNew(t *testing.T) { name: "Default initialization", options: Options{}, expectError: false, + validateFunc: func(t *testing.T, k *Knuu) { + assert.NotNil(t, k) + assert.NotNil(t, k.Logger) + assert.NotNil(t, k.K8sClient) + assert.NotNil(t, k.ImageBuilder) + assert.Equal(t, defaultTimeout, k.timeout) + }, + }, + { + name: "Minio enabled", + options: Options{MinioEnabled: true}, + expectError: false, validateFunc: func(t *testing.T, k *Knuu) { assert.NotNil(t, k) assert.NotNil(t, k.Logger) @@ -110,17 +121,6 @@ func TestNew(t *testing.T) { assert.NotNil(t, k.K8sClient) }, }, - { - name: "With custom Minio client", - options: Options{ - Minio: &minio.Minio{}, - }, - expectError: false, - validateFunc: func(t *testing.T, k *Knuu) { - assert.NotNil(t, k) - assert.NotNil(t, k.MinioClient) - }, - }, { name: "With custom Image Builder", options: Options{ diff --git a/pkg/knuu/minio.go b/pkg/knuu/minio.go deleted file mode 100644 index f12da202..00000000 --- a/pkg/knuu/minio.go +++ /dev/null @@ -1,38 +0,0 @@ -package knuu - -import ( - "context" - "io" - - "github.com/celestiaorg/knuu/pkg/minio" -) - -const minioBucketName = "knuu" - -// initMinio initializes the Minio client -// Since not always we need minio to be deployed and ready, -// We deploy it on the first use. i.e. minio.New deploys it -func (k *Knuu) initMinio(ctx context.Context) error { - if k.MinioClient != nil { - return nil - } - - var err error - k.MinioClient, err = minio.New(ctx, k.K8sClient) - return err -} - -// contentName is a unique string to identify the content in Minio -func (k *Knuu) PushFileToMinio(ctx context.Context, contentName string, reader io.Reader) error { - if err := k.initMinio(ctx); err != nil { - return err - } - return k.MinioClient.PushToMinio(ctx, reader, contentName, minioBucketName) -} - -func (k *Knuu) GetMinioURL(ctx context.Context, contentName string) (string, error) { - if err := k.initMinio(ctx); err != nil { - return "", err - } - return k.MinioClient.GetMinioURL(ctx, contentName, minioBucketName) -} diff --git a/pkg/minio/errors.go b/pkg/minio/errors.go index 0dd896db..fceb49e7 100644 --- a/pkg/minio/errors.go +++ b/pkg/minio/errors.go @@ -41,4 +41,5 @@ var ( ErrMinioFailedToCreatePersistentVolume = errors.New("MinioFailedToCreatePersistentVolume", "failed to create PersistentVolume") ErrMinioFailedToCreatePersistentVolumeClaim = errors.New("MinioFailedToCreatePersistentVolumeClaim", "failed to create PersistentVolumeClaim") ErrMinioClientNotInitialized = errors.New("MinioClientNotInitialized", "Minio client not initialized") + ErrMinioNotInitialized = errors.New("MinioNotInitialized", "Minio not initialized") ) diff --git a/pkg/minio/minio.go b/pkg/minio/minio.go index 0f749325..7f3068f8 100644 --- a/pkg/minio/minio.go +++ b/pkg/minio/minio.go @@ -42,8 +42,8 @@ const ( ) type Minio struct { - client *miniogo.Client - K8s k8s.KubeManager + client *miniogo.Client + k8sClient k8s.KubeManager } type Config struct { @@ -54,19 +54,11 @@ type Config struct { func New(ctx context.Context, k8sClient k8s.KubeManager) (*Minio, error) { m := &Minio{ - K8s: k8sClient, + k8sClient: k8sClient, } - // [optimization] since it might be called from different places, - // we need to check if minio is already deployed - isMinioDeployed, err := m.isMinioDeployed(ctx) - if err != nil { - return nil, ErrMinioFailedToGetDeployment.Wrap(err) - } - if !isMinioDeployed { - if err := m.deployMinio(ctx); err != nil { - return nil, err - } + if err := m.deployMinio(ctx); err != nil { + return nil, err } endpoint, err := m.getEndpoint(ctx) @@ -85,10 +77,10 @@ func New(ctx context.Context, k8sClient k8s.KubeManager) (*Minio, error) { return m, nil } -// PushToMinio pushes data (i.e. a reader) to Minio -func (m *Minio) PushToMinio(ctx context.Context, localReader io.Reader, minioFilePath, bucketName string) error { - if m.client == nil { - return ErrMinioClientNotInitialized +// Push pushes data (i.e. a reader) to Minio +func (m *Minio) Push(ctx context.Context, localReader io.Reader, minioFilePath, bucketName string) error { + if m == nil { + return ErrMinioNotInitialized } if err := m.createBucketIfNotExists(ctx, bucketName); err != nil { @@ -104,10 +96,10 @@ func (m *Minio) PushToMinio(ctx context.Context, localReader io.Reader, minioFil return nil } -// DeleteFromMinio deletes a file from Minio and fails if the content does not exist -func (m *Minio) DeleteFromMinio(ctx context.Context, minioFilePath, bucketName string) error { - if m.client == nil { - return ErrMinioClientNotInitialized +// Delete deletes a file from Minio and fails if the content does not exist +func (m *Minio) Delete(ctx context.Context, minioFilePath, bucketName string) error { + if m == nil { + return ErrMinioNotInitialized } // Check if the object exists before attempting to delete @@ -125,10 +117,10 @@ func (m *Minio) DeleteFromMinio(ctx context.Context, minioFilePath, bucketName s return nil } -// GetMinioURL returns an S3-compatible URL for a Minio file -func (m *Minio) GetMinioURL(ctx context.Context, minioFilePath, bucketName string) (string, error) { - if m.client == nil { - return "", ErrMinioClientNotInitialized +// GetURL returns an S3-compatible URL for a Minio file +func (m *Minio) GetURL(ctx context.Context, minioFilePath, bucketName string) (string, error) { + if m == nil { + return "", ErrMinioNotInitialized } // Set the expiration time for the URL (e.g., 24h from now) @@ -144,6 +136,10 @@ func (m *Minio) GetMinioURL(ctx context.Context, minioFilePath, bucketName strin } func (m *Minio) GetConfigs(ctx context.Context) (*Config, error) { + if m == nil { + return nil, ErrMinioNotInitialized + } + endpoint, err := m.getEndpoint(ctx) if err != nil { return nil, ErrMinioFailedToGetEndpoint.Wrap(err) @@ -169,7 +165,7 @@ func (m *Minio) deployMinio(ctx context.Context) error { return ErrMinioFailedToCreateOrUpdateService.Wrap(err) } - if err := m.K8s.WaitForService(ctx, ServiceName); err != nil { + if err := m.k8sClient.WaitForService(ctx, ServiceName); err != nil { return ErrMinioFailedToBeReadyService.Wrap(err) } @@ -178,13 +174,13 @@ func (m *Minio) deployMinio(ctx context.Context) error { } func (m *Minio) createOrUpdateDeployment(ctx context.Context) error { - deploymentClient := m.K8s.Clientset().AppsV1().Deployments(m.K8s.Namespace()) + deploymentClient := m.k8sClient.Clientset().AppsV1().Deployments(m.k8sClient.Namespace()) // Define the Minio deployment minioDeployment := &appsV1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: DeploymentName, - Namespace: m.K8s.Namespace(), + Namespace: m.k8sClient.Namespace(), }, Spec: appsV1.DeploymentSpec{ Selector: &metav1.LabelSelector{ @@ -256,28 +252,14 @@ func (m *Minio) createOrUpdateDeployment(ctx context.Context) error { return nil } -func (m *Minio) isMinioDeployed(ctx context.Context) (bool, error) { - deploymentClient := m.K8s.Clientset().AppsV1().Deployments(m.K8s.Namespace()) - - _, err := deploymentClient.Get(ctx, DeploymentName, metav1.GetOptions{}) - if err != nil { - if errors.IsNotFound(err) { - return false, nil - } - return false, ErrMinioFailedToGetService.Wrap(err) - } - - return true, nil -} - func (m *Minio) createOrUpdateService(ctx context.Context) error { - serviceClient := m.K8s.Clientset().CoreV1().Services(m.K8s.Namespace()) + serviceClient := m.k8sClient.Clientset().CoreV1().Services(m.k8sClient.Namespace()) // Define Minio service minioService := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: ServiceName, - Namespace: m.K8s.Namespace(), + Namespace: m.k8sClient.Namespace(), }, Spec: v1.ServiceSpec{ Selector: map[string]string{"app": "minio"}, @@ -342,7 +324,7 @@ func (m *Minio) createBucketIfNotExists(ctx context.Context, bucketName string) } func (m *Minio) getEndpoint(ctx context.Context) (string, error) { - minioService, err := m.K8s.Clientset().CoreV1().Services(m.K8s.Namespace()).Get(ctx, ServiceName, metav1.GetOptions{}) + minioService, err := m.k8sClient.Clientset().CoreV1().Services(m.k8sClient.Namespace()).Get(ctx, ServiceName, metav1.GetOptions{}) if err != nil { return "", ErrMinioFailedToGetService.Wrap(err) } @@ -357,7 +339,7 @@ func (m *Minio) getEndpoint(ctx context.Context) (string, error) { if minioService.Spec.Type == v1.ServiceTypeNodePort { // Use the Node IP and NodePort - nodes, err := m.K8s.Clientset().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) + nodes, err := m.k8sClient.Clientset().CoreV1().Nodes().List(ctx, metav1.ListOptions{}) if err != nil { return "", ErrMinioFailedToGetNodes.Wrap(err) } @@ -381,7 +363,7 @@ func (m *Minio) getEndpoint(ctx context.Context) (string, error) { func (m *Minio) waitForMinio(ctx context.Context) error { for { - deployment, err := m.K8s.Clientset().AppsV1().Deployments(m.K8s.Namespace()).Get(ctx, DeploymentName, metav1.GetOptions{}) + deployment, err := m.k8sClient.Clientset().AppsV1().Deployments(m.k8sClient.Namespace()).Get(ctx, DeploymentName, metav1.GetOptions{}) if err == nil && deployment.Status.ReadyReplicas > 0 { break } @@ -403,7 +385,7 @@ func (m *Minio) createPVC(ctx context.Context, pvcName string, storageSize strin return ErrMinioFailedToParseStorageSize.Wrap(err) } - pvcClient := m.K8s.Clientset().CoreV1().PersistentVolumeClaims(m.K8s.Namespace()) + pvcClient := m.k8sClient.Clientset().CoreV1().PersistentVolumeClaims(m.k8sClient.Namespace()) // Check if PVC already exists _, err = pvcClient.Get(ctx, pvcName, metav1.GetOptions{}) @@ -413,7 +395,7 @@ func (m *Minio) createPVC(ctx context.Context, pvcName string, storageSize strin } // Create a simple PersistentVolume if no suitable one is found - pvList, err := m.K8s.Clientset().CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{}) + pvList, err := m.k8sClient.Clientset().CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{}) if err != nil { return ErrMinioFailedToListPersistentVolumes.Wrap(err) } @@ -429,7 +411,7 @@ func (m *Minio) createPVC(ctx context.Context, pvcName string, storageSize strin if existingPV == nil { // Create a simple PV if no existing PV is suitable - _, err = m.K8s.Clientset().CoreV1().PersistentVolumes().Create(ctx, &v1.PersistentVolume{ + _, err = m.k8sClient.Clientset().CoreV1().PersistentVolumes().Create(ctx, &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ GenerateName: pvPrefix, }, @@ -455,7 +437,7 @@ func (m *Minio) createPVC(ctx context.Context, pvcName string, storageSize strin pvc := &v1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Name: pvcName, - Namespace: m.K8s.Namespace(), + Namespace: m.k8sClient.Namespace(), }, Spec: v1.PersistentVolumeClaimSpec{ AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, From c235d69cf770442f92436cc26ff211bc5705ecc2 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Tue, 18 Jun 2024 17:37:17 +0200 Subject: [PATCH 06/14] fix: remove extra minio deployment command --- e2e/tshark/tshark_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/e2e/tshark/tshark_test.go b/e2e/tshark/tshark_test.go index e85baf79..744a7b71 100644 --- a/e2e/tshark/tshark_test.go +++ b/e2e/tshark/tshark_test.go @@ -13,7 +13,6 @@ import ( "github.com/celestiaorg/knuu/pkg/instance" "github.com/celestiaorg/knuu/pkg/knuu" - "github.com/celestiaorg/knuu/pkg/minio" ) const ( @@ -48,10 +47,6 @@ func TestTshark(t *testing.T) { err = target.SetCommand("sleep", "infinity") require.NoError(t, err, "error setting command") - t.Log("deploying minio as s3 backend") - kn.MinioClient, err = minio.New(ctx, kn.K8sClient) - require.NoError(t, err, "error deploying minio") - t.Log("getting minio configs") minioConf, err := kn.MinioClient.GetConfigs(ctx) require.NoError(t, err, "error getting S3 (minio) configs") From aed693b07c434fc80c5e96681dad004e0637d792 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Tue, 18 Jun 2024 18:32:52 +0200 Subject: [PATCH 07/14] fix: New options for minio & knuu --- e2e/bittwister/suite_setup_test.go | 11 +++++---- e2e/system/build_from_git_test.go | 7 +++++- e2e/tshark/tshark_test.go | 10 +++++++- pkg/knuu/knuu.go | 39 +++++++++--------------------- pkg/knuu/knuu_old.go | 18 +++++++++++--- pkg/knuu/knuu_test.go | 22 +++-------------- 6 files changed, 52 insertions(+), 55 deletions(-) diff --git a/e2e/bittwister/suite_setup_test.go b/e2e/bittwister/suite_setup_test.go index 782e41ba..2a8b54fb 100644 --- a/e2e/bittwister/suite_setup_test.go +++ b/e2e/bittwister/suite_setup_test.go @@ -7,6 +7,7 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/suite" + "github.com/celestiaorg/knuu/pkg/k8s" "github.com/celestiaorg/knuu/pkg/knuu" ) @@ -16,11 +17,11 @@ type Suite struct { } func (s *Suite) SetupSuite() { - var ( - err error - ctx = context.Background() - ) - s.Knuu, err = knuu.New(ctx, knuu.Options{ProxyEnabled: true}) + ctx := context.Background() + k8sClient, err := k8s.NewClient(ctx, knuu.DefaultTestScope()) + s.Require().NoError(err) + + s.Knuu, err = knuu.New(ctx, k8sClient, 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 index 0df16300..d32568db 100644 --- a/e2e/system/build_from_git_test.go +++ b/e2e/system/build_from_git_test.go @@ -8,6 +8,7 @@ import ( "github.com/celestiaorg/knuu/pkg/builder" "github.com/celestiaorg/knuu/pkg/instance" + "github.com/celestiaorg/knuu/pkg/k8s" "github.com/celestiaorg/knuu/pkg/knuu" ) @@ -18,7 +19,11 @@ func TestBuildFromGit(t *testing.T) { ctx := context.Background() // The default image builder is kaniko here - kn, err := knuu.New(ctx, knuu.Options{}) + + k8sClient, err := k8s.NewClient(ctx, knuu.DefaultTestScope()) + require.NoError(t, err, "error creating k8s client") + + kn, err := knuu.New(ctx, k8sClient, knuu.Options{}) require.NoError(t, err, "Error creating knuu") sampleInstance, err := kn.NewInstance("git-builder") diff --git a/e2e/tshark/tshark_test.go b/e2e/tshark/tshark_test.go index 744a7b71..3bb7f79b 100644 --- a/e2e/tshark/tshark_test.go +++ b/e2e/tshark/tshark_test.go @@ -12,7 +12,9 @@ import ( "github.com/stretchr/testify/require" "github.com/celestiaorg/knuu/pkg/instance" + "github.com/celestiaorg/knuu/pkg/k8s" "github.com/celestiaorg/knuu/pkg/knuu" + "github.com/celestiaorg/knuu/pkg/minio" ) const ( @@ -26,7 +28,13 @@ func TestTshark(t *testing.T) { ctx := context.Background() - kn, err := knuu.New(ctx, knuu.Options{MinioEnabled: true}) + k8sClient, err := k8s.NewClient(ctx, knuu.DefaultTestScope()) + require.NoError(t, err, "error creating k8s client") + + minioClient, err := minio.New(ctx, k8sClient) + require.NoError(t, err, "error creating minio client") + + kn, err := knuu.New(ctx, k8sClient, knuu.Options{MinioClient: minioClient}) require.NoError(t, err, "error creating knuu") defer func() { diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go index 39713530..fe1a395b 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -39,23 +39,23 @@ type Knuu struct { } type Options struct { - K8s k8s.KubeManager TestScope string ImageBuilder builder.Builder - MinioEnabled bool + MinioClient *minio.Minio ProxyEnabled bool Timeout time.Duration Logger *logrus.Logger } -func New(ctx context.Context, opts Options) (*Knuu, error) { +func New(ctx context.Context, k8sClient k8s.KubeManager, opts Options) (*Knuu, error) { if err := loadEnvVariables(); err != nil { return nil, err } k := &Knuu{ SystemDependencies: system.SystemDependencies{ - K8sClient: opts.K8s, + K8sClient: k8sClient, + MinioClient: opts.MinioClient, ImageBuilder: opts.ImageBuilder, Logger: opts.Logger, TestScope: opts.TestScope, @@ -64,16 +64,10 @@ func New(ctx context.Context, opts Options) (*Knuu, error) { timeout: opts.Timeout, } - if err := setDefaults(ctx, k); err != nil { + if err := setDefaults(k); err != nil { return nil, err } - if opts.MinioEnabled { - if err := setupMinio(ctx, k); err != nil { - return nil, err - } - } - if opts.ProxyEnabled { if err := setupProxy(ctx, k); err != nil { return nil, err @@ -87,6 +81,11 @@ func New(ctx context.Context, opts Options) (*Knuu, error) { return k, nil } +func DefaultTestScope() string { + t := time.Now() + return fmt.Sprintf("%s-%03d", t.Format("20060102-150405"), t.Nanosecond()/1e6) +} + func (k *Knuu) Scope() string { return k.TestScope } @@ -176,28 +175,19 @@ func loadEnvVariables() error { return nil } -func setDefaults(ctx context.Context, k *Knuu) error { +func setDefaults(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) + k.TestScope = k.K8sClient.Namespace() } if k.timeout == 0 { k.timeout = defaultTimeout } - if k.K8sClient == nil { - var err error - k.K8sClient, err = k8s.NewClient(ctx, k.TestScope) - if err != nil { - return ErrCannotInitializeK8s.Wrap(err) - } - } - if k.ImageBuilder == nil { k.ImageBuilder = &kaniko.Kaniko{ SystemDependencies: k.SystemDependencies, @@ -221,8 +211,3 @@ func setupProxy(ctx context.Context, k *Knuu) error { k.Logger.Debugf("Proxy endpoint: %s", endpoint) return nil } - -func setupMinio(ctx context.Context, k *Knuu) (err error) { - k.MinioClient, err = minio.New(ctx, k.K8sClient) - return err -} diff --git a/pkg/knuu/knuu_old.go b/pkg/knuu/knuu_old.go index 26a84d1d..a8418842 100644 --- a/pkg/knuu/knuu_old.go +++ b/pkg/knuu/knuu_old.go @@ -19,6 +19,8 @@ import ( "github.com/celestiaorg/knuu/pkg/builder" "github.com/celestiaorg/knuu/pkg/builder/docker" "github.com/celestiaorg/knuu/pkg/builder/kaniko" + "github.com/celestiaorg/knuu/pkg/k8s" + "github.com/celestiaorg/knuu/pkg/minio" ) const minioBucketName = "knuu" @@ -70,12 +72,22 @@ func InitializeWithScope(testScope string) error { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() - var err error - tmpKnuu, err = New(ctx, Options{ + + k8sClient, err := k8s.NewClient(ctx, testScope) + if err != nil { + return ErrCannotInitializeKnuu.Wrap(err) + } + + minioClient, err := minio.New(ctx, k8sClient) + if err != nil { + return ErrCannotInitializeKnuu.Wrap(err) + } + + tmpKnuu, err = New(ctx, k8sClient, Options{ TestScope: testScope, Timeout: timeout, ProxyEnabled: true, - MinioEnabled: true, + MinioClient: minioClient, }) if err != nil { return ErrCannotInitializeKnuu.Wrap(err) diff --git a/pkg/knuu/knuu_test.go b/pkg/knuu/knuu_test.go index 6c9892da..fe063bed 100644 --- a/pkg/knuu/knuu_test.go +++ b/pkg/knuu/knuu_test.go @@ -14,6 +14,7 @@ import ( "github.com/celestiaorg/knuu/pkg/builder/kaniko" "github.com/celestiaorg/knuu/pkg/k8s" + "github.com/celestiaorg/knuu/pkg/minio" ) const ( @@ -76,16 +77,12 @@ func TestNew(t *testing.T) { }, }, { - name: "Minio enabled", - options: Options{MinioEnabled: true}, + name: "With custom Minio client", + options: Options{MinioClient: &minio.Minio{}}, expectError: false, validateFunc: func(t *testing.T, k *Knuu) { assert.NotNil(t, k) - assert.NotNil(t, k.Logger) - assert.NotNil(t, k.K8sClient) assert.NotNil(t, k.MinioClient) - assert.NotNil(t, k.ImageBuilder) - assert.Equal(t, defaultTimeout, k.timeout) }, }, { @@ -110,17 +107,6 @@ func TestNew(t *testing.T) { assert.Equal(t, 30*time.Minute, k.timeout) }, }, - { - name: "With custom K8s client", - options: Options{ - K8s: &mockK8s{}, - }, - expectError: false, - validateFunc: func(t *testing.T, k *Knuu) { - assert.NotNil(t, k) - assert.NotNil(t, k.K8sClient) - }, - }, { name: "With custom Image Builder", options: Options{ @@ -136,7 +122,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, &mockK8s{}, tc.options) if tc.expectError { assert.Error(t, err) return From 1dd09fb5cc3ec1bb3910572591824e14b64eb738 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Thu, 20 Jun 2024 10:38:36 +0200 Subject: [PATCH 08/14] fix: applied the requested changes --- e2e/bittwister/suite_setup_test.go | 10 ++++---- e2e/system/build_from_git_test.go | 5 ++-- e2e/tshark/tshark_test.go | 5 ++-- e2e/utils.go | 11 +++++++++ pkg/knuu/errors.go | 2 ++ pkg/knuu/knuu.go | 31 +++++++++++++++++------- pkg/knuu/knuu_old.go | 3 ++- pkg/knuu/knuu_test.go | 38 ++++++++++++++++++++++++++---- 8 files changed, 82 insertions(+), 23 deletions(-) create mode 100644 e2e/utils.go diff --git a/e2e/bittwister/suite_setup_test.go b/e2e/bittwister/suite_setup_test.go index 2a8b54fb..8dfffded 100644 --- a/e2e/bittwister/suite_setup_test.go +++ b/e2e/bittwister/suite_setup_test.go @@ -7,7 +7,7 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/suite" - "github.com/celestiaorg/knuu/pkg/k8s" + "github.com/celestiaorg/knuu/e2e" "github.com/celestiaorg/knuu/pkg/knuu" ) @@ -18,10 +18,12 @@ type Suite struct { func (s *Suite) SetupSuite() { ctx := context.Background() - k8sClient, err := k8s.NewClient(ctx, knuu.DefaultTestScope()) - s.Require().NoError(err) - s.Knuu, err = knuu.New(ctx, k8sClient, knuu.Options{ProxyEnabled: true}) + var err error + s.Knuu, err = knuu.New(ctx, knuu.Options{ + ProxyEnabled: true, + TestScope: e2e.DefaultTestScope(), + }) 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 index d32568db..0e0e22c5 100644 --- a/e2e/system/build_from_git_test.go +++ b/e2e/system/build_from_git_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/celestiaorg/knuu/e2e" "github.com/celestiaorg/knuu/pkg/builder" "github.com/celestiaorg/knuu/pkg/instance" "github.com/celestiaorg/knuu/pkg/k8s" @@ -20,10 +21,10 @@ func TestBuildFromGit(t *testing.T) { // The default image builder is kaniko here - k8sClient, err := k8s.NewClient(ctx, knuu.DefaultTestScope()) + k8sClient, err := k8s.NewClient(ctx, e2e.DefaultTestScope()) require.NoError(t, err, "error creating k8s client") - kn, err := knuu.New(ctx, k8sClient, knuu.Options{}) + kn, err := knuu.New(ctx, knuu.Options{K8sClient: k8sClient}) require.NoError(t, err, "Error creating knuu") sampleInstance, err := kn.NewInstance("git-builder") diff --git a/e2e/tshark/tshark_test.go b/e2e/tshark/tshark_test.go index 3bb7f79b..3e9492e3 100644 --- a/e2e/tshark/tshark_test.go +++ b/e2e/tshark/tshark_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/celestiaorg/knuu/e2e" "github.com/celestiaorg/knuu/pkg/instance" "github.com/celestiaorg/knuu/pkg/k8s" "github.com/celestiaorg/knuu/pkg/knuu" @@ -28,13 +29,13 @@ func TestTshark(t *testing.T) { ctx := context.Background() - k8sClient, err := k8s.NewClient(ctx, knuu.DefaultTestScope()) + k8sClient, err := k8s.NewClient(ctx, e2e.DefaultTestScope()) require.NoError(t, err, "error creating k8s client") minioClient, err := minio.New(ctx, k8sClient) require.NoError(t, err, "error creating minio client") - kn, err := knuu.New(ctx, k8sClient, knuu.Options{MinioClient: minioClient}) + kn, err := knuu.New(ctx, knuu.Options{MinioClient: minioClient, K8sClient: k8sClient}) require.NoError(t, err, "error creating knuu") defer func() { diff --git a/e2e/utils.go b/e2e/utils.go new file mode 100644 index 00000000..51f2297a --- /dev/null +++ b/e2e/utils.go @@ -0,0 +1,11 @@ +package e2e + +import ( + "fmt" + "time" +) + +func DefaultTestScope() string { + t := time.Now() + return fmt.Sprintf("%s-%03d", t.Format("20060102-150405"), t.Nanosecond()/1e6) +} diff --git a/pkg/knuu/errors.go b/pkg/knuu/errors.go index 489b58f4..eb723825 100644 --- a/pkg/knuu/errors.go +++ b/pkg/knuu/errors.go @@ -205,4 +205,6 @@ var ( ErrCannotGetTraefikEndpoint = errors.New("CannotGetTraefikEndpoint", "cannot get traefik endpoint") ErrGettingProxyURL = errors.New("GettingProxyURL", "error getting proxy URL for service '%s'") ErrTraefikAPINotAvailable = errors.New("TraefikAPINotAvailable", "traefik API is not available") + ErrTestScopeNotSet = errors.New("TestScopeNotSet", "test scope is not set") + ErrK8sClientNotSet = errors.New("K8sClientNotSet", "k8s client is not set") ) diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go index fe1a395b..c6e68aa0 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -39,6 +39,7 @@ type Knuu struct { } type Options struct { + K8sClient k8s.KubeManager TestScope string ImageBuilder builder.Builder MinioClient *minio.Minio @@ -47,14 +48,14 @@ type Options struct { Logger *logrus.Logger } -func New(ctx context.Context, k8sClient k8s.KubeManager, opts Options) (*Knuu, error) { +func New(ctx context.Context, opts Options) (*Knuu, error) { if err := loadEnvVariables(); err != nil { return nil, err } k := &Knuu{ SystemDependencies: system.SystemDependencies{ - K8sClient: k8sClient, + K8sClient: opts.K8sClient, MinioClient: opts.MinioClient, ImageBuilder: opts.ImageBuilder, Logger: opts.Logger, @@ -64,7 +65,13 @@ func New(ctx context.Context, k8sClient k8s.KubeManager, opts Options) (*Knuu, e timeout: opts.Timeout, } - if err := setDefaults(k); err != nil { + // When Minio is set, K8sClient must be set too + // to make sure that there is only one source of truth for the k8s client + if k.MinioClient != nil && k.K8sClient == nil { + return nil, ErrK8sClientNotSet + } + + if err := setDefaults(ctx, k); err != nil { return nil, err } @@ -81,11 +88,6 @@ func New(ctx context.Context, k8sClient k8s.KubeManager, opts Options) (*Knuu, e return k, nil } -func DefaultTestScope() string { - t := time.Now() - return fmt.Sprintf("%s-%03d", t.Format("20060102-150405"), t.Nanosecond()/1e6) -} - func (k *Knuu) Scope() string { return k.TestScope } @@ -175,12 +177,15 @@ func loadEnvVariables() error { return nil } -func setDefaults(k *Knuu) error { +func setDefaults(ctx context.Context, k *Knuu) error { if k.Logger == nil { k.Logger = log.DefaultLogger() } if k.TestScope == "" { + if k.K8sClient == nil { + return ErrTestScopeNotSet + } k.TestScope = k.K8sClient.Namespace() } @@ -194,6 +199,14 @@ func setDefaults(k *Knuu) error { } } + if k.K8sClient == nil { + var err error + k.K8sClient, err = k8s.NewClient(ctx, k.TestScope) + if err != nil { + return ErrCannotInitializeK8s.Wrap(err) + } + } + return nil } diff --git a/pkg/knuu/knuu_old.go b/pkg/knuu/knuu_old.go index a8418842..5ebb1218 100644 --- a/pkg/knuu/knuu_old.go +++ b/pkg/knuu/knuu_old.go @@ -83,7 +83,8 @@ func InitializeWithScope(testScope string) error { return ErrCannotInitializeKnuu.Wrap(err) } - tmpKnuu, err = New(ctx, k8sClient, Options{ + tmpKnuu, err = New(ctx, Options{ + K8sClient: k8sClient, TestScope: testScope, Timeout: timeout, ProxyEnabled: true, diff --git a/pkg/knuu/knuu_test.go b/pkg/knuu/knuu_test.go index fe063bed..42a1376a 100644 --- a/pkg/knuu/knuu_test.go +++ b/pkg/knuu/knuu_test.go @@ -66,7 +66,7 @@ func TestNew(t *testing.T) { }{ { name: "Default initialization", - options: Options{}, + options: Options{TestScope: "test"}, expectError: false, validateFunc: func(t *testing.T, k *Knuu) { assert.NotNil(t, k) @@ -77,18 +77,28 @@ func TestNew(t *testing.T) { }, }, { - name: "With custom Minio client", + name: "With custom Minio client without setting k8sClient", options: Options{MinioClient: &minio.Minio{}}, + expectError: true, + }, + { + name: "With custom Minio client and K8sClient", + options: Options{ + MinioClient: &minio.Minio{}, + K8sClient: &mockK8s{}, + }, expectError: false, validateFunc: func(t *testing.T, k *Knuu) { assert.NotNil(t, k) assert.NotNil(t, k.MinioClient) + assert.NotNil(t, k.K8sClient) }, }, { name: "With custom Logger", options: Options{ - Logger: &logrus.Logger{}, + TestScope: "test", + Logger: &logrus.Logger{}, }, expectError: false, validateFunc: func(t *testing.T, k *Knuu) { @@ -99,7 +109,8 @@ func TestNew(t *testing.T) { { name: "With custom Timeout", options: Options{ - Timeout: 30 * time.Minute, + TestScope: "test", + Timeout: 30 * time.Minute, }, expectError: false, validateFunc: func(t *testing.T, k *Knuu) { @@ -110,6 +121,7 @@ func TestNew(t *testing.T) { { name: "With custom Image Builder", options: Options{ + TestScope: "test", ImageBuilder: &kaniko.Kaniko{}, }, expectError: false, @@ -118,11 +130,27 @@ func TestNew(t *testing.T) { assert.NotNil(t, k.ImageBuilder) }, }, + { + name: "Without TestScope and K8sClient", + options: Options{}, + expectError: true, + }, + { + name: "With K8sClient but without TestScope", + options: Options{ + K8sClient: &mockK8s{}, + }, + expectError: false, + validateFunc: func(t *testing.T, k *Knuu) { + assert.NotNil(t, k) + assert.Equal(t, "test", k.TestScope) + }, + }, } for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { - k, err := New(ctx, &mockK8s{}, tc.options) + k, err := New(ctx, tc.options) if tc.expectError { assert.Error(t, err) return From fef330665aaf10601944e2d15111ef747ce97b96 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Mon, 24 Jun 2024 18:00:38 +0200 Subject: [PATCH 09/14] fix: applied changes. inc validateOpts --- e2e/system/build_from_git_test.go | 7 +---- pkg/knuu/knuu.go | 28 +++++++++++------- pkg/knuu/knuu_test.go | 48 +++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 16 deletions(-) diff --git a/e2e/system/build_from_git_test.go b/e2e/system/build_from_git_test.go index 0e0e22c5..b86f8e3c 100644 --- a/e2e/system/build_from_git_test.go +++ b/e2e/system/build_from_git_test.go @@ -9,7 +9,6 @@ import ( "github.com/celestiaorg/knuu/e2e" "github.com/celestiaorg/knuu/pkg/builder" "github.com/celestiaorg/knuu/pkg/instance" - "github.com/celestiaorg/knuu/pkg/k8s" "github.com/celestiaorg/knuu/pkg/knuu" ) @@ -20,11 +19,7 @@ func TestBuildFromGit(t *testing.T) { ctx := context.Background() // The default image builder is kaniko here - - k8sClient, err := k8s.NewClient(ctx, e2e.DefaultTestScope()) - require.NoError(t, err, "error creating k8s client") - - kn, err := knuu.New(ctx, knuu.Options{K8sClient: k8sClient}) + kn, err := knuu.New(ctx, knuu.Options{TestScope: e2e.DefaultTestScope()}) 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 index da541406..885f466d 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -49,6 +49,10 @@ type Options struct { } func New(ctx context.Context, opts Options) (*Knuu, error) { + if err := validateOptions(opts); err != nil { + return nil, err + } + if err := loadEnvVariables(); err != nil { return nil, err } @@ -65,12 +69,6 @@ func New(ctx context.Context, opts Options) (*Knuu, error) { timeout: opts.Timeout, } - // When Minio is set, K8sClient must be set too - // to make sure that there is only one source of truth for the k8s client - if k.MinioClient != nil && k.K8sClient == nil { - return nil, ErrK8sClientNotSet - } - if err := setDefaults(ctx, k); err != nil { return nil, err } @@ -166,6 +164,19 @@ func (k *Knuu) handleTimeout(ctx context.Context) error { return nil } +func validateOptions(opts Options) error { + // When Minio is set, K8sClient must be set too + // to make sure that there is only one source of truth for the k8s client + if opts.MinioClient != nil && opts.K8sClient == nil { + return ErrK8sClientNotSet + } + + if opts.TestScope == "" && opts.K8sClient == nil { + return ErrTestScopeNotSet + } + return nil +} + func loadEnvVariables() error { err := godotenv.Load() if err != nil && !os.IsNotExist(err) { @@ -182,10 +193,7 @@ func setDefaults(ctx context.Context, k *Knuu) error { k.Logger = log.DefaultLogger() } - if k.TestScope == "" { - if k.K8sClient == nil { - return ErrTestScopeNotSet - } + if k.TestScope == "" && k.K8sClient != nil { k.TestScope = k.K8sClient.Namespace() } k.TestScope = k8s.SanitizeName(k.TestScope) diff --git a/pkg/knuu/knuu_test.go b/pkg/knuu/knuu_test.go index 42a1376a..ee7ebf26 100644 --- a/pkg/knuu/knuu_test.go +++ b/pkg/knuu/knuu_test.go @@ -161,3 +161,51 @@ func TestNew(t *testing.T) { }) } } + +func TestValidateOptions(t *testing.T) { + tests := []struct { + name string + options Options + wantErr bool + }{ + { + name: "MinioClient set without K8sClient", + options: Options{ + MinioClient: &minio.Minio{}, + }, + wantErr: true, + }, + { + name: "Both MinioClient and K8sClient set", + options: Options{ + MinioClient: &minio.Minio{}, + K8sClient: &mockK8s{}, + }, + wantErr: false, + }, + { + name: "TestScope and K8sClient not set", + options: Options{ + TestScope: "", + K8sClient: nil, + }, + wantErr: true, + }, + { + name: "Valid options", + options: Options{ + TestScope: "test", + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateOptions(tt.options) + if (err != nil) != tt.wantErr { + t.Errorf("validateOptions() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} From 6793a8bec432b8eae72ba93764b582c0c52f6de0 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Tue, 25 Jun 2024 09:58:00 +0200 Subject: [PATCH 10/14] fix: build from git + minio & tests --- e2e/system/build_from_git_test.go | 63 +++++++++++++++++++++++++++++++ e2e/system/main_test.go | 14 +++---- pkg/knuu/knuu.go | 12 +++--- 3 files changed, 74 insertions(+), 15 deletions(-) diff --git a/e2e/system/build_from_git_test.go b/e2e/system/build_from_git_test.go index b86f8e3c..b5075921 100644 --- a/e2e/system/build_from_git_test.go +++ b/e2e/system/build_from_git_test.go @@ -2,14 +2,18 @@ package system import ( "context" + "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/celestiaorg/knuu/e2e" "github.com/celestiaorg/knuu/pkg/builder" "github.com/celestiaorg/knuu/pkg/instance" + "github.com/celestiaorg/knuu/pkg/k8s" "github.com/celestiaorg/knuu/pkg/knuu" + "github.com/celestiaorg/knuu/pkg/minio" ) func TestBuildFromGit(t *testing.T) { @@ -22,6 +26,65 @@ func TestBuildFromGit(t *testing.T) { kn, err := knuu.New(ctx, knuu.Options{TestScope: e2e.DefaultTestScope()}) require.NoError(t, err, "Error creating knuu") + target, err := kn.NewInstance("git-builder") + require.NoError(t, err, "Error creating instance") + + t.Log("Building the image") + + // This is a blocking call which builds the image from git repo + err = target.SetGitRepo(ctx, builder.GitContext{ + Repo: "https://github.com/celestiaorg/knuu.git", + Branch: "test/build-from-git", // This branch has a Dockerfile and is protected as to not be deleted + Username: "", + Password: "", + }) + require.NoError(t, err, "Error setting git repo") + + t.Log("Image built") + + t.Cleanup(func() { + if err := target.Destroy(ctx); err != nil { + t.Logf("Error destroying instance: %v", err) + } + }) + + require.NoError(t, target.Commit()) + + t.Logf("Starting instance") + + assert.NoError(t, target.Start(ctx)) + + t.Logf("Instance started") + + // The file is created by the dockerfile in the repo, + // so to make sure it is built correctly, we check the file + data, err := target.GetFileBytes(ctx, "/test.txt") + require.NoError(t, err, "Error getting file bytes") + + data = []byte(strings.TrimSpace(string(data))) + assert.Equal(t, []byte("Hello, World!"), data, "File bytes do not match") +} +func TestBuildFromGitWithModifications(t *testing.T) { + t.Parallel() + + // Setup + ctx := context.Background() + + k8sClient, err := k8s.NewClient(ctx, e2e.DefaultTestScope()) + require.NoError(t, err, "Error creating k8s client") + + // Since we are modifying the git repo, + // we need to setup minio to allow the builder to push the changes + minioClient, err := minio.New(ctx, k8sClient) + require.NoError(t, err, "Error creating minio client") + + // The default image builder is kaniko here + kn, err := knuu.New(ctx, knuu.Options{ + K8sClient: k8sClient, + MinioClient: minioClient, + }) + require.NoError(t, err, "Error creating knuu") + sampleInstance, err := kn.NewInstance("git-builder") require.NoError(t, err, "Error creating instance") diff --git a/e2e/system/main_test.go b/e2e/system/main_test.go index b8f33f1e..6acc7d16 100644 --- a/e2e/system/main_test.go +++ b/e2e/system/main_test.go @@ -3,18 +3,14 @@ package system import ( "os" "testing" - - "github.com/sirupsen/logrus" - - "github.com/celestiaorg/knuu/pkg/knuu" ) func TestMain(m *testing.M) { - err := knuu.Initialize() - if err != nil { - logrus.Fatalf("error initializing knuu: %v", err) - } - logrus.Infof("Scope: %s", knuu.Scope()) + // err := knuu.Initialize() + // if err != nil { + // logrus.Fatalf("error initializing knuu: %v", err) + // } + // logrus.Infof("Scope: %s", knuu.Scope()) exitVal := m.Run() os.Exit(exitVal) } diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go index 885f466d..81bb3c16 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -202,12 +202,6 @@ func setDefaults(ctx context.Context, k *Knuu) error { k.timeout = defaultTimeout } - if k.ImageBuilder == nil { - k.ImageBuilder = &kaniko.Kaniko{ - SystemDependencies: k.SystemDependencies, - } - } - if k.K8sClient == nil { var err error k.K8sClient, err = k8s.NewClient(ctx, k.TestScope) @@ -216,6 +210,12 @@ func setDefaults(ctx context.Context, k *Knuu) error { } } + if k.ImageBuilder == nil { + k.ImageBuilder = &kaniko.Kaniko{ + SystemDependencies: k.SystemDependencies, + } + } + return nil } From 9c36eac1d1e14a98a58cda1610e41c6fafa98fbf Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Tue, 25 Jun 2024 12:09:36 +0200 Subject: [PATCH 11/14] fix: uncomment the main test code --- e2e/system/main_test.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/e2e/system/main_test.go b/e2e/system/main_test.go index 6acc7d16..3e2709f8 100644 --- a/e2e/system/main_test.go +++ b/e2e/system/main_test.go @@ -3,14 +3,17 @@ package system import ( "os" "testing" + + "github.com/celestiaorg/knuu/pkg/knuu" + "github.com/sirupsen/logrus" ) func TestMain(m *testing.M) { - // err := knuu.Initialize() - // if err != nil { - // logrus.Fatalf("error initializing knuu: %v", err) - // } - // logrus.Infof("Scope: %s", knuu.Scope()) + err := knuu.Initialize() + if err != nil { + logrus.Fatalf("error initializing knuu: %v", err) + } + logrus.Infof("Scope: %s", knuu.Scope()) exitVal := m.Run() os.Exit(exitVal) } From 41fff2e769f1e40f1a336d265a1579265647fc47 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Tue, 25 Jun 2024 12:13:09 +0200 Subject: [PATCH 12/14] fix: linter complain on import sorting --- e2e/system/main_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/e2e/system/main_test.go b/e2e/system/main_test.go index 3e2709f8..b8f33f1e 100644 --- a/e2e/system/main_test.go +++ b/e2e/system/main_test.go @@ -4,8 +4,9 @@ import ( "os" "testing" - "github.com/celestiaorg/knuu/pkg/knuu" "github.com/sirupsen/logrus" + + "github.com/celestiaorg/knuu/pkg/knuu" ) func TestMain(m *testing.M) { From dd6c3785fe5300fb9afe0e2135ad447eae45ba36 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Tue, 25 Jun 2024 15:34:53 +0200 Subject: [PATCH 13/14] Update pkg/knuu/knuu.go Co-authored-by: Matthew Sevey <15232757+MSevey@users.noreply.github.com> --- pkg/knuu/knuu.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go index 81bb3c16..ab1b7b74 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -171,8 +171,8 @@ func validateOptions(opts Options) error { return ErrK8sClientNotSet } - if opts.TestScope == "" && opts.K8sClient == nil { - return ErrTestScopeNotSet + if opts.TestScope != "" && opts.K8sClient != nil && opts.TestScope != opts.K8sClient.Namespace() { + return ErrTestScopeMistMatch } return nil } From ba554f438e619dfcec79370aeb5b0f74a5843c27 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Tue, 25 Jun 2024 17:04:34 +0200 Subject: [PATCH 14/14] fix: applied latest feedback --- e2e/bittwister/suite_setup_test.go | 2 - e2e/system/build_from_git_test.go | 5 +-- e2e/tshark/tshark_test.go | 3 +- e2e/utils.go | 11 ----- pkg/knuu/errors.go | 1 + pkg/knuu/knuu.go | 19 +++++--- pkg/knuu/knuu_test.go | 71 ++++++++++++++++-------------- 7 files changed, 56 insertions(+), 56 deletions(-) delete mode 100644 e2e/utils.go diff --git a/e2e/bittwister/suite_setup_test.go b/e2e/bittwister/suite_setup_test.go index 8dfffded..213fe883 100644 --- a/e2e/bittwister/suite_setup_test.go +++ b/e2e/bittwister/suite_setup_test.go @@ -7,7 +7,6 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/suite" - "github.com/celestiaorg/knuu/e2e" "github.com/celestiaorg/knuu/pkg/knuu" ) @@ -22,7 +21,6 @@ func (s *Suite) SetupSuite() { var err error s.Knuu, err = knuu.New(ctx, knuu.Options{ ProxyEnabled: true, - TestScope: e2e.DefaultTestScope(), }) s.Require().NoError(err) s.T().Logf("Scope: %s", s.Knuu.Scope()) diff --git a/e2e/system/build_from_git_test.go b/e2e/system/build_from_git_test.go index b5075921..03975f39 100644 --- a/e2e/system/build_from_git_test.go +++ b/e2e/system/build_from_git_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/celestiaorg/knuu/e2e" "github.com/celestiaorg/knuu/pkg/builder" "github.com/celestiaorg/knuu/pkg/instance" "github.com/celestiaorg/knuu/pkg/k8s" @@ -23,7 +22,7 @@ func TestBuildFromGit(t *testing.T) { ctx := context.Background() // The default image builder is kaniko here - kn, err := knuu.New(ctx, knuu.Options{TestScope: e2e.DefaultTestScope()}) + kn, err := knuu.New(ctx, knuu.Options{}) require.NoError(t, err, "Error creating knuu") target, err := kn.NewInstance("git-builder") @@ -70,7 +69,7 @@ func TestBuildFromGitWithModifications(t *testing.T) { // Setup ctx := context.Background() - k8sClient, err := k8s.NewClient(ctx, e2e.DefaultTestScope()) + k8sClient, err := k8s.NewClient(ctx, knuu.DefaultTestScope()) require.NoError(t, err, "Error creating k8s client") // Since we are modifying the git repo, diff --git a/e2e/tshark/tshark_test.go b/e2e/tshark/tshark_test.go index 979209d7..0d834f5d 100644 --- a/e2e/tshark/tshark_test.go +++ b/e2e/tshark/tshark_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/celestiaorg/knuu/e2e" "github.com/celestiaorg/knuu/pkg/instance" "github.com/celestiaorg/knuu/pkg/k8s" "github.com/celestiaorg/knuu/pkg/knuu" @@ -29,7 +28,7 @@ func TestTshark(t *testing.T) { ctx := context.Background() - k8sClient, err := k8s.NewClient(ctx, e2e.DefaultTestScope()) + k8sClient, err := k8s.NewClient(ctx, knuu.DefaultTestScope()) require.NoError(t, err, "error creating k8s client") minioClient, err := minio.New(ctx, k8sClient) diff --git a/e2e/utils.go b/e2e/utils.go deleted file mode 100644 index 51f2297a..00000000 --- a/e2e/utils.go +++ /dev/null @@ -1,11 +0,0 @@ -package e2e - -import ( - "fmt" - "time" -) - -func DefaultTestScope() string { - t := time.Now() - return fmt.Sprintf("%s-%03d", t.Format("20060102-150405"), t.Nanosecond()/1e6) -} diff --git a/pkg/knuu/errors.go b/pkg/knuu/errors.go index eb723825..d2be8969 100644 --- a/pkg/knuu/errors.go +++ b/pkg/knuu/errors.go @@ -207,4 +207,5 @@ var ( ErrTraefikAPINotAvailable = errors.New("TraefikAPINotAvailable", "traefik API is not available") ErrTestScopeNotSet = errors.New("TestScopeNotSet", "test scope is not set") ErrK8sClientNotSet = errors.New("K8sClientNotSet", "k8s client is not set") + ErrTestScopeMistMatch = errors.New("TestScopeMistMatch", "test scope '%s' set in options does not match scope '%s' set by the k8sClient namespace") ) diff --git a/pkg/knuu/knuu.go b/pkg/knuu/knuu.go index ab1b7b74..b960733c 100644 --- a/pkg/knuu/knuu.go +++ b/pkg/knuu/knuu.go @@ -40,9 +40,9 @@ type Knuu struct { type Options struct { K8sClient k8s.KubeManager - TestScope string - ImageBuilder builder.Builder MinioClient *minio.Minio + ImageBuilder builder.Builder + TestScope string ProxyEnabled bool Timeout time.Duration Logger *logrus.Logger @@ -164,6 +164,11 @@ func (k *Knuu) handleTimeout(ctx context.Context) error { return nil } +func DefaultTestScope() string { + t := time.Now() + return fmt.Sprintf("%s-%03d", t.Format("20060102-150405"), t.Nanosecond()/1e6) +} + func validateOptions(opts Options) error { // When Minio is set, K8sClient must be set too // to make sure that there is only one source of truth for the k8s client @@ -172,7 +177,7 @@ func validateOptions(opts Options) error { } if opts.TestScope != "" && opts.K8sClient != nil && opts.TestScope != opts.K8sClient.Namespace() { - return ErrTestScopeMistMatch + return ErrTestScopeMistMatch.WithParams(opts.TestScope, opts.K8sClient.Namespace()) } return nil } @@ -193,8 +198,12 @@ func setDefaults(ctx context.Context, k *Knuu) error { k.Logger = log.DefaultLogger() } - if k.TestScope == "" && k.K8sClient != nil { - k.TestScope = k.K8sClient.Namespace() + if k.TestScope == "" { + if k.K8sClient != nil { + k.TestScope = k.K8sClient.Namespace() + } else { + k.TestScope = DefaultTestScope() + } } k.TestScope = k8s.SanitizeName(k.TestScope) diff --git a/pkg/knuu/knuu_test.go b/pkg/knuu/knuu_test.go index ee7ebf26..3e5b521d 100644 --- a/pkg/knuu/knuu_test.go +++ b/pkg/knuu/knuu_test.go @@ -59,35 +59,36 @@ func TestNew(t *testing.T) { defer cancel() tt := []struct { - name string - options Options - expectError bool - validateFunc func(*testing.T, *Knuu) + name string + options Options + expectedError error + validateFunc func(*testing.T, *Knuu) }{ { - name: "Default initialization", - options: Options{TestScope: "test"}, - expectError: false, + name: "Default initialization", + options: Options{}, + expectedError: nil, validateFunc: func(t *testing.T, k *Knuu) { assert.NotNil(t, k) assert.NotNil(t, k.Logger) assert.NotNil(t, k.K8sClient) assert.NotNil(t, k.ImageBuilder) + assert.NotEmpty(t, k.TestScope) assert.Equal(t, defaultTimeout, k.timeout) }, }, { - name: "With custom Minio client without setting k8sClient", - options: Options{MinioClient: &minio.Minio{}}, - expectError: true, + name: "With Minio client without setting k8sClient", + options: Options{MinioClient: &minio.Minio{}}, + expectedError: ErrK8sClientNotSet, }, { - name: "With custom Minio client and K8sClient", + name: "With Minio client and K8sClient", options: Options{ MinioClient: &minio.Minio{}, K8sClient: &mockK8s{}, }, - expectError: false, + expectedError: nil, validateFunc: func(t *testing.T, k *Knuu) { assert.NotNil(t, k) assert.NotNil(t, k.MinioClient) @@ -100,7 +101,7 @@ func TestNew(t *testing.T) { TestScope: "test", Logger: &logrus.Logger{}, }, - expectError: false, + expectedError: nil, validateFunc: func(t *testing.T, k *Knuu) { assert.NotNil(t, k) assert.NotNil(t, k.Logger) @@ -112,7 +113,7 @@ func TestNew(t *testing.T) { TestScope: "test", Timeout: 30 * time.Minute, }, - expectError: false, + expectedError: nil, validateFunc: func(t *testing.T, k *Knuu) { assert.NotNil(t, k) assert.Equal(t, 30*time.Minute, k.timeout) @@ -124,23 +125,18 @@ func TestNew(t *testing.T) { TestScope: "test", ImageBuilder: &kaniko.Kaniko{}, }, - expectError: false, + expectedError: nil, validateFunc: func(t *testing.T, k *Knuu) { assert.NotNil(t, k) assert.NotNil(t, k.ImageBuilder) }, }, - { - name: "Without TestScope and K8sClient", - options: Options{}, - expectError: true, - }, { name: "With K8sClient but without TestScope", options: Options{ K8sClient: &mockK8s{}, }, - expectError: false, + expectedError: nil, validateFunc: func(t *testing.T, k *Knuu) { assert.NotNil(t, k) assert.Equal(t, "test", k.TestScope) @@ -151,8 +147,9 @@ func TestNew(t *testing.T) { for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { k, err := New(ctx, tc.options) - if tc.expectError { + if tc.expectedError != nil { assert.Error(t, err) + assert.ErrorIs(t, err, tc.expectedError) return } @@ -164,16 +161,16 @@ func TestNew(t *testing.T) { func TestValidateOptions(t *testing.T) { tests := []struct { - name string - options Options - wantErr bool + name string + options Options + expectedErr error }{ { name: "MinioClient set without K8sClient", options: Options{ MinioClient: &minio.Minio{}, }, - wantErr: true, + expectedErr: ErrK8sClientNotSet, }, { name: "Both MinioClient and K8sClient set", @@ -181,7 +178,7 @@ func TestValidateOptions(t *testing.T) { MinioClient: &minio.Minio{}, K8sClient: &mockK8s{}, }, - wantErr: false, + expectedErr: nil, }, { name: "TestScope and K8sClient not set", @@ -189,23 +186,31 @@ func TestValidateOptions(t *testing.T) { TestScope: "", K8sClient: nil, }, - wantErr: true, + expectedErr: nil, }, { - name: "Valid options", + name: "TestScope does not match K8sClient namespace", options: Options{ - TestScope: "test", + TestScope: "another_scope", + K8sClient: &mockK8s{}, }, - wantErr: false, + expectedErr: ErrTestScopeMistMatch.WithParams("another_scope", "test"), + }, + { + name: "No options set", + options: Options{}, + expectedErr: nil, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { err := validateOptions(tt.options) - if (err != nil) != tt.wantErr { - t.Errorf("validateOptions() error = %v, wantErr %v", err, tt.wantErr) + if tt.expectedErr != nil { + assert.ErrorIs(t, err, tt.expectedErr) + return } + assert.NoError(t, err) }) } }