diff --git a/.golangci.yml b/.golangci.yml index 4ef851d..44e084f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,32 +1,14 @@ +version: "2" run: - timeout: 5m allow-parallel-runners: true - -issues: - # don't skip warning about doc comments - # don't exclude the default set of lint - exclude-use-default: false - # restore some of the defaults - # (fill in the rest as needed) - exclude-rules: - - path: "api/*" - linters: - - lll - - path: "internal/*" - linters: - - dupl linters: - disable-all: true + default: none enable: - dupl - errcheck - - exportloopref - ginkgolinter - goconst - gocyclo - - gofmt - - goimports - - gosimple - govet - ineffassign - misspell @@ -34,12 +16,36 @@ linters: - prealloc - revive - staticcheck - - typecheck - unconvert - unparam - unused - -linters-settings: - revive: + settings: + revive: + rules: + - name: comment-spacings + exclusions: + generated: lax rules: - - name: comment-spacings + - linters: + - goconst + path: _test\.go + - linters: + - lll + path: api/* + - linters: + - dupl + path: internal/* + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + enable: + - gofmt + - goimports + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/api/raczylo.com/v1/clusterimage_types.go b/api/raczylo.com/v1/clusterimage_types.go index c31e30e..8c2c3a8 100644 --- a/api/raczylo.com/v1/clusterimage_types.go +++ b/api/raczylo.com/v1/clusterimage_types.go @@ -19,6 +19,7 @@ package v1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" ) // ClusterImageSpec defines the desired state of ClusterImage @@ -81,5 +82,8 @@ type ClusterImageList struct { } func init() { - SchemeBuilder.Register(&ClusterImage{}, &ClusterImageList{}) + SchemeBuilder.Register(func(s *runtime.Scheme) error { + s.AddKnownTypes(GroupVersion, &ClusterImage{}, &ClusterImageList{}) + return nil + }) } diff --git a/api/raczylo.com/v1/clusterimageexport_types.go b/api/raczylo.com/v1/clusterimageexport_types.go index 1a18478..e1b0bba 100644 --- a/api/raczylo.com/v1/clusterimageexport_types.go +++ b/api/raczylo.com/v1/clusterimageexport_types.go @@ -19,6 +19,7 @@ package v1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" ) // RetentionPolicy defines how many completed ClusterImageExport resources to keep @@ -136,5 +137,8 @@ type ClusterImageExportList struct { } func init() { - SchemeBuilder.Register(&ClusterImageExport{}, &ClusterImageExportList{}) + SchemeBuilder.Register(func(s *runtime.Scheme) error { + s.AddKnownTypes(GroupVersion, &ClusterImageExport{}, &ClusterImageExportList{}) + return nil + }) } diff --git a/api/raczylo.com/v1/groupversion_info.go b/api/raczylo.com/v1/groupversion_info.go index d689874..1219b94 100644 --- a/api/raczylo.com/v1/groupversion_info.go +++ b/api/raczylo.com/v1/groupversion_info.go @@ -20,8 +20,9 @@ limitations under the License. package v1 import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/scheme" ) var ( @@ -29,7 +30,10 @@ var ( GroupVersion = schema.GroupVersion{Group: "raczylo.com", Version: "v1"} // SchemeBuilder is used to add go types to the GroupVersionKind scheme - SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} + SchemeBuilder = runtime.NewSchemeBuilder(func(s *runtime.Scheme) error { + metav1.AddToGroupVersion(s, GroupVersion) + return nil + }) // AddToScheme adds the types in this group-version to the given scheme. AddToScheme = SchemeBuilder.AddToScheme diff --git a/cmd/worker/main.go b/cmd/worker/main.go index c61b557..3d7aac9 100644 --- a/cmd/worker/main.go +++ b/cmd/worker/main.go @@ -233,7 +233,7 @@ func uploadToS3(source, destination string) error { if err != nil { return fmt.Errorf("failed to open source file: %w", err) } - defer file.Close() + defer func() { _ = file.Close() }() fmt.Printf("Uploading %s to s3://%s/%s\n", source, bucket, key) @@ -261,7 +261,7 @@ func copyLocal(source, destination string) error { if err != nil { return fmt.Errorf("failed to open source file: %w", err) } - defer srcFile.Close() + defer func() { _ = srcFile.Close() }() // Get source file info for permissions srcInfo, err := srcFile.Stat() @@ -274,7 +274,7 @@ func copyLocal(source, destination string) error { if err != nil { return fmt.Errorf("failed to create destination file: %w", err) } - defer dstFile.Close() + defer func() { _ = dstFile.Close() }() // Copy content if _, err := io.Copy(dstFile, srcFile); err != nil { diff --git a/internal/controller/raczylo.com/clusterimage_controller.go b/internal/controller/raczylo.com/clusterimage_controller.go index ae287a4..45694ce 100644 --- a/internal/controller/raczylo.com/clusterimage_controller.go +++ b/internal/controller/raczylo.com/clusterimage_controller.go @@ -144,7 +144,7 @@ func (r *ClusterImageReconciler) handlePendingClusterImage(ctx context.Context, } // Create the backup job - if err := r.createBackupJob(ctx, clusterImage, clusterImageExport, l); err != nil { + if err := r.createBackupJob(ctx, clusterImage, clusterImageExport); err != nil { l.Error(err, "unable to create backup job") return ctrl.Result{}, err } @@ -349,7 +349,7 @@ func (r *ClusterImageReconciler) cleanupJobAndPods(ctx context.Context, job *v1b return nil } -func (r *ClusterImageReconciler) createBackupJob(ctx context.Context, clusterImage *raczylocomv1.ClusterImage, clusterImageExport *raczylocomv1.ClusterImageExport, l logr.Logger) error { +func (r *ClusterImageReconciler) createBackupJob(ctx context.Context, clusterImage *raczylocomv1.ClusterImage, clusterImageExport *raczylocomv1.ClusterImageExport) error { normalisedImageName := shared.NormalizeImageName(clusterImage.Spec.FullName) defaultCommands := []string{ @@ -357,13 +357,14 @@ func (r *ClusterImageReconciler) createBackupJob(ctx context.Context, clusterIma "podman save --quiet -o /tmp/" + normalisedImageName + ".tar " + clusterImage.Spec.FullName, } - if clusterImage.Spec.Storage == shared.STORAGE_S3 { + switch clusterImage.Spec.Storage { + case shared.STORAGE_S3: s3Params := shared.SetupS3Params(clusterImageExport.Spec.Storage.S3) additionalCommands := []string{ "./worker export " + strings.Join(s3Params, " ") + " '/tmp/" + normalisedImageName + ".tar' " + "'s3://" + clusterImageExport.Spec.Storage.S3.Bucket + clusterImage.Spec.ExportPath + "/" + clusterImage.Spec.ExportName + "/" + normalisedImageName + ".tar'", } defaultCommands = append(defaultCommands, additionalCommands...) - } else if clusterImage.Spec.Storage == shared.STORAGE_FILE { + case shared.STORAGE_FILE: additionalCommands := []string{ "./worker export '/tmp/" + normalisedImageName + ".tar' '" + clusterImage.Spec.ExportPath + "/" + clusterImage.Spec.ExportName + "/" + normalisedImageName + ".tar'", } @@ -426,7 +427,7 @@ func (r *ClusterImageReconciler) updateClusterImageExportStatus(ctx context.Cont } clusterImageList := &raczylocomv1.ClusterImageList{} - if err := r.List(ctx, clusterImageList, client.InNamespace(clusterImage.Namespace), client.MatchingFields{"spec.exportName": clusterImage.Spec.ExportName}); err != nil { + if err := r.List(ctx, clusterImageList, client.InNamespace(clusterImage.Namespace), client.MatchingFields{shared.FIELD_EXPORT_NAME: clusterImage.Spec.ExportName}); err != nil { l.Error(err, "unable to list ClusterImages") return ctrl.Result{}, err } diff --git a/internal/controller/raczylo.com/clusterimageexport_controller.go b/internal/controller/raczylo.com/clusterimageexport_controller.go index c4c3330..05377e8 100644 --- a/internal/controller/raczylo.com/clusterimageexport_controller.go +++ b/internal/controller/raczylo.com/clusterimageexport_controller.go @@ -7,7 +7,6 @@ import ( "strings" "time" - "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -48,6 +47,7 @@ func (r *ClusterImageExportReconciler) InjectPodAnnotations(annotations map[stri const clusterImageExportFinalizer = "raczylo.com/clusterimageexport-finalizer" +//nolint:gocyclo // single reconcile state machine; splitting is out of scope func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { l := log.FromContext(ctx) // l.Info("Reconciling ClusterImageExport") @@ -58,7 +58,7 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, client.IgnoreNotFound(err) } - if !clusterImageExport.ObjectMeta.DeletionTimestamp.IsZero() { + if !clusterImageExport.DeletionTimestamp.IsZero() { return r.handleDeletion(ctx, clusterImageExport) } @@ -99,7 +99,7 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R // Proceed with reconciliation logic // Get list of all images to be exported - fullImagesList, err := r.listImagesInCluster(ctx, l, clusterImageExport) + fullImagesList, err := r.listImagesInCluster(ctx, clusterImageExport) if err != nil { l.Error(err, "unable to list images in the cluster") return ctrl.Result{}, err @@ -201,7 +201,7 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R pendingCount := 0 clusterImageList := &raczylocomv1.ClusterImageList{} if err := r.List(ctx, clusterImageList, client.InNamespace(clusterImageExport.Namespace), - client.MatchingFields{"spec.exportName": clusterImageExport.Name}); err != nil { + client.MatchingFields{shared.FIELD_EXPORT_NAME: clusterImageExport.Name}); err != nil { l.Error(err, "unable to list ClusterImages") return ctrl.Result{}, err } @@ -288,7 +288,7 @@ func (r *ClusterImageExportReconciler) SetupWithManager(mgr ctrl.Manager) error Complete(r) } -func (r *ClusterImageExportReconciler) listImagesInCluster(ctx context.Context, l logr.Logger, clusterImageExport *raczylocomv1.ClusterImageExport) (shared.ContainersList, error) { +func (r *ClusterImageExportReconciler) listImagesInCluster(ctx context.Context, clusterImageExport *raczylocomv1.ClusterImageExport) (shared.ContainersList, error) { containersList := shared.ContainersList{} if err := shared.ListAndProcessResources[*shared.DeploymentWrapper](ctx, r.Client, &appsv1.DeploymentList{}, &containersList); err != nil { return shared.ContainersList{}, err @@ -388,7 +388,7 @@ func (r *ClusterImageExportReconciler) deleteAssociatedClusterImages(ctx context // List all ClusterImages associated with this export clusterImageList := &raczylocomv1.ClusterImageList{} if err := r.List(ctx, clusterImageList, client.InNamespace(clusterImageExport.Namespace), - client.MatchingFields{"spec.exportName": clusterImageExport.Name}); err != nil { + client.MatchingFields{shared.FIELD_EXPORT_NAME: clusterImageExport.Name}); err != nil { return fmt.Errorf("failed to list ClusterImages: %w", err) } @@ -409,15 +409,16 @@ func (r *ClusterImageExportReconciler) runCleanupJob(ctx context.Context, cluste defaultCommands := []string{} - if clusterImageExport.Spec.Storage.StorageTarget == shared.STORAGE_S3 { + switch clusterImageExport.Spec.Storage.StorageTarget { + case shared.STORAGE_S3: s3Params := shared.SetupS3Params(clusterImageExport.Spec.Storage.S3) additionalCommands := []string{ - "./worker cleanup " + strings.Join(s3Params, " ") + " 's3://" + clusterImageExport.Spec.Storage.S3.Bucket + clusterImageExport.Spec.BasePath + "/" + clusterImageExport.ObjectMeta.Name + "/'", + "./worker cleanup " + strings.Join(s3Params, " ") + " 's3://" + clusterImageExport.Spec.Storage.S3.Bucket + clusterImageExport.Spec.BasePath + "/" + clusterImageExport.Name + "/'", } defaultCommands = append(defaultCommands, additionalCommands...) - } else if clusterImageExport.Spec.Storage.StorageTarget == shared.STORAGE_FILE { + case shared.STORAGE_FILE: additionalCommands := []string{ - "./worker cleanup '" + clusterImageExport.Spec.BasePath + "/" + clusterImageExport.ObjectMeta.Name + "/'", + "./worker cleanup '" + clusterImageExport.Spec.BasePath + "/" + clusterImageExport.Name + "/'", } defaultCommands = append(defaultCommands, additionalCommands...) } @@ -534,9 +535,10 @@ func (r *ClusterImageExportReconciler) cleanupByRetention(ctx context.Context, c export.Status.Progress != shared.STATUS_FAILED { continue } - if export.Status.Progress == shared.STATUS_SUCCESS { + switch export.Status.Progress { + case shared.STATUS_SUCCESS: successfulExports = append(successfulExports, export) - } else if export.Status.Progress == shared.STATUS_FAILED { + case shared.STATUS_FAILED: failedExports = append(failedExports, export) } } diff --git a/internal/controller/raczylo.com/controller_unit_test.go b/internal/controller/raczylo.com/controller_unit_test.go index 2897b35..a4ba034 100644 --- a/internal/controller/raczylo.com/controller_unit_test.go +++ b/internal/controller/raczylo.com/controller_unit_test.go @@ -24,6 +24,8 @@ import ( type TestScenario string +const testNamespace = "default" + const ( ScenarioGood TestScenario = "good" ScenarioNotGood TestScenario = "not_good" @@ -60,7 +62,7 @@ func (s *ControllerTestSuite) newFakeClient(objs ...client.Object) client.Client } // Helper to create a test ClusterImageExport -func (s *ControllerTestSuite) createClusterImageExport(name, namespace string, opts ...func(*raczylocomv1.ClusterImageExport)) *raczylocomv1.ClusterImageExport { +func (s *ControllerTestSuite) createClusterImageExport(name string, opts ...func(*raczylocomv1.ClusterImageExport)) *raczylocomv1.ClusterImageExport { export := &raczylocomv1.ClusterImageExport{ TypeMeta: metav1.TypeMeta{ APIVersion: "raczylo.com/v1", @@ -68,7 +70,7 @@ func (s *ControllerTestSuite) createClusterImageExport(name, namespace string, o }, ObjectMeta: metav1.ObjectMeta{ Name: name, - Namespace: namespace, + Namespace: testNamespace, UID: "test-uid-export", }, Spec: raczylocomv1.ClusterImageExportSpec{ @@ -91,7 +93,7 @@ func (s *ControllerTestSuite) createClusterImageExport(name, namespace string, o } // Helper to create a test ClusterImage -func (s *ControllerTestSuite) createClusterImage(name, namespace, exportName string, opts ...func(*raczylocomv1.ClusterImage)) *raczylocomv1.ClusterImage { +func (s *ControllerTestSuite) createClusterImage(name, exportName string, opts ...func(*raczylocomv1.ClusterImage)) *raczylocomv1.ClusterImage { image := &raczylocomv1.ClusterImage{ TypeMeta: metav1.TypeMeta{ APIVersion: "raczylo.com/v1", @@ -99,7 +101,7 @@ func (s *ControllerTestSuite) createClusterImage(name, namespace, exportName str }, ObjectMeta: metav1.ObjectMeta{ Name: name, - Namespace: namespace, + Namespace: testNamespace, UID: "test-uid-image", }, Spec: raczylocomv1.ClusterImageSpec{ @@ -141,8 +143,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_NotFound() { func (s *ControllerTestSuite) TestClusterImageReconcile_InitialStatus_Pending() { // Scenario: Good - new ClusterImage should be set to PENDING - export := s.createClusterImageExport("test-export", "default") - image := s.createClusterImage("test-image", "default", "test-export") + export := s.createClusterImageExport("test-export") + image := s.createClusterImage("test-image", "test-export") client := s.newFakeClient(export, image) reconciler := &ClusterImageReconciler{ @@ -160,7 +162,7 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_InitialStatus_Pending() result, err := reconciler.Reconcile(s.ctx, req) require.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior // Verify status was updated to PENDING updatedImage := &raczylocomv1.ClusterImage{} @@ -171,7 +173,7 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_InitialStatus_Pending() func (s *ControllerTestSuite) TestClusterImageReconcile_MissingExport() { // Scenario: Not Good - ClusterImageExport doesn't exist - image := s.createClusterImage("test-image", "default", "non-existent-export", func(i *raczylocomv1.ClusterImage) { + image := s.createClusterImage("test-image", "non-existent-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_PENDING }) @@ -196,8 +198,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MissingExport() { func (s *ControllerTestSuite) TestClusterImageReconcile_MaxParallelJobsReached() { // Scenario: Good - should requeue when max jobs reached - export := s.createClusterImageExport("test-export", "default") - image := s.createClusterImage("test-image", "default", "test-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("test-export") + image := s.createClusterImage("test-image", "test-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_PENDING }) @@ -223,8 +225,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MaxParallelJobsReached() func (s *ControllerTestSuite) TestClusterImageReconcile_SuccessStatus() { // Scenario: Good - success status should not trigger further action - export := s.createClusterImageExport("test-export", "default") - image := s.createClusterImage("test-image", "default", "test-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("test-export") + image := s.createClusterImage("test-image", "test-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_SUCCESS }) @@ -248,8 +250,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_SuccessStatus() { func (s *ControllerTestSuite) TestClusterImageReconcile_FailedStatus() { // Scenario: Good - failed status should not trigger further action - export := s.createClusterImageExport("test-export", "default") - image := s.createClusterImage("test-image", "default", "test-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("test-export") + image := s.createClusterImage("test-image", "test-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_FAILED }) @@ -273,8 +275,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_FailedStatus() { func (s *ControllerTestSuite) TestClusterImageReconcile_PresentStatus() { // Scenario: Good - present status should not trigger further action - export := s.createClusterImageExport("test-export", "default") - image := s.createClusterImage("test-image", "default", "test-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("test-export") + image := s.createClusterImage("test-image", "test-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_PRESENT }) @@ -320,7 +322,7 @@ func (s *ControllerTestSuite) TestClusterImageExportReconcile_NotFound() { func (s *ControllerTestSuite) TestClusterImageExportReconcile_AddFinalizer() { // Scenario: Good - should add finalizer to new export - export := s.createClusterImageExport("test-export", "default") + export := s.createClusterImageExport("test-export") client := s.newFakeClient(export) reconciler := &ClusterImageExportReconciler{ @@ -347,7 +349,7 @@ func (s *ControllerTestSuite) TestClusterImageExportReconcile_AddFinalizer() { func (s *ControllerTestSuite) TestClusterImageExportReconcile_AddCreationTimestamp() { // Scenario: Good - should add creation timestamp annotation - export := s.createClusterImageExport("test-export", "default") + export := s.createClusterImageExport("test-export") client := s.newFakeClient(export) reconciler := &ClusterImageExportReconciler{ @@ -405,8 +407,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MatrixScenarios() { Scenario: ScenarioGood, Description: "New ClusterImage should be initialized to PENDING", SetupFunc: func(s *ControllerTestSuite) (client.Client, *ClusterImageReconciler, reconcile.Request) { - export := s.createClusterImageExport("export-1", "default") - image := s.createClusterImage("image-1", "default", "export-1") + export := s.createClusterImageExport("export-1") + image := s.createClusterImage("image-1", "export-1") c := s.newFakeClient(export, image) r := &ClusterImageReconciler{Client: c, Scheme: s.scheme, MaxParallelJobs: 5} req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "image-1", Namespace: "default"}} @@ -420,7 +422,7 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MatrixScenarios() { Scenario: ScenarioNotGood, Description: "ClusterImage with missing export should error", SetupFunc: func(s *ControllerTestSuite) (client.Client, *ClusterImageReconciler, reconcile.Request) { - image := s.createClusterImage("orphan-image", "default", "missing-export", func(i *raczylocomv1.ClusterImage) { + image := s.createClusterImage("orphan-image", "missing-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_PENDING }) c := s.newFakeClient(image) @@ -435,8 +437,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MatrixScenarios() { Scenario: ScenarioGood, Description: "ClusterImage with SUCCESS status should not trigger action", SetupFunc: func(s *ControllerTestSuite) (client.Client, *ClusterImageReconciler, reconcile.Request) { - export := s.createClusterImageExport("export-2", "default") - image := s.createClusterImage("success-image", "default", "export-2", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("export-2") + image := s.createClusterImage("success-image", "export-2", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_SUCCESS }) c := s.newFakeClient(export, image) @@ -452,8 +454,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MatrixScenarios() { Scenario: ScenarioNotGood, Description: "ClusterImage with FAILED status should not trigger action", SetupFunc: func(s *ControllerTestSuite) (client.Client, *ClusterImageReconciler, reconcile.Request) { - export := s.createClusterImageExport("export-3", "default") - image := s.createClusterImage("failed-image", "default", "export-3", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("export-3") + image := s.createClusterImage("failed-image", "export-3", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_FAILED }) c := s.newFakeClient(export, image) @@ -507,8 +509,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MatrixScenarios() { func (s *ControllerTestSuite) TestClusterImage_ConcurrentUpdates() { // Scenario: Good - simulate concurrent reconciliation on PENDING status // This test verifies that multiple reconciliations don't corrupt state - export := s.createClusterImageExport("concurrent-export", "default") - image := s.createClusterImage("concurrent-image", "default", "concurrent-export") + export := s.createClusterImageExport("concurrent-export") + image := s.createClusterImage("concurrent-image", "concurrent-export") fakeClient := s.newFakeClient(export, image) reconciler := &ClusterImageReconciler{ @@ -527,7 +529,7 @@ func (s *ControllerTestSuite) TestClusterImage_ConcurrentUpdates() { // First reconciliation should set status to PENDING result, err := reconciler.Reconcile(s.ctx, req) assert.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior // Verify status was set finalImage := &raczylocomv1.ClusterImage{} @@ -578,7 +580,7 @@ func (s *ControllerTestSuite) TestClusterImage_ActiveJobsMutex() { func (s *ControllerTestSuite) TestClusterImageExport_WithDeletionTimestamp() { // Scenario: Good - export being deleted should trigger cleanup - export := s.createClusterImageExport("deleting-export", "default") + export := s.createClusterImageExport("deleting-export") now := metav1.Now() export.DeletionTimestamp = &now export.Finalizers = []string{clusterImageExportFinalizer} @@ -607,8 +609,8 @@ func (s *ControllerTestSuite) TestClusterImageExport_WithDeletionTimestamp() { func (s *ControllerTestSuite) TestClusterImage_SHAPinnedImages() { // Scenario: Good - SHA-pinned images should be handled correctly - export := s.createClusterImageExport("sha-export", "default") - image := s.createClusterImage("sha-image", "default", "sha-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("sha-export") + image := s.createClusterImage("sha-image", "sha-export", func(i *raczylocomv1.ClusterImage) { i.Spec.Image = "quay.io/cilium/cilium" i.Spec.Tag = "v1.18.4" i.Spec.Sha = "sha256:49d87af187eeeb9e9e3ec2bc6bd372261a0b5cb2d845659463ba7cc10fe9e45f" @@ -631,7 +633,7 @@ func (s *ControllerTestSuite) TestClusterImage_SHAPinnedImages() { result, err := reconciler.Reconcile(s.ctx, req) assert.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior } func (s *ControllerTestSuite) TestClusterImage_MultipleRegistries() { @@ -647,11 +649,12 @@ func (s *ControllerTestSuite) TestClusterImage_MultipleRegistries() { {"quay-image", "quay.io/coreos/etcd", "quay.io/coreos/etcd:v3.5.0"}, } - export := s.createClusterImageExport("multi-registry-export", "default") - objs := []client.Object{export} + export := s.createClusterImageExport("multi-registry-export") + objs := make([]client.Object, 0, 1+len(registries)) + objs = append(objs, export) for _, reg := range registries { - img := s.createClusterImage(reg.name, "default", "multi-registry-export", func(i *raczylocomv1.ClusterImage) { + img := s.createClusterImage(reg.name, "multi-registry-export", func(i *raczylocomv1.ClusterImage) { i.Spec.Image = reg.image i.Spec.FullName = reg.fullName }) @@ -675,7 +678,7 @@ func (s *ControllerTestSuite) TestClusterImage_MultipleRegistries() { result, err := reconciler.Reconcile(s.ctx, req) assert.NoError(s.T(), err, "Failed for registry: %s", reg.name) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior } } @@ -683,7 +686,7 @@ func (s *ControllerTestSuite) TestClusterImage_MultipleRegistries() { func (s *ControllerTestSuite) TestClusterImageExport_S3Storage() { // Scenario: Good - S3 storage configuration - export := s.createClusterImageExport("s3-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("s3-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.Storage = raczylocomv1.ClusterImageStorageSpec{ StorageTarget: shared.STORAGE_S3, S3: raczylocomv1.ClusterImageStorageS3{ @@ -721,7 +724,7 @@ func (s *ControllerTestSuite) TestClusterImageExport_S3Storage() { func (s *ControllerTestSuite) TestClusterImageExport_FileStorage() { // Scenario: Good - File storage configuration - export := s.createClusterImageExport("file-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("file-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.Storage = raczylocomv1.ClusterImageStorageSpec{ StorageTarget: shared.STORAGE_FILE, } @@ -749,7 +752,7 @@ func (s *ControllerTestSuite) TestClusterImageExport_FileStorage() { func (s *ControllerTestSuite) TestClusterImage_EmptySpec() { // Scenario: Really Bad - empty spec should be handled - export := s.createClusterImageExport("empty-spec-export", "default") + export := s.createClusterImageExport("empty-spec-export") image := &raczylocomv1.ClusterImage{ TypeMeta: metav1.TypeMeta{ APIVersion: "raczylo.com/v1", @@ -782,12 +785,12 @@ func (s *ControllerTestSuite) TestClusterImage_EmptySpec() { result, err := reconciler.Reconcile(s.ctx, req) // Should not error, just set to pending assert.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior } func (s *ControllerTestSuite) TestClusterImageExport_EmptyNamespaces() { // Scenario: Good - export with no namespace filters should process all - export := s.createClusterImageExport("all-ns-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("all-ns-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.Namespaces = []string{} e.Spec.ExcludedNamespaces = []string{} }) @@ -811,7 +814,7 @@ func (s *ControllerTestSuite) TestClusterImageExport_EmptyNamespaces() { func (s *ControllerTestSuite) TestClusterImageExport_WithAdditionalImages() { // Scenario: Good - export with additional images specified - export := s.createClusterImageExport("additional-images-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("additional-images-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.AdditionalImages = []string{ "nginx:1.21", "redis:7.0", @@ -838,7 +841,7 @@ func (s *ControllerTestSuite) TestClusterImageExport_WithAdditionalImages() { func (s *ControllerTestSuite) TestClusterImageExport_WithIncludesExcludes() { // Scenario: Good - export with include/exclude filters - export := s.createClusterImageExport("filtered-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("filtered-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.Includes = []string{"nginx", "redis"} e.Spec.Excludes = []string{"test", "dev"} e.Spec.Namespaces = []string{"production", "staging"} @@ -864,8 +867,8 @@ func (s *ControllerTestSuite) TestClusterImageExport_WithIncludesExcludes() { func (s *ControllerTestSuite) TestClusterImage_WithImagePullSecrets() { // Scenario: Good - image with pull secrets should work - export := s.createClusterImageExport("secret-export", "default") - image := s.createClusterImage("secret-image", "default", "secret-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("secret-export") + image := s.createClusterImage("secret-image", "secret-export", func(i *raczylocomv1.ClusterImage) { i.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ {Name: "docker-registry-secret"}, {Name: "gcr-json-key"}, @@ -888,17 +891,17 @@ func (s *ControllerTestSuite) TestClusterImage_WithImagePullSecrets() { result, err := reconciler.Reconcile(s.ctx, req) assert.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior } func (s *ControllerTestSuite) TestClusterImage_WithJobAnnotations() { // Scenario: Good - image with job annotations - export := s.createClusterImageExport("annotated-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("annotated-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.JobAnnotations = map[string]string{ "iam.amazonaws.com/role": "arn:aws:iam::123456789:role/BackupRole", } }) - image := s.createClusterImage("annotated-image", "default", "annotated-export", func(i *raczylocomv1.ClusterImage) { + image := s.createClusterImage("annotated-image", "annotated-export", func(i *raczylocomv1.ClusterImage) { i.Spec.JobAnnotations = map[string]string{ "custom/annotation": "value", } @@ -920,7 +923,7 @@ func (s *ControllerTestSuite) TestClusterImage_WithJobAnnotations() { result, err := reconciler.Reconcile(s.ctx, req) assert.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior } // ==================== Job Status Tests ==================== @@ -930,8 +933,8 @@ func (s *ControllerTestSuite) TestClusterImage_SuccessfulJobCompletion() { // Note: Full integration testing requires envtest for KubeClient // This test validates the basic state machine transitions - export := s.createClusterImageExport("job-success-export", "default") - image := s.createClusterImage("job-success-image", "default", "job-success-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("job-success-export") + image := s.createClusterImage("job-success-image", "job-success-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_SUCCESS // Already completed }) @@ -963,8 +966,8 @@ func (s *ControllerTestSuite) TestClusterImage_SuccessfulJobCompletion() { func (s *ControllerTestSuite) TestClusterImage_RetryCount() { // Scenario: Good - verify retry count is tracked properly - export := s.createClusterImageExport("retry-export", "default") - image := s.createClusterImage("retry-image", "default", "retry-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("retry-export") + image := s.createClusterImage("retry-image", "retry-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_FAILED // Terminal state i.Status.RetryCount = 2 }) @@ -997,8 +1000,8 @@ func (s *ControllerTestSuite) TestClusterImage_RetryCount() { func (s *ControllerTestSuite) TestClusterImage_MaxRetriesReached() { // Scenario: Not Good - max retries reached and FAILED status - export := s.createClusterImageExport("max-retry-export", "default") - image := s.createClusterImage("max-retry-image", "default", "max-retry-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("max-retry-export") + image := s.createClusterImage("max-retry-image", "max-retry-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_FAILED i.Status.RetryCount = 3 // Max retries reached }) diff --git a/internal/shared/definitions.go b/internal/shared/definitions.go index 6ed569c..42b23b7 100644 --- a/internal/shared/definitions.go +++ b/internal/shared/definitions.go @@ -29,6 +29,11 @@ const ( // STORAGE DEFINITIONS STORAGE_S3 = "S3" STORAGE_FILE = "FILE" + + FIELD_EXPORT_NAME = "spec.exportName" + DEFAULT_IMAGE_TAG = "latest" + LABEL_APP = "app" + EXPORTER_CONTAINER = "exporter" ) type Container struct { diff --git a/internal/shared/jobs.go b/internal/shared/jobs.go index 925580b..7da1c93 100644 --- a/internal/shared/jobs.go +++ b/internal/shared/jobs.go @@ -59,7 +59,7 @@ func CreateJob[T any](params JobParams, setupFunc func(T) []string) *batchv1.Job Namespace: params.Namespace, OwnerReferences: params.OwnerReferences, Labels: map[string]string{ - "app": "image-export", + LABEL_APP: "image-export", }, Annotations: params.Annotations, }, @@ -69,7 +69,7 @@ func CreateJob[T any](params JobParams, setupFunc func(T) []string) *batchv1.Job Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "app": "image-export", + LABEL_APP: "image-export", }, Annotations: params.Annotations, }, @@ -80,7 +80,7 @@ func CreateJob[T any](params JobParams, setupFunc func(T) []string) *batchv1.Job Volumes: volumes, Containers: []corev1.Container{ { - Name: "exporter", + Name: EXPORTER_CONTAINER, Image: params.Image, TTY: true, Command: []string{}, diff --git a/internal/shared/jobs_test.go b/internal/shared/jobs_test.go index e9e9331..b7ebe06 100644 --- a/internal/shared/jobs_test.go +++ b/internal/shared/jobs_test.go @@ -204,18 +204,18 @@ func (s *JobsTestSuite) TestCreateJob() { require.NotNil(s.T(), job, "Job should not be nil") // Verify job metadata - assert.Equal(s.T(), tc.expectJobName, job.ObjectMeta.Name) - assert.Equal(s.T(), tc.expectNamespace, job.ObjectMeta.Namespace) + assert.Equal(s.T(), tc.expectJobName, job.Name) + assert.Equal(s.T(), tc.expectNamespace, job.Namespace) // Verify labels - assert.Equal(s.T(), "image-export", job.ObjectMeta.Labels["app"]) - assert.Equal(s.T(), "image-export", job.Spec.Template.ObjectMeta.Labels["app"]) + assert.Equal(s.T(), "image-export", job.Labels["app"]) + assert.Equal(s.T(), "image-export", job.Spec.Template.Labels["app"]) // Verify annotations if expected if tc.expectAnnotations != nil { for k, v := range tc.expectAnnotations { - assert.Equal(s.T(), v, job.ObjectMeta.Annotations[k]) - assert.Equal(s.T(), v, job.Spec.Template.ObjectMeta.Annotations[k]) + assert.Equal(s.T(), v, job.Annotations[k]) + assert.Equal(s.T(), v, job.Spec.Template.Annotations[k]) } } @@ -268,9 +268,9 @@ func (s *JobsTestSuite) TestCreateJobWithOwnerReferences() { job := CreateJob(params, func(raczylocomv1.ClusterImageExport) []string { return nil }) - require.Len(s.T(), job.ObjectMeta.OwnerReferences, 1) - assert.Equal(s.T(), "ClusterImage", job.ObjectMeta.OwnerReferences[0].Kind) - assert.Equal(s.T(), "test-image", job.ObjectMeta.OwnerReferences[0].Name) + require.Len(s.T(), job.OwnerReferences, 1) + assert.Equal(s.T(), "ClusterImage", job.OwnerReferences[0].Kind) + assert.Equal(s.T(), "test-image", job.OwnerReferences[0].Name) } // TestCreateJobCommands tests command concatenation @@ -525,7 +525,7 @@ func (s *JobsTestSuite) TestSecretVolumeMounting() { // Verify volumes are created require.Len(s.T(), podSpec.Volumes, 3) for i, vol := range podSpec.Volumes { - assert.Equal(s.T(), secrets[i].Name, vol.VolumeSource.Secret.SecretName) + assert.Equal(s.T(), secrets[i].Name, vol.Secret.SecretName) assert.Contains(s.T(), vol.Name, "secret-") } diff --git a/internal/shared/k8s.go b/internal/shared/k8s.go index 51c22eb..cbe8749 100644 --- a/internal/shared/k8s.go +++ b/internal/shared/k8s.go @@ -84,7 +84,7 @@ func ProcessContainerName(containerName string) (Container, error) { } if cnt.Sha == "" && cnt.Tag == "" { - cnt.Tag = "latest" + cnt.Tag = DEFAULT_IMAGE_TAG } if cnt.Image == "" { @@ -109,7 +109,7 @@ func processContainers(ctx context.Context, resource K8sResource, namespace stri } for _, container := range podSpec.EphemeralContainers { - if err := processContainer(ctx, container.EphemeralContainerCommon.Image, namespace, containersList); err != nil { + if err := processContainer(ctx, container.Image, namespace, containersList); err != nil { return err } } @@ -211,7 +211,7 @@ func SetupIndexers(mgr manager.Manager) error { Jitter: 0.1, Steps: 5, }, func() (bool, error) { - err := mgr.GetFieldIndexer().IndexField(context.Background(), &raczylocomv1.ClusterImage{}, "spec.exportName", func(rawObj client.Object) []string { + err := mgr.GetFieldIndexer().IndexField(context.Background(), &raczylocomv1.ClusterImage{}, FIELD_EXPORT_NAME, func(rawObj client.Object) []string { clusterImage := rawObj.(*raczylocomv1.ClusterImage) return []string{clusterImage.Spec.ExportName} })