diff --git a/.golangci.yaml b/.golangci.yaml index 4faccee..0ad9b7c 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -114,6 +114,7 @@ issues: # we need refactoring to enable this linter - linters: - cyclop + - funlen text: "Reconcile" # Independently of option `exclude` we use default exclude patterns, diff --git a/controllers/cdpipeline/cdpipeline_controller.go b/controllers/cdpipeline/cdpipeline_controller.go index a2d5d59..0c5aa9a 100644 --- a/controllers/cdpipeline/cdpipeline_controller.go +++ b/controllers/cdpipeline/cdpipeline_controller.go @@ -6,7 +6,6 @@ import ( "reflect" "time" - "github.com/go-logr/logr" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -22,18 +21,16 @@ import ( "github.com/epam/edp-cd-pipeline-operator/v2/pkg/util/consts" ) -func NewReconcileCDPipeline(c client.Client, scheme *runtime.Scheme, log logr.Logger) *ReconcileCDPipeline { +func NewReconcileCDPipeline(c client.Client, scheme *runtime.Scheme) *ReconcileCDPipeline { return &ReconcileCDPipeline{ client: c, scheme: scheme, - log: log.WithName("cd-pipeline"), } } type ReconcileCDPipeline struct { client client.Client scheme *runtime.Scheme - log logr.Logger } const ( diff --git a/controllers/cdpipeline/cdpipeline_controller_test.go b/controllers/cdpipeline/cdpipeline_controller_test.go index d64fd38..cb69a74 100644 --- a/controllers/cdpipeline/cdpipeline_controller_test.go +++ b/controllers/cdpipeline/cdpipeline_controller_test.go @@ -54,15 +54,13 @@ func createScheme(t *testing.T) *runtime.Scheme { func TestNewReconcileCDPipeline_Success(t *testing.T) { scheme := runtime.NewScheme() client := fake.NewClientBuilder().Build() - log := logr.Discard() expectedReconcileCdPipeline := &ReconcileCDPipeline{ client: client, scheme: scheme, - log: log.WithName("cd-pipeline"), } - reconciledCdPipeline := NewReconcileCDPipeline(client, scheme, log) + reconciledCdPipeline := NewReconcileCDPipeline(client, scheme) assert.Equal(t, expectedReconcileCdPipeline, reconciledCdPipeline) } @@ -71,7 +69,7 @@ func TestReconcile_Success(t *testing.T) { scheme := createScheme(t) client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(emptyCdPipeline).Build() - reconcileCDPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard()) + reconcileCDPipeline := NewReconcileCDPipeline(client, scheme) _, err := reconcileCDPipeline.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{ Namespace: namespace, @@ -95,7 +93,7 @@ func TestReconcile_PipelineIsNotFound(t *testing.T) { scheme := createScheme(t) client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&cdPipeline).Build() - reconcileCDPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard()) + reconcileCDPipeline := NewReconcileCDPipeline(client, scheme) _, err := reconcileCDPipeline.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{ Namespace: namespace, @@ -109,7 +107,7 @@ func TestReconcile_GetCdPipelineError(t *testing.T) { scheme := runtime.NewScheme() client := fake.NewClientBuilder().WithScheme(scheme).Build() - reconcileCDPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard()) + reconcileCDPipeline := NewReconcileCDPipeline(client, scheme) _, err := reconcileCDPipeline.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{ Namespace: namespace, @@ -133,7 +131,7 @@ func TestAddFinalizer_DeletionTimestampNotZero(t *testing.T) { scheme := createScheme(t) client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&cdPipeline).Build() - reconcileCdPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard()) + reconcileCdPipeline := NewReconcileCDPipeline(client, scheme) res, err := reconcileCdPipeline.tryToDeletePipeline(ctrl.LoggerInto(context.Background(), logr.Discard()), &cdPipeline) assert.NoError(t, err) @@ -172,7 +170,7 @@ func TestAddFinalizer_PostponeDeletion(t *testing.T) { scheme := createScheme(t) client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&cdPipeline, stage).Build() - reconcileCdPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard()) + reconcileCdPipeline := NewReconcileCDPipeline(client, scheme) res, err := reconcileCdPipeline.tryToDeletePipeline(ctrl.LoggerInto(context.Background(), logr.Discard()), &cdPipeline) assert.NoError(t, err) @@ -192,7 +190,7 @@ func TestAddFinalizer_DeletionTimestampIsZero(t *testing.T) { scheme := createScheme(t) client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cdPipeline).Build() - reconcileCdPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard()) + reconcileCdPipeline := NewReconcileCDPipeline(client, scheme) res, err := reconcileCdPipeline.tryToDeletePipeline(ctrl.LoggerInto(context.Background(), logr.Discard()), cdPipeline) assert.NoError(t, err) @@ -212,7 +210,7 @@ func TestSetFinishStatus_Success(t *testing.T) { scheme := createScheme(t) client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cdPipeline).Build() - reconcileCdPipeline := NewReconcileCDPipeline(client, scheme, logr.Discard()) + reconcileCdPipeline := NewReconcileCDPipeline(client, scheme) err := reconcileCdPipeline.setFinishStatus(context.Background(), cdPipeline) assert.NoError(t, err) diff --git a/controllers/stage/chain/chain.go b/controllers/stage/chain/chain.go new file mode 100644 index 0000000..77ed020 --- /dev/null +++ b/controllers/stage/chain/chain.go @@ -0,0 +1,38 @@ +package chain + +import ( + "context" + "fmt" + + ctrl "sigs.k8s.io/controller-runtime" + + cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" + "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" +) + +type chain struct { + handlers []handler.CdStageHandler +} + +func (ch *chain) Use(handlers ...handler.CdStageHandler) { + ch.handlers = append(ch.handlers, handlers...) +} + +func (ch *chain) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { + log := ctrl.LoggerFrom(ctx) + + log.Info("Starting Stage chain") + + for i := 0; i < len(ch.handlers); i++ { + h := ch.handlers[i] + + err := h.ServeRequest(ctx, stage) + if err != nil { + return fmt.Errorf("failed to serve handler: %w", err) + } + } + + log.Info("Handling of Stage has been finished") + + return nil +} diff --git a/controllers/stage/chain/check_namespace_exist.go b/controllers/stage/chain/check_namespace_exist.go index 734740b..81b32f6 100644 --- a/controllers/stage/chain/check_namespace_exist.go +++ b/controllers/stage/chain/check_namespace_exist.go @@ -4,45 +4,43 @@ import ( "context" "fmt" - "github.com/go-logr/logr" projectApi "github.com/openshift/api/project/v1" v1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/platform" ) // CheckNamespaceExist checks if namespace exists. type CheckNamespaceExist struct { - next handler.CdStageHandler client multiClusterClient - log logr.Logger } // ServeRequest serves request to check if namespace/project exists. -func (h CheckNamespaceExist) ServeRequest(stage *cdPipeApi.Stage) error { +func (h CheckNamespaceExist) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { name := stage.Spec.Namespace if platform.IsOpenshift() { - if err := h.projectExist(context.Background(), name); err != nil { + if err := h.projectExist(ctx, name); err != nil { return err } } if platform.IsKubernetes() { - if err := h.namespaceExist(context.Background(), name); err != nil { + if err := h.namespaceExist(ctx, name); err != nil { return err } } - return nextServeOrNil(h.next, stage) + return nil } func (h CheckNamespaceExist) namespaceExist(ctx context.Context, name string) error { - h.log.Info("Checking existence of namespace", "name", name) + log := ctrl.LoggerFrom(ctx) + log.Info("Checking existence of namespace", "name", name) if err := h.client.Get(ctx, types.NamespacedName{ Name: name, @@ -58,7 +56,8 @@ func (h CheckNamespaceExist) namespaceExist(ctx context.Context, name string) er } func (h CheckNamespaceExist) projectExist(ctx context.Context, name string) error { - h.log.Info("Checking existence of project", "name", name) + log := ctrl.LoggerFrom(ctx) + log.Info("Checking existence of project", "name", name) if err := h.client.Get(ctx, types.NamespacedName{ Name: name, diff --git a/controllers/stage/chain/check_namespace_exist_test.go b/controllers/stage/chain/check_namespace_exist_test.go index 74e6def..fabdcef 100644 --- a/controllers/stage/chain/check_namespace_exist_test.go +++ b/controllers/stage/chain/check_namespace_exist_test.go @@ -1,6 +1,7 @@ package chain import ( + "context" "testing" "github.com/go-logr/logr" @@ -10,6 +11,7 @@ import ( corev1 "k8s.io/api/core/v1" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -122,10 +124,9 @@ func TestCheckNamespaceExist_ServeRequest(t *testing.T) { h := CheckNamespaceExist{ client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build(), - log: logr.Discard(), } - tt.wantErr(t, h.ServeRequest(tt.stage)) + tt.wantErr(t, h.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage)) }) } } diff --git a/controllers/stage/chain/configure_registryviewer_rbac.go b/controllers/stage/chain/configure_registryviewer_rbac.go index 03c2ea9..a3eabb5 100644 --- a/controllers/stage/chain/configure_registryviewer_rbac.go +++ b/controllers/stage/chain/configure_registryviewer_rbac.go @@ -4,36 +4,33 @@ import ( "context" "fmt" - "github.com/go-logr/logr" rbacApi "k8s.io/api/rbac/v1" + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/platform" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/rbac" ) type ConfigureRegistryViewerRbac struct { - next handler.CdStageHandler - log logr.Logger rbac rbac.Manager } -func (h ConfigureRegistryViewerRbac) ServeRequest(stage *cdPipeApi.Stage) error { +func (h ConfigureRegistryViewerRbac) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { targetNamespace := stage.Spec.Namespace roleBindingName := generateSaRegistryViewerRoleBindingName(stage) - logger := h.log.WithValues("stage", stage.Name, "targetNamespace", targetNamespace, "roleBindingName", roleBindingName) + logger := ctrl.LoggerFrom(ctx).WithValues("targetNamespace", targetNamespace, "roleBindingName", roleBindingName) logger.Info("Configuring RoleBinding sa-registry-viewer") if !platform.IsOpenshift() { logger.Info("Skip configuring RoleBinding sa-registry-viewer for non-openshift platform") - return nextServeOrNil(h.next, stage) + return nil } if err := h.rbac.CreateRoleBindingIfNotExists( - context.TODO(), + ctx, roleBindingName, stage.Namespace, []rbacApi.Subject{ @@ -54,7 +51,7 @@ func (h ConfigureRegistryViewerRbac) ServeRequest(stage *cdPipeApi.Stage) error logger.Info("RoleBinding sa-registry-viewer has been configured") - return nextServeOrNil(h.next, stage) + return nil } // generateSaRegistryViewerRoleBindingName generates name for RoleBinding for registry-viewer role. diff --git a/controllers/stage/chain/configure_registryviewer_rbac_test.go b/controllers/stage/chain/configure_registryviewer_rbac_test.go index fb1b0ee..37cd6e6 100644 --- a/controllers/stage/chain/configure_registryviewer_rbac_test.go +++ b/controllers/stage/chain/configure_registryviewer_rbac_test.go @@ -10,6 +10,7 @@ import ( k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -114,11 +115,10 @@ func TestConfigureRegistryViewerRbac_ServeRequest(t *testing.T) { k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tt.objects...).Build() h := ConfigureRegistryViewerRbac{ - log: logr.Discard(), rbac: rbac.NewRbacManager(k8sClient, logr.Discard()), } - err := h.ServeRequest(tt.stage) + err := h.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage) tt.wantErr(t, err) tt.wantCheck(t, tt.stage, k8sClient) }) diff --git a/controllers/stage/chain/configure_secret_manager.go b/controllers/stage/chain/configure_secret_manager.go index 9b98df2..318809a 100644 --- a/controllers/stage/chain/configure_secret_manager.go +++ b/controllers/stage/chain/configure_secret_manager.go @@ -5,7 +5,6 @@ import ( "fmt" "os" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" rbacApi "k8s.io/api/rbac/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -15,7 +14,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/externalsecrets" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/rbac" ) @@ -32,18 +30,14 @@ const ( // ConfigureSecretManager is a stage chain element that configures secret management. type ConfigureSecretManager struct { - next handler.CdStageHandler multiClusterClient multiClusterClient internalClient client.Client - log logr.Logger } // ServeRequest implements the logic to configure secret management. -func (h ConfigureSecretManager) ServeRequest(stage *cdPipeApi.Stage) error { - ctx := context.Background() // TODO: pass ctx from the caller +func (h ConfigureSecretManager) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { secretManager := os.Getenv(secretManagerEnv) - logger := h.log.WithValues( - "stage", stage.Name, + logger := ctrl.LoggerFrom(ctx).WithValues( "target-ns", stage.Spec.Namespace, "secret-manager", secretManager, ) @@ -59,10 +53,10 @@ func (h ConfigureSecretManager) ServeRequest(stage *cdPipeApi.Stage) error { } default: logger.Info("Secrets management is disabled, skipping") - return nextServeOrNil(h.next, stage) + return nil } - return nextServeOrNil(h.next, stage) + return nil } func (h ConfigureSecretManager) configureEso(ctx context.Context, stage *cdPipeApi.Stage) error { diff --git a/controllers/stage/chain/configure_secret_manager_test.go b/controllers/stage/chain/configure_secret_manager_test.go index 8af6b62..6121dc3 100644 --- a/controllers/stage/chain/configure_secret_manager_test.go +++ b/controllers/stage/chain/configure_secret_manager_test.go @@ -11,6 +11,7 @@ import ( rbacApi "k8s.io/api/rbac/v1" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -304,10 +305,9 @@ func TestConfigureManageSecretsRBAC_ServeRequest(t *testing.T) { h := ConfigureSecretManager{ multiClusterClient: cl, internalClient: cl, - log: logr.Discard(), } - tt.wantErr(t, h.ServeRequest(tt.stage)) + tt.wantErr(t, h.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage)) tt.want(t, h.multiClusterClient, tt.stage) }) } diff --git a/controllers/stage/chain/configure_tenantadmin_rbac.go b/controllers/stage/chain/configure_tenantadmin_rbac.go index db5fe0e..09ffe16 100644 --- a/controllers/stage/chain/configure_tenantadmin_rbac.go +++ b/controllers/stage/chain/configure_tenantadmin_rbac.go @@ -4,11 +4,10 @@ import ( "context" "fmt" - "github.com/go-logr/logr" rbacApi "k8s.io/api/rbac/v1" + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/rbac" ) @@ -17,18 +16,16 @@ const ( ) type ConfigureTenantAdminRbac struct { - next handler.CdStageHandler - log logr.Logger rbac rbac.Manager } -func (h ConfigureTenantAdminRbac) ServeRequest(stage *cdPipeApi.Stage) error { +func (h ConfigureTenantAdminRbac) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { targetNamespace := stage.Spec.Namespace - logger := h.log.WithValues("stage", stage.Name, "target-ns", targetNamespace) + logger := ctrl.LoggerFrom(ctx).WithValues("target-ns", targetNamespace) logger.Info("Configuring tenant admin RBAC") if err := h.rbac.CreateRoleBindingIfNotExists( - context.TODO(), + ctx, tenantAdminRbName, targetNamespace, []rbacApi.Subject{ @@ -54,5 +51,5 @@ func (h ConfigureTenantAdminRbac) ServeRequest(stage *cdPipeApi.Stage) error { logger.Info("RBAC for tenant admin has been configured successfully") - return nextServeOrNil(h.next, stage) + return nil } diff --git a/controllers/stage/chain/configure_tenantadmin_rbac_test.go b/controllers/stage/chain/configure_tenantadmin_rbac_test.go index 6513c51..8789ec4 100644 --- a/controllers/stage/chain/configure_tenantadmin_rbac_test.go +++ b/controllers/stage/chain/configure_tenantadmin_rbac_test.go @@ -9,6 +9,7 @@ import ( rbacApi "k8s.io/api/rbac/v1" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -87,11 +88,10 @@ func TestConfigureTenantAdminRbac_ServeRequest(t *testing.T) { k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tt.objects...).Build() h := ConfigureTenantAdminRbac{ - log: logr.Discard(), rbac: rbac.NewRbacManager(k8sClient, logr.Discard()), } - err := h.ServeRequest(tt.stage) + err := h.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage) tt.wantErr(t, err) tt.wantCheck(t, tt.stage, k8sClient) }) diff --git a/controllers/stage/chain/delegate_namespace_creation.go b/controllers/stage/chain/delegate_namespace_creation.go index ad28785..66422f2 100644 --- a/controllers/stage/chain/delegate_namespace_creation.go +++ b/controllers/stage/chain/delegate_namespace_creation.go @@ -1,32 +1,31 @@ package chain import ( - "github.com/go-logr/logr" + "context" + + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/kiosk" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/platform" ) // DelegateNamespaceCreation is a stage chain element that decides whether to create a namespace, kiosk space or project. type DelegateNamespaceCreation struct { - next handler.CdStageHandler client multiClusterClient - log logr.Logger } // ServeRequest creates for kubernetes platform PutNamespace or PutKioskSpace if the kiosk is enabled. // For platform openshift it creates PutOpenshiftProject. // By default, it creates PutOpenshiftProject. // If the namespace is not managed by the operator, it creates CheckNamespaceExist. -func (c DelegateNamespaceCreation) ServeRequest(stage *cdPipeApi.Stage) error { - logger := c.log.WithValues("stage name", stage.Name) +func (c DelegateNamespaceCreation) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { + logger := ctrl.LoggerFrom(ctx) if !platform.ManageNamespace() { logger.Info("Namespace is not managed by the operator") - return nextServeOrNil(CheckNamespaceExist(c), stage) + return CheckNamespaceExist(c).ServeRequest(ctx, stage) } if platform.IsKubernetes() { @@ -35,18 +34,16 @@ func (c DelegateNamespaceCreation) ServeRequest(stage *cdPipeApi.Stage) error { if !stage.InCluster() { logger.Info("Stage is not in cluster. Skip multi-tenancy engines") - return nextServeOrNil(PutNamespace(c), stage) + return PutNamespace(c).ServeRequest(ctx, stage) } if platform.KioskEnabled() { logger.Info("Kiosk is enabled") - return nextServeOrNil(PutKioskSpace{ - next: c.next, + return PutKioskSpace{ space: kiosk.InitSpace(c.client), client: c.client, - log: c.log, - }, stage) + }.ServeRequest(ctx, stage) } if platform.CapsuleEnabled() { @@ -55,10 +52,10 @@ func (c DelegateNamespaceCreation) ServeRequest(stage *cdPipeApi.Stage) error { logger.Info("None of multi-tenancy engines is enabled") } - return nextServeOrNil(PutNamespace(c), stage) + return PutNamespace(c).ServeRequest(ctx, stage) } logger.Info("Platform is openshift") - return nextServeOrNil(PutOpenshiftProject(c), stage) + return PutOpenshiftProject(c).ServeRequest(ctx, stage) } diff --git a/controllers/stage/chain/delegate_namespace_creation_test.go b/controllers/stage/chain/delegate_namespace_creation_test.go index 557a89a..d86152b 100644 --- a/controllers/stage/chain/delegate_namespace_creation_test.go +++ b/controllers/stage/chain/delegate_namespace_creation_test.go @@ -10,6 +10,7 @@ import ( corev1 "k8s.io/api/core/v1" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -217,10 +218,9 @@ func TestDelegateNamespaceCreation_ServeRequest(t *testing.T) { c := DelegateNamespaceCreation{ client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build(), - log: logr.Discard(), } - err := c.ServeRequest(tt.stage) + err := c.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage) tt.wantErr(t, err) tt.wantAssert(t, c.client, tt.stage) }) diff --git a/controllers/stage/chain/delegate_namespace_deletion.go b/controllers/stage/chain/delegate_namespace_deletion.go index 7e0549b..cdd2130 100644 --- a/controllers/stage/chain/delegate_namespace_deletion.go +++ b/controllers/stage/chain/delegate_namespace_deletion.go @@ -1,19 +1,18 @@ package chain import ( - "github.com/go-logr/logr" + "context" + + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/kiosk" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/platform" ) // DelegateNamespaceDeletion is a stage chain element that decides whether to delete a namespace or project. type DelegateNamespaceDeletion struct { - next handler.CdStageHandler multiClusterClient multiClusterClient - log logr.Logger } // ServeRequest creates for kubernetes platform DeleteNamespace or DeleteSpace if kiosk is enabled. @@ -21,16 +20,13 @@ type DelegateNamespaceDeletion struct { // The decision is made based on the environment variable PLATFORM_TYPE. // By default, it creates DeleteOpenshiftProject. // If the namespace is not managed by the operator, it creates Skip chain element. -func (c DelegateNamespaceDeletion) ServeRequest(stage *cdPipeApi.Stage) error { - logger := c.log.WithValues("stage name", stage.Name) +func (c DelegateNamespaceDeletion) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { + logger := ctrl.LoggerFrom(ctx) if !platform.ManageNamespace() { logger.Info("Namespace is not managed by the operator") - return nextServeOrNil(Skip{ - next: c.next, - log: c.log, - }, stage) + return Skip{}.ServeRequest(ctx, stage) } if platform.IsKubernetes() { @@ -39,17 +35,15 @@ func (c DelegateNamespaceDeletion) ServeRequest(stage *cdPipeApi.Stage) error { if !stage.InCluster() { logger.Info("Stage is not in cluster. Skip multi-tenancy engines") - return nextServeOrNil(DeleteNamespace(c), stage) + return DeleteNamespace(c).ServeRequest(ctx, stage) } if platform.KioskEnabled() { logger.Info("Kiosk is enabled") - return nextServeOrNil(DeleteSpace{ - next: c.next, + return DeleteSpace{ space: kiosk.InitSpace(c.multiClusterClient), - log: c.log, - }, stage) + }.ServeRequest(ctx, stage) } if platform.CapsuleEnabled() { @@ -58,10 +52,10 @@ func (c DelegateNamespaceDeletion) ServeRequest(stage *cdPipeApi.Stage) error { logger.Info("None of multi-tenancy engines is enabled") } - return nextServeOrNil(DeleteNamespace(c), stage) + return DeleteNamespace(c).ServeRequest(ctx, stage) } logger.Info("Platform is openshift") - return nextServeOrNil(DeleteOpenshiftProject(c), stage) + return DeleteOpenshiftProject(c).ServeRequest(ctx, stage) } diff --git a/controllers/stage/chain/delegate_namespace_deletion_test.go b/controllers/stage/chain/delegate_namespace_deletion_test.go index c5280af..b9265c3 100644 --- a/controllers/stage/chain/delegate_namespace_deletion_test.go +++ b/controllers/stage/chain/delegate_namespace_deletion_test.go @@ -11,6 +11,7 @@ import ( apiErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -229,10 +230,9 @@ func TestDelegateNamespaceDeletion_ServeRequest(t *testing.T) { c := DelegateNamespaceDeletion{ multiClusterClient: fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build(), - log: logr.Discard(), } - err := c.ServeRequest(tt.stage) + err := c.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage) tt.wantErr(t, err) tt.wantAssert(t, c.multiClusterClient, tt.stage) }) diff --git a/controllers/stage/chain/delete_environment_label_from_codebase_image_streams.go b/controllers/stage/chain/delete_environment_label_from_codebase_image_streams.go index 3f95fcb..2579162 100644 --- a/controllers/stage/chain/delete_environment_label_from_codebase_image_streams.go +++ b/controllers/stage/chain/delete_environment_label_from_codebase_image_streams.go @@ -4,14 +4,13 @@ import ( "context" "fmt" - "github.com/go-logr/logr" "golang.org/x/exp/slices" k8sErrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/util" edpErr "github.com/epam/edp-cd-pipeline-operator/v2/pkg/error" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/util/cluster" @@ -19,24 +18,23 @@ import ( ) type DeleteEnvironmentLabelFromCodebaseImageStreams struct { - next handler.CdStageHandler client client.Client - log logr.Logger } -func (h DeleteEnvironmentLabelFromCodebaseImageStreams) ServeRequest(stage *cdPipeApi.Stage) error { - h.log.Info("Start deleting environment labels from codebase image streams") +func (h DeleteEnvironmentLabelFromCodebaseImageStreams) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { + log := ctrl.LoggerFrom(ctx) + log.Info("Start deleting environment labels from codebase image streams") - if err := h.deleteEnvironmentLabel(stage); err != nil { + if err := h.deleteEnvironmentLabel(ctx, stage); err != nil { return fmt.Errorf("failed to set environment status: %w", err) } - h.log.Info("Environment labels have been deleted from codebase image streams") + log.Info("Environment labels have been deleted from codebase image streams") - return nextServeOrNil(h.next, stage) + return nil } -func (h DeleteEnvironmentLabelFromCodebaseImageStreams) deleteEnvironmentLabel(stage *cdPipeApi.Stage) error { +func (h DeleteEnvironmentLabelFromCodebaseImageStreams) deleteEnvironmentLabel(ctx context.Context, stage *cdPipeApi.Stage) error { pipe, err := util.GetCdPipeline(h.client, stage) if err != nil { return fmt.Errorf("failed to get %s cd pipeline: %w", stage.Spec.CdPipeline, err) @@ -53,19 +51,19 @@ func (h DeleteEnvironmentLabelFromCodebaseImageStreams) deleteEnvironmentLabel(s } if stage.IsFirst() { - if envErr := h.setEnvLabel(stage.Spec.Name, pipe.Spec.Name, stream); envErr != nil { + if envErr := h.setEnvLabel(ctx, stage.Spec.Name, pipe.Spec.Name, stream); envErr != nil { return envErr } continue } - if envErr := h.setEnvLabelForVerifiedImageStream(stage, stream, pipe.Spec.Name, name); envErr != nil { + if envErr := h.setEnvLabelForVerifiedImageStream(ctx, stage, stream, pipe.Spec.Name, name); envErr != nil { return envErr } if !slices.Contains(pipe.Spec.ApplicationsToPromote, stream.Spec.Codebase) { - if envErr := h.setEnvLabel(stage.Spec.Name, pipe.Spec.Name, stream); envErr != nil { + if envErr := h.setEnvLabel(ctx, stage.Spec.Name, pipe.Spec.Name, stream); envErr != nil { return envErr } } @@ -74,21 +72,24 @@ func (h DeleteEnvironmentLabelFromCodebaseImageStreams) deleteEnvironmentLabel(s return nil } -func (h DeleteEnvironmentLabelFromCodebaseImageStreams) setEnvLabel(stageName, pipeName string, stream *codebaseApi.CodebaseImageStream) error { +func (h DeleteEnvironmentLabelFromCodebaseImageStreams) setEnvLabel(ctx context.Context, stageName, pipeName string, stream *codebaseApi.CodebaseImageStream) error { + log := ctrl.LoggerFrom(ctx) env := createLabelName(pipeName, stageName) deleteLabel(&stream.ObjectMeta, env) - if err := h.client.Update(context.Background(), stream); err != nil { + if err := h.client.Update(ctx, stream); err != nil { return fmt.Errorf("failed to update %v codebase image stream: %w", stream, err) } - h.log.Info("Label has been deleted from CodebaseImageStream", "label", env, "codebaseImageStream", stream.Name) + log.Info("Label has been deleted from CodebaseImageStream", "label", env, "codebaseImageStream", stream.Name) return nil } -func (h DeleteEnvironmentLabelFromCodebaseImageStreams) setEnvLabelForVerifiedImageStream(stage *cdPipeApi.Stage, stream *codebaseApi.CodebaseImageStream, pipeName, dockerStreamName string) error { - previousStageName, err := util.FindPreviousStageName(context.TODO(), h.client, stage) +func (h DeleteEnvironmentLabelFromCodebaseImageStreams) setEnvLabelForVerifiedImageStream(ctx context.Context, stage *cdPipeApi.Stage, stream *codebaseApi.CodebaseImageStream, pipeName, dockerStreamName string) error { + log := ctrl.LoggerFrom(ctx) + + previousStageName, err := util.FindPreviousStageName(ctx, h.client, stage) if err != nil { return fmt.Errorf("failed to previous stage name: %w", err) } @@ -107,11 +108,11 @@ func (h DeleteEnvironmentLabelFromCodebaseImageStreams) setEnvLabelForVerifiedIm env := createLabelName(pipeName, stage.Spec.Name) deleteLabel(&stream.ObjectMeta, env) - if err = h.client.Update(context.Background(), stream); err != nil { + if err = h.client.Update(ctx, stream); err != nil { return fmt.Errorf("failed to update %v codebase image stream: %w", stream, err) } - h.log.Info("Label has been deleted from CodebaseImageStream", "label", env, "dockerStream", dockerStreamName, "codebaseImageStream", stream.Name) + log.Info("Label has been deleted from CodebaseImageStream", "label", env, "dockerStream", dockerStreamName, "codebaseImageStream", stream.Name) return nil } diff --git a/controllers/stage/chain/delete_environment_label_from_codebase_image_streams_test.go b/controllers/stage/chain/delete_environment_label_from_codebase_image_streams_test.go index 01da3c6..5e6804b 100644 --- a/controllers/stage/chain/delete_environment_label_from_codebase_image_streams_test.go +++ b/controllers/stage/chain/delete_environment_label_from_codebase_image_streams_test.go @@ -1,6 +1,7 @@ package chain import ( + "context" "fmt" "testing" @@ -8,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" @@ -49,10 +51,9 @@ func TestServeRequest_Success(t *testing.T) { deleteEnvLabel := DeleteEnvironmentLabelFromCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &cdPipeline, &image).Build(), - log: logr.Discard(), } - err := deleteEnvLabel.ServeRequest(&stage) + err := deleteEnvLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.NoError(t, err) result, err := cluster.GetCodebaseImageStream(deleteEnvLabel.client, dockerImageName, namespace) @@ -105,10 +106,9 @@ func TestDeleteEnvironmentLabel_VerifiedImageStream(t *testing.T) { deleteEnvLabel := DeleteEnvironmentLabelFromCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &prevStage, &cdPipeline, &image, &previousImage).Build(), - log: logr.Discard(), } - err := deleteEnvLabel.deleteEnvironmentLabel(&stage) + err := deleteEnvLabel.deleteEnvironmentLabel(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.NoError(t, err) previousImageStream, err := cluster.GetCodebaseImageStream(deleteEnvLabel.client, cisName, namespace) @@ -167,10 +167,9 @@ func TestDeleteEnvironmentLabel_ApplicationToPromote(t *testing.T) { deleteEnvLabel := DeleteEnvironmentLabelFromCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &prevStage, &cdPipeline, &image, &previousImage).Build(), - log: logr.Discard(), } - err := deleteEnvLabel.deleteEnvironmentLabel(&stage) + err := deleteEnvLabel.deleteEnvironmentLabel(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.NoError(t, err) previousImageStream, err := cluster.GetCodebaseImageStream(deleteEnvLabel.client, cisName, namespace) @@ -187,10 +186,9 @@ func TestServeRequest_Error(t *testing.T) { deleteEnvLabel := DeleteEnvironmentLabelFromCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).Build(), - log: logr.Discard(), } - err := deleteEnvLabel.ServeRequest(&stage) + err := deleteEnvLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.True(t, k8sErrors.IsNotFound(err)) } @@ -209,10 +207,9 @@ func TestDeleteEnvironmentLabel_EmptyInputDockerStream(t *testing.T) { deleteEnvLabel := DeleteEnvironmentLabelFromCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &cdPipeline).Build(), - log: logr.Discard(), } - err := deleteEnvLabel.deleteEnvironmentLabel(&stage) + err := deleteEnvLabel.deleteEnvironmentLabel(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.Equal(t, fmt.Errorf("pipeline %s doesn't contain codebase image streams", cdPipeline.Spec.Name), err) } @@ -232,10 +229,9 @@ func TestDeleteEnvironmentLabel_CantGetImageDockerStream(t *testing.T) { deleteEnvLabel := DeleteEnvironmentLabelFromCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &cdPipeline).Build(), - log: logr.Discard(), } - err := deleteEnvLabel.deleteEnvironmentLabel(&stage) + err := deleteEnvLabel.deleteEnvironmentLabel(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.True(t, k8sErrors.IsNotFound(err)) } @@ -280,10 +276,9 @@ func TestSetDeleteEnvironmentLabel_NoPreviousStageError(t *testing.T) { deleteEnvLabel := DeleteEnvironmentLabelFromCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &cdPipeline, &image, &previousImage).Build(), - log: logr.Discard(), } - err := deleteEnvLabel.deleteEnvironmentLabel(&stage) + err := deleteEnvLabel.deleteEnvironmentLabel(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.Contains(t, err.Error(), "previous stage not found") } @@ -303,11 +298,10 @@ func TestSetEnvLabelForVerifiedImageStream_IsNotFoundPreviousImageStream(t *test deleteEnvLabel := DeleteEnvironmentLabelFromCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &prevStage, &image).Build(), - log: logr.Discard(), } cisName := createCisName(name, previousStageName, image.Spec.Codebase) - err := deleteEnvLabel.setEnvLabelForVerifiedImageStream(&stage, &image, name, dockerImageName) + err := deleteEnvLabel.setEnvLabelForVerifiedImageStream(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage, &image, name, dockerImageName) assert.Equal(t, edpErr.CISNotFoundError(fmt.Sprintf("codebase image stream %s is not found", cisName)), err) } diff --git a/controllers/stage/chain/delete_namespace.go b/controllers/stage/chain/delete_namespace.go index 50dae7e..1c7c1e3 100644 --- a/controllers/stage/chain/delete_namespace.go +++ b/controllers/stage/chain/delete_namespace.go @@ -4,25 +4,20 @@ import ( "context" "fmt" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" ) type DeleteNamespace struct { - next handler.CdStageHandler multiClusterClient multiClusterClient - log logr.Logger } -func (h DeleteNamespace) ServeRequest(stage *cdPipeApi.Stage) error { - l := h.log.WithValues("namespace", stage.Spec.Namespace) - ctx := ctrl.LoggerInto(context.TODO(), l) +func (h DeleteNamespace) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { + l := ctrl.LoggerFrom(ctx).WithValues("namespace", stage.Spec.Namespace) ns := &corev1.Namespace{ ObjectMeta: metaV1.ObjectMeta{ @@ -42,5 +37,5 @@ func (h DeleteNamespace) ServeRequest(stage *cdPipeApi.Stage) error { l.Info("Namespace has been deleted") - return nextServeOrNil(h.next, stage) + return nil } diff --git a/controllers/stage/chain/delete_namespace_test.go b/controllers/stage/chain/delete_namespace_test.go index 68208d4..812cc53 100644 --- a/controllers/stage/chain/delete_namespace_test.go +++ b/controllers/stage/chain/delete_namespace_test.go @@ -10,6 +10,7 @@ import ( v1 "k8s.io/api/core/v1" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" @@ -18,7 +19,6 @@ import ( func TestDeleteNamespace_NSDoestExists(t *testing.T) { ch := DeleteNamespace{ multiClusterClient: fake.NewClientBuilder().Build(), - log: logr.Discard(), } s := &cdPipeApi.Stage{ @@ -27,7 +27,7 @@ func TestDeleteNamespace_NSDoestExists(t *testing.T) { Namespace: namespace, }, } - err := ch.ServeRequest(s) + err := ch.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), s) assert.NoError(t, err) ns := &v1.Namespace{} @@ -48,7 +48,6 @@ func TestDeleteNamespace_DeleteNS(t *testing.T) { ch := DeleteNamespace{ multiClusterClient: fake.NewClientBuilder().WithRuntimeObjects(ns).Build(), - log: logr.Discard(), } s := &cdPipeApi.Stage{ @@ -60,11 +59,11 @@ func TestDeleteNamespace_DeleteNS(t *testing.T) { Namespace: n, }, } - err := ch.ServeRequest(s) + err := ch.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), s) assert.NoError(t, err) ns = &v1.Namespace{} - err = ch.multiClusterClient.Get(context.TODO(), types.NamespacedName{ + err = ch.multiClusterClient.Get(context.Background(), types.NamespacedName{ Name: n, }, ns) assert.Error(t, err, "ns doesn't exist") diff --git a/controllers/stage/chain/delete_openshift_project.go b/controllers/stage/chain/delete_openshift_project.go index d7a9c90..d93d5f6 100644 --- a/controllers/stage/chain/delete_openshift_project.go +++ b/controllers/stage/chain/delete_openshift_project.go @@ -4,26 +4,23 @@ import ( "context" "fmt" - "github.com/go-logr/logr" projectApi "github.com/openshift/api/project/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" ) // DeleteOpenshiftProject is a handler that deletes an openshift project for a stage. type DeleteOpenshiftProject struct { - next handler.CdStageHandler multiClusterClient multiClusterClient - log logr.Logger } // ServeRequest is a function that deletes openshift project. -func (h DeleteOpenshiftProject) ServeRequest(stage *cdPipeApi.Stage) error { +func (h DeleteOpenshiftProject) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { projectName := stage.Spec.Namespace - logger := h.log.WithValues("name", projectName) + logger := ctrl.LoggerFrom(ctx).WithValues("project", projectName) project := &projectApi.Project{ ObjectMeta: metaV1.ObjectMeta{ @@ -31,7 +28,7 @@ func (h DeleteOpenshiftProject) ServeRequest(stage *cdPipeApi.Stage) error { }, } - if err := h.multiClusterClient.Delete(context.TODO(), project); err != nil { + if err := h.multiClusterClient.Delete(ctx, project); err != nil { if apierrors.IsNotFound(err) { logger.Info("Project has already been deleted") @@ -43,5 +40,5 @@ func (h DeleteOpenshiftProject) ServeRequest(stage *cdPipeApi.Stage) error { logger.Info("Project has been deleted") - return nextServeOrNil(h.next, stage) + return nil } diff --git a/controllers/stage/chain/delete_openshift_project_test.go b/controllers/stage/chain/delete_openshift_project_test.go index 227bedd..5b27440 100644 --- a/controllers/stage/chain/delete_openshift_project_test.go +++ b/controllers/stage/chain/delete_openshift_project_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -79,10 +80,9 @@ func TestDeleteOpenshiftProject_ServeRequest(t *testing.T) { h := DeleteOpenshiftProject{ multiClusterClient: fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tt.objects...).Build(), - log: logr.Discard(), } - tt.wantErr(t, h.ServeRequest(tt.stage)) + tt.wantErr(t, h.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage)) tt.wantAssert(t, h.multiClusterClient, tt.stage) }) } diff --git a/controllers/stage/chain/delete_registryviewer_rbac.go b/controllers/stage/chain/delete_registryviewer_rbac.go index 81167ea..ef1c511 100644 --- a/controllers/stage/chain/delete_registryviewer_rbac.go +++ b/controllers/stage/chain/delete_registryviewer_rbac.go @@ -4,38 +4,35 @@ import ( "context" "fmt" - "github.com/go-logr/logr" rbacApi "k8s.io/api/rbac/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/platform" ) // DeleteRegistryViewerRbac deletes sa-registry-viewer RoleBinding. type DeleteRegistryViewerRbac struct { - next handler.CdStageHandler multiClusterCl multiClusterClient - log logr.Logger } // ServeRequest deletes sa-registry-viewer RoleBinding. -func (h DeleteRegistryViewerRbac) ServeRequest(stage *cdPipeApi.Stage) error { +func (h DeleteRegistryViewerRbac) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { targetNamespace := stage.Spec.Namespace roleBindingName := generateSaRegistryViewerRoleBindingName(stage) - logger := h.log.WithValues("stage", stage.Name, "targetNamespace", targetNamespace, "roleBindingName", roleBindingName) + logger := ctrl.LoggerFrom(ctx).WithValues("targetNamespace", targetNamespace, "roleBindingName", roleBindingName) logger.Info("Deleting RoleBinding sa-registry-viewer") if !platform.IsOpenshift() { logger.Info("Skip deleting RoleBinding sa-registry-viewer non-openshift platform") - return nextServeOrNil(h.next, stage) + return nil } - if err := h.multiClusterCl.Delete(context.TODO(), &rbacApi.RoleBinding{ + if err := h.multiClusterCl.Delete(ctx, &rbacApi.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: roleBindingName, Namespace: stage.Namespace, @@ -43,7 +40,7 @@ func (h DeleteRegistryViewerRbac) ServeRequest(stage *cdPipeApi.Stage) error { }); err != nil { if k8sErrors.IsNotFound(err) { logger.Info("RoleBinding sa-registry-viewer has been already deleted") - return nextServeOrNil(h.next, stage) + return nil } return fmt.Errorf("failed to delete %s RoleBinding: %w", roleBindingName, err) @@ -51,5 +48,5 @@ func (h DeleteRegistryViewerRbac) ServeRequest(stage *cdPipeApi.Stage) error { logger.Info("RoleBinding for registry-viewer has been deleted") - return nextServeOrNil(h.next, stage) + return nil } diff --git a/controllers/stage/chain/delete_registryviewer_rbac_test.go b/controllers/stage/chain/delete_registryviewer_rbac_test.go index dd5d16d..8977a00 100644 --- a/controllers/stage/chain/delete_registryviewer_rbac_test.go +++ b/controllers/stage/chain/delete_registryviewer_rbac_test.go @@ -11,6 +11,7 @@ import ( k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -102,10 +103,9 @@ func TestDeleteRegistryViewerRbac_ServeRequest(t *testing.T) { h := DeleteRegistryViewerRbac{ multiClusterCl: fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build(), - log: logr.Discard(), } - err := h.ServeRequest(tt.stage) + err := h.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage) tt.wantErr(t, err) tt.wantCheck(t, tt.stage, h.multiClusterCl) diff --git a/controllers/stage/chain/delete_space.go b/controllers/stage/chain/delete_space.go index 6747b84..96f8270 100644 --- a/controllers/stage/chain/delete_space.go +++ b/controllers/stage/chain/delete_space.go @@ -1,25 +1,23 @@ package chain import ( + "context" "fmt" - "github.com/go-logr/logr" k8sErrors "k8s.io/apimachinery/pkg/api/errors" + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/kiosk" ) type DeleteSpace struct { - next handler.CdStageHandler - log logr.Logger space kiosk.SpaceManager } -func (h DeleteSpace) ServeRequest(stage *cdPipeApi.Stage) error { +func (h DeleteSpace) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { name := stage.Spec.Namespace - logger := h.log.WithValues("stage name", stage.Name, "space", name, "namespace", name) + logger := ctrl.LoggerFrom(ctx).WithValues("space", name, "namespace", name) logger.Info("deleting loft kiosk space resource and namespace related to this space") if err := h.space.Delete(name); err != nil { @@ -33,5 +31,5 @@ func (h DeleteSpace) ServeRequest(stage *cdPipeApi.Stage) error { logger.Info("namespace has been deleted.") - return nextServeOrNil(h.next, stage) + return nil } diff --git a/controllers/stage/chain/delete_space_test.go b/controllers/stage/chain/delete_space_test.go index d708a5f..755a92e 100644 --- a/controllers/stage/chain/delete_space_test.go +++ b/controllers/stage/chain/delete_space_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -11,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" @@ -54,11 +56,10 @@ func TestDeleteSpace_DeleteSpaceSuccess(t *testing.T) { } deleteSpaceInstance := DeleteSpace{ - log: logger, space: testSpace, } - err := deleteSpaceInstance.ServeRequest(stage) + err := deleteSpaceInstance.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), stage) assert.NoError(t, err) emptySpace := &unstructured.Unstructured{} @@ -86,11 +87,10 @@ func TestDeleteSpace_SpaceDoesntExist(t *testing.T) { } deleteSpaceInstance := DeleteSpace{ - log: log, space: testSpace, } - err := deleteSpaceInstance.ServeRequest(stage) + err := deleteSpaceInstance.ServeRequest(ctrl.LoggerInto(context.Background(), log), stage) loggerSink, ok := log.GetSink().(*commonmock.Logger) assert.True(t, ok) diff --git a/controllers/stage/chain/factory.go b/controllers/stage/chain/factory.go index 8dd17e7..b327b56 100644 --- a/controllers/stage/chain/factory.go +++ b/controllers/stage/chain/factory.go @@ -13,38 +13,11 @@ import ( "github.com/epam/edp-cd-pipeline-operator/v2/pkg/rbac" ) -var log = ctrl.Log.WithName("stage") - -const ( - putCodebaseImageStreamChain = "put-codebase-image-stream-chain" - deleteEnvironmentLabelFromCodebaseImageStream = "delete-env-label-cis" - logKeyRegistryViewerRbac = "sa-registry-viewer-rbac" - logKeyTenantAdminRbac = "tenant-admin-rbac" - logKeyPutNamespace = "put-namespace" - logKeyManageSecretsRBAC = "manage-secrets-rbac" -) - // multiClusterClient is a internalClient for external cluster connection. // It will connect to internal cluster if stage.Spec.ClusterName is "in-cluster". type multiClusterClient client.Client -func nextServeOrNil(next handler.CdStageHandler, stage *cdPipeApi.Stage) error { - if next != nil { - if err := next.ServeRequest(stage); err != nil { - return fmt.Errorf("failed to serve request: %w", err) - } - - return nil - } - - log.Info("handling of cd stage has been finished", "name", stage.Name) - - return nil -} - func CreateChain(ctx context.Context, c client.Client, stage *cdPipeApi.Stage) (handler.CdStageHandler, error) { - logger := ctrl.Log.WithName("create-chain") - multiClusterCl, err := multiclusterclient.NewClientProvider(c).GetClusterClient(ctx, stage.Namespace, stage.Spec.ClusterName, client.Options{}) if err != nil { return nil, fmt.Errorf("failed to get cluster internalClient: %w", err) @@ -52,63 +25,62 @@ func CreateChain(ctx context.Context, c client.Client, stage *cdPipeApi.Stage) ( rbacManager := rbac.NewRbacManager(multiClusterCl, ctrl.Log.WithName("rbac-manager")) - logger.Info("Tekton chain is selected") - - return PutCodebaseImageStream{ - next: DelegateNamespaceCreation{ + ch := &chain{} + ch.Use( + PutCodebaseImageStream{ + client: c, + }, + DelegateNamespaceCreation{ client: multiClusterCl, - log: ctrl.Log.WithName(logKeyPutNamespace), - next: RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate{ - client: c, - log: ctrl.Log.WithName("remove-labels-from-codebase-docker-streams-after-cd-pipeline-update"), - next: DeleteEnvironmentLabelFromCodebaseImageStreams{ - client: c, - log: ctrl.Log.WithName(deleteEnvironmentLabelFromCodebaseImageStream), - next: PutEnvironmentLabelToCodebaseImageStreams{ - client: c, - log: ctrl.Log.WithName("put-environment-label-to-codebase-image-streams-chain"), - next: ConfigureRegistryViewerRbac{ - log: ctrl.Log.WithName(logKeyRegistryViewerRbac), - rbac: rbacManager, - next: ConfigureTenantAdminRbac{ - log: ctrl.Log.WithName(logKeyTenantAdminRbac), - rbac: rbacManager, - next: ConfigureSecretManager{ - multiClusterClient: multiClusterCl, - internalClient: c, - log: ctrl.Log.WithName(logKeyManageSecretsRBAC), - }, - }, - }, - }, - }, - }, }, - client: c, - log: ctrl.Log.WithName(putCodebaseImageStreamChain), - }, nil + RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate{ + client: c, + }, + DeleteEnvironmentLabelFromCodebaseImageStreams{ + client: c, + }, + PutEnvironmentLabelToCodebaseImageStreams{ + client: c, + }, + ConfigureRegistryViewerRbac{ + rbac: rbacManager, + }, + ConfigureTenantAdminRbac{ + rbac: rbacManager, + }, + ConfigureSecretManager{ + multiClusterClient: multiClusterCl, + internalClient: c, + }, + ) + + return ch, nil } func CreateDeleteChain(ctx context.Context, c client.Client, stage *cdPipeApi.Stage) (handler.CdStageHandler, error) { - logger := ctrl.LoggerFrom(ctx) + log := ctrl.LoggerFrom(ctx) + ch := &chain{} - logger.Info("Delete chain is selected") + ch.Use( + DeleteEnvironmentLabelFromCodebaseImageStreams{ + client: c, + }, + ) multiClusterCl, err := multiclusterclient.NewClientProvider(c).GetClusterClient(ctx, stage.Namespace, stage.Spec.ClusterName, client.Options{}) if err != nil { - return nil, fmt.Errorf("failed to get cluster internalClient: %w", err) + log.Error(err, "Failed to get cluster internalClient. Skipping namespace deletion.") + return ch, nil } - return DeleteEnvironmentLabelFromCodebaseImageStreams{ - client: c, - log: logger.WithName(deleteEnvironmentLabelFromCodebaseImageStream), - next: DelegateNamespaceDeletion{ + ch.Use( + DelegateNamespaceDeletion{ multiClusterClient: multiClusterCl, - log: logger.WithName("delete-namespace"), - next: DeleteRegistryViewerRbac{ - multiClusterCl: multiClusterCl, - log: logger.WithName("delete-registry-viewer-rbac"), - }, }, - }, nil + DeleteRegistryViewerRbac{ + multiClusterCl: multiClusterCl, + }, + ) + + return ch, nil } diff --git a/controllers/stage/chain/handler/cd_stage_handler.go b/controllers/stage/chain/handler/cd_stage_handler.go index 9eb9064..05d9b7b 100644 --- a/controllers/stage/chain/handler/cd_stage_handler.go +++ b/controllers/stage/chain/handler/cd_stage_handler.go @@ -1,9 +1,11 @@ package handler import ( + "context" + cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" ) type CdStageHandler interface { - ServeRequest(stage *cdPipeApi.Stage) error + ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error } diff --git a/controllers/stage/chain/put_codebase_image_stream.go b/controllers/stage/chain/put_codebase_image_stream.go index 4e9e405..df5cb71 100644 --- a/controllers/stage/chain/put_codebase_image_stream.go +++ b/controllers/stage/chain/put_codebase_image_stream.go @@ -4,15 +4,14 @@ import ( "context" "fmt" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/util" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/util/cluster" codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" @@ -20,9 +19,7 @@ import ( ) type PutCodebaseImageStream struct { - next handler.CdStageHandler client client.Client - log logr.Logger } const ( @@ -32,11 +29,10 @@ const ( edpConfigContainerRegistrySpace = "container_registry_space" ) -func (h PutCodebaseImageStream) ServeRequest(stage *cdPipeApi.Stage) error { - logger := h.log.WithValues("stage name", stage.Name) - ctx := context.TODO() +func (h PutCodebaseImageStream) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { + log := ctrl.LoggerFrom(ctx) - logger.Info("start creating codebase image streams.") + log.Info("start creating codebase image streams.") pipe, err := util.GetCdPipeline(h.client, stage) if err != nil { @@ -57,14 +53,14 @@ func (h PutCodebaseImageStream) ServeRequest(stage *cdPipeApi.Stage) error { cisName := fmt.Sprintf("%v-%v-%v-verified", pipe.Name, stage.Spec.Name, stream.Spec.Codebase) image := fmt.Sprintf("%v/%v", registryUrl, stream.Spec.Codebase) - if err := h.createCodebaseImageStreamIfNotExists(cisName, image, stream.Spec.Codebase, stage.Namespace); err != nil { + if err := h.createCodebaseImageStreamIfNotExists(ctx, cisName, image, stream.Spec.Codebase, stage.Namespace); err != nil { return fmt.Errorf("failed to create %v codebase image stream: %w", cisName, err) } } - logger.Info("codebase image stream have been created.") + log.Info("codebase image stream have been created.") - return nextServeOrNil(h.next, stage) + return nil } func (h PutCodebaseImageStream) getDockerRegistryUrl(ctx context.Context, namespace string) (string, error) { @@ -101,7 +97,8 @@ func (h PutCodebaseImageStream) getDockerRegistryUrl(ctx context.Context, namesp return fmt.Sprintf("%s/%s", config.Data[edpConfigContainerRegistryHost], config.Data[edpConfigContainerRegistrySpace]), nil } -func (h PutCodebaseImageStream) createCodebaseImageStreamIfNotExists(name, imageName, codebaseName, namespace string) error { +func (h PutCodebaseImageStream) createCodebaseImageStreamIfNotExists(ctx context.Context, name, imageName, codebaseName, namespace string) error { + log := ctrl.LoggerFrom(ctx) cis := &codebaseApi.CodebaseImageStream{ TypeMeta: metaV1.TypeMeta{ APIVersion: "v2.edp.epam.com/v1", @@ -117,16 +114,16 @@ func (h PutCodebaseImageStream) createCodebaseImageStreamIfNotExists(name, image }, } - if err := h.client.Create(context.TODO(), cis); err != nil { + if err := h.client.Create(ctx, cis); err != nil { if k8sErrors.IsAlreadyExists(err) { - h.log.Info("codebase image stream already exists. skip creating...", "name", cis.Name) + log.Info("codebase image stream already exists. skip creating...", "name", cis.Name) return nil } return fmt.Errorf("failed to create codebase stream: %w", err) } - h.log.Info("codebase image stream has been created", "name", name) + log.Info("codebase image stream has been created", "name", name) return nil } diff --git a/controllers/stage/chain/put_codebase_image_stream_test.go b/controllers/stage/chain/put_codebase_image_stream_test.go index 5e4c4be..ab44c01 100644 --- a/controllers/stage/chain/put_codebase_image_stream_test.go +++ b/controllers/stage/chain/put_codebase_image_stream_test.go @@ -12,6 +12,7 @@ import ( metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" @@ -75,10 +76,9 @@ func TestPutCodebaseImageStream_ShouldCreateCis(t *testing.T) { cisChain := PutCodebaseImageStream{ client: c, - log: logr.Discard(), } - err := cisChain.ServeRequest(s) + err := cisChain.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), s) assert.NoError(t, err) cisResp := &codebaseApi.CodebaseImageStream{} @@ -125,10 +125,9 @@ func TestPutCodebaseImageStream_ShouldNotFindCDPipeline(t *testing.T) { cisChain := PutCodebaseImageStream{ client: c, - log: logr.Discard(), } - err := cisChain.ServeRequest(s) + err := cisChain.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), s) assert.Error(t, err) if !strings.Contains(err.Error(), "non-existing-pipeline") { @@ -169,10 +168,9 @@ func TestPutCodebaseImageStream_ShouldNotFindRegistryUrl(t *testing.T) { cisChain := PutCodebaseImageStream{ client: c, - log: logr.Discard(), } - err := cisChain.ServeRequest(s) + err := cisChain.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), s) require.Error(t, err) assert.Contains(t, err.Error(), "failed to get container registry url") } @@ -223,10 +221,9 @@ func TestPutCodebaseImageStream_ShouldNotFindCbis(t *testing.T) { cisChain := PutCodebaseImageStream{ client: c, - log: logr.Discard(), } - err := cisChain.ServeRequest(s) + err := cisChain.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), s) assert.Error(t, err) if !strings.Contains(err.Error(), "failed to get cbis-name codebase image stream") { @@ -300,10 +297,9 @@ func TestPutCodebaseImageStream_ShouldNotFailWithExistingCbis(t *testing.T) { cisChain := PutCodebaseImageStream{ client: c, - log: logr.Discard(), } - err := cisChain.ServeRequest(s) + err := cisChain.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), s) assert.NoError(t, err) cisResp := &codebaseApi.CodebaseImageStream{} @@ -373,10 +369,9 @@ func TestPutCodebaseImageStream_ShouldCreateCisFromConfigMap(t *testing.T) { cisChain := PutCodebaseImageStream{ client: c, - log: logr.Discard(), } - err := cisChain.ServeRequest(stage) + err := cisChain.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), stage) assert.NoError(t, err) cisResp := &codebaseApi.CodebaseImageStream{} diff --git a/controllers/stage/chain/put_environment_label_to_codebase_image_streams.go b/controllers/stage/chain/put_environment_label_to_codebase_image_streams.go index d3f530c..072bc50 100644 --- a/controllers/stage/chain/put_environment_label_to_codebase_image_streams.go +++ b/controllers/stage/chain/put_environment_label_to_codebase_image_streams.go @@ -4,13 +4,12 @@ import ( "context" "fmt" - "github.com/go-logr/logr" "golang.org/x/exp/slices" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/util" edpError "github.com/epam/edp-cd-pipeline-operator/v2/pkg/error" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/util/cluster" @@ -19,17 +18,15 @@ import ( ) type PutEnvironmentLabelToCodebaseImageStreams struct { - next handler.CdStageHandler client client.Client - log logr.Logger } // nolint -func (h PutEnvironmentLabelToCodebaseImageStreams) ServeRequest(stage *cdPipeApi.Stage) error { - logger := h.log.WithValues("stage name", stage.Name) +func (h PutEnvironmentLabelToCodebaseImageStreams) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { + logger := ctrl.LoggerFrom(ctx) if consts.AutoDeployTriggerType != stage.Spec.TriggerType { logger.Info("Trigger type is not auto deploy, skipping") - return nextServeOrNil(h.next, stage) + return nil } logger.Info("start creating environment labels in codebase image stream resources.") @@ -50,14 +47,14 @@ func (h PutEnvironmentLabelToCodebaseImageStreams) ServeRequest(stage *cdPipeApi } if stage.IsFirst() || !slices.Contains(pipe.Spec.ApplicationsToPromote, stream.Spec.Codebase) { - if updErr := h.updateLabel(stream, pipe.Name, stage.Spec.Name); updErr != nil { + if updErr := h.updateLabel(ctx, stream, pipe.Name, stage.Spec.Name); updErr != nil { return updErr } continue } - previousStageName, err := util.FindPreviousStageName(context.TODO(), h.client, stage) + previousStageName, err := util.FindPreviousStageName(ctx, h.client, stage) if err != nil { return fmt.Errorf("failed to previous stage name: %w", err) } @@ -69,24 +66,26 @@ func (h PutEnvironmentLabelToCodebaseImageStreams) ServeRequest(stage *cdPipeApi return edpError.CISNotFoundError(fmt.Sprintf("couldn't get %s codebase image stream", name)) } - if err := h.updateLabel(verifiedStream, pipe.Name, stage.Spec.Name); err != nil { + if err := h.updateLabel(ctx, verifiedStream, pipe.Name, stage.Spec.Name); err != nil { return err } } logger.Info("environment labels have been added to codebase image stream resources.") - return nextServeOrNil(h.next, stage) + return nil } -func (h PutEnvironmentLabelToCodebaseImageStreams) updateLabel(cis *codebaseApi.CodebaseImageStream, pipeName, stageName string) error { +func (h PutEnvironmentLabelToCodebaseImageStreams) updateLabel(ctx context.Context, cis *codebaseApi.CodebaseImageStream, pipeName, stageName string) error { + log := ctrl.LoggerFrom(ctx) + setLabel(&cis.ObjectMeta, pipeName, stageName) - if err := h.client.Update(context.Background(), cis); err != nil { + if err := h.client.Update(ctx, cis); err != nil { return fmt.Errorf("couldn't update %s codebase image stream: %w", cis.Name, err) } - h.log.Info("label has been added to codebase image stream", + log.Info("label has been added to codebase image stream", "label", fmt.Sprintf("%s/%s", pipeName, stageName), "stream", cis.Name) return nil diff --git a/controllers/stage/chain/put_environment_label_to_codebase_image_streams_test.go b/controllers/stage/chain/put_environment_label_to_codebase_image_streams_test.go index 93a759c..97c2cc3 100644 --- a/controllers/stage/chain/put_environment_label_to_codebase_image_streams_test.go +++ b/controllers/stage/chain/put_environment_label_to_codebase_image_streams_test.go @@ -1,6 +1,7 @@ package chain import ( + "context" "fmt" "testing" @@ -10,6 +11,7 @@ import ( k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" @@ -77,10 +79,9 @@ func TestPutEnvironmentLabelToCodebaseImageStreams_ServeRequest_Success(t *testi putEnvLabel := PutEnvironmentLabelToCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &cdPipeline, &image).Build(), - log: logr.Discard(), } - err := putEnvLabel.ServeRequest(&stage) + err := putEnvLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.NoError(t, err) imageStream, err := cluster.GetCodebaseImageStream(putEnvLabel.client, dockerImageName, namespace) @@ -132,10 +133,9 @@ func TestPutEnvironmentLabelToCodebaseImageStreams_ServeRequest_PreviousStageIma putEnvLabel := PutEnvironmentLabelToCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &prevStage, &cdPipeline, &image, &previousImage).Build(), - log: logr.Discard(), } - err := putEnvLabel.ServeRequest(&stage) + err := putEnvLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.NoError(t, err) imageStream, err := cluster.GetCodebaseImageStream(putEnvLabel.client, cisName, namespace) @@ -150,10 +150,9 @@ func TestPutEnvironmentLabelToCodebaseImageStreams_ServeRequest_CantGetCdPipelin putEnvLabel := PutEnvironmentLabelToCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage).Build(), - log: logr.Discard(), } - err := putEnvLabel.ServeRequest(&stage) + err := putEnvLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.True(t, k8sErrors.IsNotFound(err)) } @@ -174,10 +173,9 @@ func TestPutEnvironmentLabelToCodebaseImageStreams_ServeRequest_EmptyInputDocker putEnvLabel := PutEnvironmentLabelToCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &cdPipeline).Build(), - log: logr.Discard(), } - err := putEnvLabel.ServeRequest(&stage) + err := putEnvLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.Equal(t, fmt.Errorf("pipeline %s doesn't contain codebase image streams", cdPipeline.Name), err) } @@ -198,10 +196,9 @@ func TestPutEnvironmentLabelToCodebaseImageStreams_ServeRequest_CantGetImage(t * putEnvLabel := PutEnvironmentLabelToCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &cdPipeline).Build(), - log: logr.Discard(), } - err := putEnvLabel.ServeRequest(&stage) + err := putEnvLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.True(t, k8sErrors.IsNotFound(err)) } @@ -238,9 +235,8 @@ func TestPutEnvironmentLabelToCodebaseImageStreams_ServeRequest_CantGetPreviousS putEnvLabel := PutEnvironmentLabelToCodebaseImageStreams{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &prevStage, &cdPipeline, &image).Build(), - log: logr.Discard(), } - err := putEnvLabel.ServeRequest(&stage) + err := putEnvLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.Equal(t, edpErr.CISNotFoundError(fmt.Sprintf("couldn't get %v codebase image stream", dockerImageName)), err) } diff --git a/controllers/stage/chain/put_kiosk_space.go b/controllers/stage/chain/put_kiosk_space.go index 46e29e4..f1dc58b 100644 --- a/controllers/stage/chain/put_kiosk_space.go +++ b/controllers/stage/chain/put_kiosk_space.go @@ -4,41 +4,35 @@ import ( "context" "fmt" - "github.com/go-logr/logr" k8sErrors "k8s.io/apimachinery/pkg/api/errors" - metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/kiosk" - "github.com/epam/edp-cd-pipeline-operator/v2/pkg/util/consts" ) type PutKioskSpace struct { - next handler.CdStageHandler space kiosk.SpaceManager client client.Client - log logr.Logger } -func (h PutKioskSpace) ServeRequest(stage *cdPipeApi.Stage) error { +func (h PutKioskSpace) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { name := stage.Spec.Namespace - h.log.Info("try to create namespace", "name", name) - - if err := h.createSpace(name, stage.Namespace); err != nil { - if setErr := h.setFailedStatus(context.Background(), stage, err); setErr != nil { - return fmt.Errorf("failed to update stage %s status: %w", stage.Name, err) - } + log := ctrl.LoggerFrom(ctx).WithValues("space", name) + log.Info("Creating kiosk space") + if err := h.createSpace(ctrl.LoggerInto(ctx, log), name, stage.Namespace); err != nil { return fmt.Errorf("failed to create %s lofk kiosk space cr: %w", name, err) } - return nextServeOrNil(h.next, stage) + return nil } -func (h PutKioskSpace) createSpace(name, account string) error { - exists, err := h.spaceExists(name) +func (h PutKioskSpace) createSpace(ctx context.Context, name, account string) error { + log := ctrl.LoggerFrom(ctx) + + exists, err := h.spaceExists(ctx, name) if err != nil { return err } @@ -56,8 +50,10 @@ func (h PutKioskSpace) createSpace(name, account string) error { return nil } -func (h PutKioskSpace) spaceExists(name string) (bool, error) { - h.log.Info("checking existence of space cr", "name", name) +func (h PutKioskSpace) spaceExists(ctx context.Context, name string) (bool, error) { + log := ctrl.LoggerFrom(ctx) + + log.Info("checking existence of space cr", "name", name) _, err := h.space.Get(name) if err != nil { @@ -70,29 +66,3 @@ func (h PutKioskSpace) spaceExists(name string) (bool, error) { return true, nil } - -func (h PutKioskSpace) setFailedStatus(ctx context.Context, stage *cdPipeApi.Stage, err error) error { - updateStatus := func(ctx context.Context, stage *cdPipeApi.Stage) error { - if err = h.client.Status().Update(ctx, stage); err != nil { - if err = h.client.Update(ctx, stage); err != nil { - return fmt.Errorf("failed to update kiosk space status: %w", err) - } - } - - h.log.Info("stage status has been updated.", "name", stage.Name) - - return nil - } - - stage.Status = cdPipeApi.StageStatus{ - Status: consts.FailedStatus, - Available: false, - LastTimeUpdated: metaV1.Now(), - Username: stage.Status.Username, - Result: cdPipeApi.Error, - DetailedMessage: err.Error(), - Value: consts.FailedStatus, - } - - return updateStatus(ctx, stage) -} diff --git a/controllers/stage/chain/put_kiosk_space_test.go b/controllers/stage/chain/put_kiosk_space_test.go index ba7593d..a99cf8a 100644 --- a/controllers/stage/chain/put_kiosk_space_test.go +++ b/controllers/stage/chain/put_kiosk_space_test.go @@ -5,16 +5,15 @@ import ( "testing" "github.com/go-logr/logr" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/kiosk" - "github.com/epam/edp-cd-pipeline-operator/v2/pkg/util/consts" ) const ( @@ -38,10 +37,9 @@ func TestSpaceExist_NotFound(t *testing.T) { putKioskSpace := PutKioskSpace{ space: space, client: client, - log: logr.Discard(), } - exists, err := putKioskSpace.spaceExists(name) + exists, err := putKioskSpace.spaceExists(ctrl.LoggerInto(context.Background(), logr.Discard()), name) assert.NoError(t, err) assert.False(t, exists) } @@ -63,10 +61,9 @@ func TestSpaceExist_Success(t *testing.T) { putKioskSpace := PutKioskSpace{ space: spaceManager, client: client, - log: logr.Discard(), } - exists, err := putKioskSpace.spaceExists(name) + exists, err := putKioskSpace.spaceExists(ctrl.LoggerInto(context.Background(), logr.Discard()), name) assert.NoError(t, err) assert.True(t, exists) } @@ -79,10 +76,9 @@ func TestCreateSpace_Success(t *testing.T) { putKioskSpace := PutKioskSpace{ space: spaceManager, client: client, - log: logr.Discard(), } - err := putKioskSpace.createSpace(name, account) + err := putKioskSpace.createSpace(ctrl.LoggerInto(context.Background(), logr.Discard()), name, account) assert.NoError(t, err) space, err := spaceManager.Get(name) @@ -107,34 +103,15 @@ func TestCreateSpace_AlreadyExists(t *testing.T) { putKioskSpace := PutKioskSpace{ space: spaceManager, client: client, - log: logr.Discard(), } _, err := spaceManager.Get(name) assert.NoError(t, err) - err = putKioskSpace.createSpace(name, account) + err = putKioskSpace.createSpace(ctrl.LoggerInto(context.Background(), logr.Discard()), name, account) assert.NoError(t, err) } -func TestSetFailedStatus_Success(t *testing.T) { - stage := emptyStageInit(t) - - client := fake.NewClientBuilder().WithScheme(kioskSpaceScheme(t)).WithObjects(stage).Build() - - spaceManager := kiosk.InitSpace(client) - - putKioskSpace := PutKioskSpace{ - space: spaceManager, - client: client, - log: logr.Discard(), - } - - err := putKioskSpace.setFailedStatus(context.Background(), stage, errors.New("")) - assert.NoError(t, err) - assert.Equal(t, consts.FailedStatus, stage.Status.Status) -} - func TestPutKioskSpace_ServeRequest_Success(t *testing.T) { client := fake.NewClientBuilder().WithScheme(kioskSpaceScheme(t)).Build() @@ -143,12 +120,11 @@ func TestPutKioskSpace_ServeRequest_Success(t *testing.T) { putKioskSpace := PutKioskSpace{ space: spaceManager, client: client, - log: logr.Discard(), } stage := emptyStageInit(t) - err := putKioskSpace.ServeRequest(stage) + err := putKioskSpace.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), stage) assert.NoError(t, err) space, err := spaceManager.Get(stage.Spec.Namespace) diff --git a/controllers/stage/chain/put_namespace.go b/controllers/stage/chain/put_namespace.go index a0dfe61..d9220bb 100644 --- a/controllers/stage/chain/put_namespace.go +++ b/controllers/stage/chain/put_namespace.go @@ -4,26 +4,21 @@ import ( "context" "fmt" - "github.com/go-logr/logr" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/util" ) type PutNamespace struct { - next handler.CdStageHandler client multiClusterClient - log logr.Logger } -func (h PutNamespace) ServeRequest(stage *cdPipeApi.Stage) error { - l := h.log.WithValues("namespace", stage.Spec.Namespace) - ctx := ctrl.LoggerInto(context.TODO(), l) +func (h PutNamespace) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { + l := ctrl.LoggerFrom(context.Background()) l.Info("Creating namespace") @@ -39,7 +34,7 @@ func (h PutNamespace) ServeRequest(stage *cdPipeApi.Stage) error { if apierrors.IsAlreadyExists(err) { l.Info("Namespace already exists") - return nextServeOrNil(h.next, stage) + return nil } return fmt.Errorf("failed to create namespace: %w", err) @@ -47,5 +42,5 @@ func (h PutNamespace) ServeRequest(stage *cdPipeApi.Stage) error { l.Info("Namespace has been created") - return nextServeOrNil(h.next, stage) + return nil } diff --git a/controllers/stage/chain/put_namespace_test.go b/controllers/stage/chain/put_namespace_test.go index 95472e9..b00d961 100644 --- a/controllers/stage/chain/put_namespace_test.go +++ b/controllers/stage/chain/put_namespace_test.go @@ -10,6 +10,7 @@ import ( v1 "k8s.io/api/core/v1" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" @@ -23,7 +24,6 @@ var ( func TestPutNamespace_CreateNs(t *testing.T) { ch := PutNamespace{ client: fake.NewClientBuilder().Build(), - log: logr.Discard(), } s := &cdPipeApi.Stage{ ObjectMeta: metaV1.ObjectMeta{ @@ -34,7 +34,7 @@ func TestPutNamespace_CreateNs(t *testing.T) { Namespace: "default-stage-1", }, } - err := ch.ServeRequest(s) + err := ch.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), s) assert.NoError(t, err) ns := &v1.Namespace{} @@ -53,7 +53,6 @@ func TestPutNamespace_NSExists(t *testing.T) { ch := PutNamespace{ client: fake.NewClientBuilder().WithRuntimeObjects(ns).Build(), - log: logr.Discard(), } s := &cdPipeApi.Stage{ ObjectMeta: metaV1.ObjectMeta{ @@ -64,6 +63,6 @@ func TestPutNamespace_NSExists(t *testing.T) { Namespace: ns.Name, }, } - err := ch.ServeRequest(s) + err := ch.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), s) assert.NoError(t, err) } diff --git a/controllers/stage/chain/put_openshift_project.go b/controllers/stage/chain/put_openshift_project.go index 8f39077..f5a1033 100644 --- a/controllers/stage/chain/put_openshift_project.go +++ b/controllers/stage/chain/put_openshift_project.go @@ -4,27 +4,24 @@ import ( "context" "fmt" - "github.com/go-logr/logr" projectApi "github.com/openshift/api/project/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/util" ) // PutOpenshiftProject is a handler that creates an openshift project for a stage. type PutOpenshiftProject struct { - next handler.CdStageHandler client multiClusterClient - log logr.Logger } // ServeRequest creates a project for a stage. -func (c PutOpenshiftProject) ServeRequest(stage *cdPipeApi.Stage) error { +func (c PutOpenshiftProject) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { projectName := stage.Spec.Namespace - logger := c.log.WithValues("name", projectName) + logger := ctrl.LoggerFrom(ctx).WithValues("project", projectName) logger.Info("Try to create project") @@ -41,7 +38,7 @@ func (c PutOpenshiftProject) ServeRequest(stage *cdPipeApi.Stage) error { if apierrors.IsAlreadyExists(err) { logger.Info("Project already exists") - return nextServeOrNil(c.next, stage) + return nil } return fmt.Errorf("failed to create project: %w", err) @@ -49,5 +46,5 @@ func (c PutOpenshiftProject) ServeRequest(stage *cdPipeApi.Stage) error { logger.Info("Project has been created") - return nextServeOrNil(c.next, stage) + return nil } diff --git a/controllers/stage/chain/put_openshift_project_test.go b/controllers/stage/chain/put_openshift_project_test.go index e27a8dc..414afa7 100644 --- a/controllers/stage/chain/put_openshift_project_test.go +++ b/controllers/stage/chain/put_openshift_project_test.go @@ -10,6 +10,7 @@ import ( metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -91,10 +92,9 @@ func TestPutOpenshiftProject_ServeRequest(t *testing.T) { c := PutOpenshiftProject{ client: k8sClient, - log: logr.Discard(), } - err := c.ServeRequest(tt.stage) + err := c.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage) tt.wantErr(t, err) tt.wantAssert(t, k8sClient, tt.stage) }) diff --git a/controllers/stage/chain/remove_labels_from_codebase_docker_streams.go b/controllers/stage/chain/remove_labels_from_codebase_docker_streams.go index a2fb6dc..ae39353 100644 --- a/controllers/stage/chain/remove_labels_from_codebase_docker_streams.go +++ b/controllers/stage/chain/remove_labels_from_codebase_docker_streams.go @@ -5,29 +5,26 @@ import ( "fmt" "strings" - "github.com/go-logr/logr" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/util" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/util/cluster" "github.com/epam/edp-cd-pipeline-operator/v2/pkg/util/consts" ) type RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate struct { - next handler.CdStageHandler client client.Client - log logr.Logger } const dockerStreamsBeforeUpdateAnnotationKey = "deploy.edp.epam.com/docker-streams-before-update" -func (h RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate) ServeRequest(stage *cdPipeApi.Stage) error { - log := h.log.WithValues("stage name", stage.Name) +func (h RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate) ServeRequest(ctx context.Context, stage *cdPipeApi.Stage) error { + log := ctrl.LoggerFrom(ctx) if consts.AutoDeployTriggerType != stage.Spec.TriggerType { log.Info("Trigger type is not auto deploy, skipping") - return nextServeOrNil(h.next, stage) + return nil } log.Info("start deleting environment labels from codebase image stream resources.") @@ -39,9 +36,9 @@ func (h RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate) ServeRequest annotations := pipe.GetAnnotations()[dockerStreamsBeforeUpdateAnnotationKey] if annotations == "" { - h.log.Info("CodebaseImageStream doesn't contain %v annotation." + + log.Info("CodebaseImageStream doesn't contain %v annotation." + " skip deleting env labels from CodebaseImageStream resources") - return nextServeOrNil(h.next, stage) + return nil } streams := strings.Split(annotations, ",") @@ -61,5 +58,5 @@ func (h RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate) ServeRequest log.Info("environment labels have been deleted from codebase image stream resources.") - return nextServeOrNil(h.next, stage) + return nil } diff --git a/controllers/stage/chain/remove_labels_from_codebase_docker_streams_test.go b/controllers/stage/chain/remove_labels_from_codebase_docker_streams_test.go index aff34ca..fcbb806 100644 --- a/controllers/stage/chain/remove_labels_from_codebase_docker_streams_test.go +++ b/controllers/stage/chain/remove_labels_from_codebase_docker_streams_test.go @@ -1,12 +1,14 @@ package chain import ( + "context" "testing" "github.com/go-logr/logr" "github.com/stretchr/testify/assert" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" @@ -51,10 +53,9 @@ func TestRemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate_ServeRequest removeLabel := RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &cdPipeline, &image).Build(), - log: logr.Discard(), } - err := removeLabel.ServeRequest(&stage) + err := removeLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.NoError(t, err) currentImageStream, err := cluster.GetCodebaseImageStream(removeLabel.client, dockerImageName, namespace) @@ -67,10 +68,9 @@ func TestRemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate_ServeRequest removeLabel := RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage).Build(), - log: logr.Discard(), } - err := removeLabel.ServeRequest(&stage) + err := removeLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.True(t, k8sErrors.IsNotFound(err)) } @@ -84,10 +84,9 @@ func TestRemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate_ServeRequest removeLabel := RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &cdPipeline, &image).Build(), - log: logr.Discard(), } - err := removeLabel.ServeRequest(&stage) + err := removeLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.NoError(t, err) currentImageStream, err := cluster.GetCodebaseImageStream(removeLabel.client, dockerImageName, namespace) @@ -105,9 +104,8 @@ func TestRemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate_ServeRequest removeLabel := RemoveLabelsFromCodebaseDockerStreamsAfterCdPipelineUpdate{ client: fake.NewClientBuilder().WithScheme(schemeInit(t)).WithObjects(&stage, &cdPipeline, &image).Build(), - log: logr.Discard(), } - err := removeLabel.ServeRequest(&stage) + err := removeLabel.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), &stage) assert.True(t, k8sErrors.IsNotFound(err)) } diff --git a/controllers/stage/chain/skip.go b/controllers/stage/chain/skip.go index 0cc3477..c219d3e 100644 --- a/controllers/stage/chain/skip.go +++ b/controllers/stage/chain/skip.go @@ -1,21 +1,20 @@ package chain import ( - "github.com/go-logr/logr" + "context" + + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" - "github.com/epam/edp-cd-pipeline-operator/v2/controllers/stage/chain/handler" ) // Skip is a stage chain element that do nothing. -type Skip struct { - next handler.CdStageHandler - log logr.Logger -} +type Skip struct{} // ServeRequest does nothing. -func (c Skip) ServeRequest(stage *cdPipeApi.Stage) error { - c.log.Info("skip chain", "name", stage.Name) +func (Skip) ServeRequest(ctx context.Context, _ *cdPipeApi.Stage) error { + log := ctrl.LoggerFrom(ctx) + log.Info("Skip chain") - return nextServeOrNil(c.next, stage) + return nil } diff --git a/controllers/stage/chain/skip_test.go b/controllers/stage/chain/skip_test.go index d785a9d..41d6389 100644 --- a/controllers/stage/chain/skip_test.go +++ b/controllers/stage/chain/skip_test.go @@ -1,11 +1,13 @@ package chain import ( + "context" "testing" "github.com/go-logr/logr" "github.com/stretchr/testify/assert" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" cdPipeApi "github.com/epam/edp-cd-pipeline-operator/v2/api/v1" ) @@ -35,11 +37,7 @@ func TestSkip_ServeRequest(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - c := Skip{ - log: logr.Discard(), - } - - tt.wantErr(t, c.ServeRequest(tt.stage)) + tt.wantErr(t, Skip{}.ServeRequest(ctrl.LoggerInto(context.Background(), logr.Discard()), tt.stage)) }) } } diff --git a/controllers/stage/stage_controller.go b/controllers/stage/stage_controller.go index bd0221b..e83cde8 100644 --- a/controllers/stage/stage_controller.go +++ b/controllers/stage/stage_controller.go @@ -126,10 +126,14 @@ func (r *ReconcileStage) Reconcile(ctx context.Context, request reconcile.Reques ch, err := chain.CreateChain(ctx, r.client, stage) if err != nil { + if statusErr := r.setFailedStatus(ctx, stage, err); statusErr != nil { + log.Error(statusErr, "Failed to set failed status") + } + return reconcile.Result{}, fmt.Errorf("failed to create chain: %w", err) } - if err = ch.ServeRequest(stage); err != nil { + if err = ch.ServeRequest(ctx, stage); err != nil { var e edpError.CISNotFoundError if errors.As(err, &e) { log.Error(err, "cis wasn't found. reconcile again...") @@ -190,7 +194,7 @@ func (r *ReconcileStage) tryToDeleteCDStage(ctx context.Context, stage *cdPipeAp return &reconcile.Result{}, fmt.Errorf("failed to create delete chain: %w", err) } - if err = ch.ServeRequest(stage); err != nil { + if err = ch.ServeRequest(ctx, stage); err != nil { return &reconcile.Result{}, fmt.Errorf("failed to delete Stage: %w", err) } diff --git a/go.mod b/go.mod index 906ff36..4fb1dd9 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,6 @@ require ( github.com/epam/edp-component-operator v0.12.0 github.com/go-logr/logr v1.2.3 github.com/openshift/api v3.9.0+incompatible - github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.8.1 golang.org/x/exp v0.0.0-20230315142452-642cacee5cc0 k8s.io/api v0.26.1 @@ -53,6 +52,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.14.0 // indirect github.com/prometheus/client_model v0.3.0 // indirect diff --git a/main.go b/main.go index 940aca6..e2e6571 100644 --- a/main.go +++ b/main.go @@ -120,7 +120,7 @@ func main() { } ctrlLog := ctrl.Log.WithName("controllers") - cdPipeCtrl := cdpipeline.NewReconcileCDPipeline(cl, mgr.GetScheme(), ctrlLog) + cdPipeCtrl := cdpipeline.NewReconcileCDPipeline(cl, mgr.GetScheme()) if err = cdPipeCtrl.SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "cd-pipeline")