diff --git a/api/cmd/portainer/main.go b/api/cmd/portainer/main.go index a2cb8ae7d..389f94bbd 100644 --- a/api/cmd/portainer/main.go +++ b/api/cmd/portainer/main.go @@ -486,7 +486,7 @@ func buildServer(flags *portainer.CLIFlags) portainer.Server { pendingActionsService := pendingactions.NewService(dataStore, kubernetesClientFactory) pendingActionsService.RegisterHandler(actions.CleanNAPWithOverridePolicies, handlers.NewHandlerCleanNAPWithOverridePolicies(authorizationService, dataStore)) - pendingActionsService.RegisterHandler(actions.DeleteK8sRegistrySecrets, handlers.NewHandlerDeleteRegistrySecrets(authorizationService, dataStore, kubernetesClientFactory)) + pendingActionsService.RegisterHandler(actions.DeletePortainerK8sRegistrySecrets, handlers.NewHandlerDeleteRegistrySecrets(authorizationService, dataStore, kubernetesClientFactory)) pendingActionsService.RegisterHandler(actions.PostInitMigrateEnvironment, handlers.NewHandlerPostInitMigrateEnvironment(authorizationService, dataStore, kubernetesClientFactory, dockerClientFactory, *flags.Assets, kubernetesDeployer)) snapshotService, err := initSnapshotService(*flags.SnapshotInterval, dataStore, dockerClientFactory, kubernetesClientFactory, shutdownCtx, pendingActionsService) diff --git a/api/datastore/postinit/migrate_post_init.go b/api/datastore/postinit/migrate_post_init.go index 5093b2162..2e831aabd 100644 --- a/api/datastore/postinit/migrate_post_init.go +++ b/api/datastore/postinit/migrate_post_init.go @@ -2,8 +2,6 @@ package postinit import ( "context" - "fmt" - "reflect" "github.com/docker/docker/api/types/container" "github.com/docker/docker/client" @@ -74,28 +72,11 @@ func (postInitMigrator *PostInitMigrator) PostInitMigrate() error { // this function exists for readability, not reusability // TODO: This should be moved into pending actions as part of the pending action migration func (postInitMigrator *PostInitMigrator) createPostInitMigrationPendingAction(environmentID portainer.EndpointID) error { - migrateEnvPendingAction := portainer.PendingAction{ + // If there are no pending actions for the given endpoint, create one + err := postInitMigrator.dataStore.PendingActions().Create(&portainer.PendingAction{ EndpointID: environmentID, Action: actions.PostInitMigrateEnvironment, - } - - // Get all pending actions and filter them by endpoint, action and action args that are equal to the migrateEnvPendingAction - pendingActions, err := postInitMigrator.dataStore.PendingActions().ReadAll() - if err != nil { - log.Error().Err(err).Msgf("Error retrieving pending actions") - return fmt.Errorf("failed to retrieve pending actions for environment %d: %w", environmentID, err) - } - for _, pendingAction := range pendingActions { - if pendingAction.EndpointID == environmentID && - pendingAction.Action == migrateEnvPendingAction.Action && - reflect.DeepEqual(pendingAction.ActionData, migrateEnvPendingAction.ActionData) { - log.Debug().Msgf("Migration pending action for environment %d already exists, skipping creating another", environmentID) - return nil - } - } - - // If there are no pending actions for the given endpoint, create one - err = postInitMigrator.dataStore.PendingActions().Create(&migrateEnvPendingAction) + }) if err != nil { log.Error().Err(err).Msgf("Error creating pending action for environment %d", environmentID) } diff --git a/api/internal/snapshot/snapshot.go b/api/internal/snapshot/snapshot.go index 11cfe85b8..6d57abbcc 100644 --- a/api/internal/snapshot/snapshot.go +++ b/api/internal/snapshot/snapshot.go @@ -312,10 +312,7 @@ func updateEndpointStatus(tx dataservices.DataStoreTx, endpoint *portainer.Endpo // Run the pending actions if latestEndpointReference.Status == portainer.EndpointStatusUp { - err = pendingActionsService.Execute(endpoint.ID) - if err != nil { - log.Error().Err(err).Msg("background schedule error (environment snapshot), unable to execute pending actions") - } + pendingActionsService.Execute(endpoint.ID) } } diff --git a/api/pendingactions/actions/actions.go b/api/pendingactions/actions/actions.go index 744fffcfd..49315891d 100644 --- a/api/pendingactions/actions/actions.go +++ b/api/pendingactions/actions/actions.go @@ -1,7 +1,7 @@ package actions const ( - CleanNAPWithOverridePolicies = "CleanNAPWithOverridePolicies" - DeleteK8sRegistrySecrets = "DeleteK8sRegistrySecrets" - PostInitMigrateEnvironment = "PostInitMigrateEnvironment" + CleanNAPWithOverridePolicies = "CleanNAPWithOverridePolicies" + DeletePortainerK8sRegistrySecrets = "DeletePortainerK8sRegistrySecrets" + PostInitMigrateEnvironment = "PostInitMigrateEnvironment" ) diff --git a/api/pendingactions/handlers/delete_k8s_registry_secrets.go b/api/pendingactions/handlers/delete_k8s_registry_secrets.go index 648b31d59..6f38bf4a4 100644 --- a/api/pendingactions/handlers/delete_k8s_registry_secrets.go +++ b/api/pendingactions/handlers/delete_k8s_registry_secrets.go @@ -25,7 +25,7 @@ type ( func NewDeleteK8sRegistrySecrets(endpointID portainer.EndpointID, registryID portainer.RegistryID, namespaces []string) portainer.PendingAction { return portainer.PendingAction{ EndpointID: endpointID, - Action: actions.DeleteK8sRegistrySecrets, + Action: actions.DeletePortainerK8sRegistrySecrets, ActionData: &deleteK8sRegistrySecretsData{ RegistryID: registryID, Namespaces: namespaces, diff --git a/api/pendingactions/pendingactions.go b/api/pendingactions/pendingactions.go index 434294d06..be5a466f0 100644 --- a/api/pendingactions/pendingactions.go +++ b/api/pendingactions/pendingactions.go @@ -2,6 +2,7 @@ package pendingactions import ( "fmt" + "reflect" "sync" portainer "github.com/portainer/portainer/api" @@ -34,72 +35,87 @@ func (service *PendingActionsService) RegisterHandler(name string, handler porta handlers[name] = handler } -func (service *PendingActionsService) Create(r portainer.PendingAction) error { - return service.dataStore.PendingActions().Create(&r) +func (service *PendingActionsService) Create(action portainer.PendingAction) error { + // Check if this pendingAction already exists + pendingActions, err := service.dataStore.PendingActions().ReadAll() + if err != nil { + return fmt.Errorf("failed to retrieve pending actions: %w", err) + } + + for _, dba := range pendingActions { + // Same endpoint, same action and data, don't create a repeat + if dba.EndpointID == action.EndpointID && dba.Action == action.Action && + reflect.DeepEqual(dba.ActionData, action.ActionData) { + log.Debug().Msgf("pending action %s already exists for environment %d, skipping...", action.Action, action.EndpointID) + return nil + } + } + + return service.dataStore.PendingActions().Create(&action) } -func (service *PendingActionsService) Execute(id portainer.EndpointID) error { +func (service *PendingActionsService) Execute(id portainer.EndpointID) { service.mu.Lock() defer service.mu.Unlock() endpoint, err := service.dataStore.Endpoint().Endpoint(id) if err != nil { - return fmt.Errorf("failed to retrieve environment %d: %w", id, err) + log.Debug().Msgf("failed to retrieve environment %d: %v", id, err) + return } isKubernetesEndpoint := endpointutils.IsKubernetesEndpoint(endpoint) && !endpointutils.IsEdgeEndpoint(endpoint) // EndpointStatusUp is only relevant for non-Kubernetes endpoints // Sometimes the endpoint is UP but the status is not updated in the database - if !isKubernetesEndpoint && endpoint.Status != portainer.EndpointStatusUp { - log.Debug().Msgf("Environment %q (id: %d) is not up", endpoint.Name, id) - return fmt.Errorf("environment %q (id: %d) is not up", endpoint.Name, id) - } - - // For Kubernetes endpoints, we need to check if the endpoint is up by creating a kube client - if isKubernetesEndpoint { - _, err := service.kubeFactory.GetKubeClient(endpoint) - if err != nil { - log.Debug().Err(err).Msgf("Environment %q (id: %d) is not up", endpoint.Name, id) - return fmt.Errorf("environment %q (id: %d) is not up", endpoint.Name, id) + if !isKubernetesEndpoint { + if endpoint.Status != portainer.EndpointStatusUp { + return + } + } else { + // For Kubernetes endpoints, we need to check if the client can be created + if _, err := service.kubeFactory.GetKubeClient(endpoint); err != nil { + return } } pendingActions, err := service.dataStore.PendingActions().ReadAll() if err != nil { - log.Error().Err(err).Msgf("failed to retrieve pending actions") - return fmt.Errorf("failed to retrieve pending actions for environment %d: %w", id, err) + log.Warn().Msgf("failed to read pending actions: %v", err) + return } - for _, endpointPendingAction := range pendingActions { - if endpointPendingAction.EndpointID == id { - err := service.executePendingAction(endpointPendingAction, endpoint) + log.Debug().Msgf("Executing pending actions for environment %d", id) + for _, pendingAction := range pendingActions { + if pendingAction.EndpointID == id { + log.Debug().Msgf("executing pending action id=%d, action=%s", pendingAction.ID, pendingAction.Action) + err := service.executePendingAction(pendingAction, endpoint) if err != nil { - log.Warn().Err(err).Msgf("failed to execute pending action") - return fmt.Errorf("failed to execute pending action: %w", err) + log.Warn().Msgf("failed to execute pending action: %v", err) + return } - err = service.dataStore.PendingActions().Delete(endpointPendingAction.ID) + err = service.dataStore.PendingActions().Delete(pendingAction.ID) if err != nil { - log.Error().Err(err).Msgf("failed to delete pending action") - return fmt.Errorf("failed to delete pending action: %w", err) + log.Warn().Msgf("failed to delete pending action: %v", err) + return } + + log.Debug().Msgf("pending action %d finished", pendingAction.ID) } } - - return nil } func (service *PendingActionsService) executePendingAction(pendingAction portainer.PendingAction, endpoint *portainer.Endpoint) error { - log.Debug().Msgf("Executing pending action %s for environment %d", pendingAction.Action, pendingAction.EndpointID) - defer func() { - log.Debug().Msgf("End executing pending action %s for environment %d", pendingAction.Action, pendingAction.EndpointID) + if r := recover(); r != nil { + log.Error().Msgf("recovered from panic while executing pending action %s for environment %d: %v", pendingAction.Action, pendingAction.EndpointID, r) + } }() handler, ok := handlers[pendingAction.Action] if !ok { - log.Warn().Msgf("No handler found for pending action %s", pendingAction.Action) + log.Warn().Msgf("no handler found for pending action %s", pendingAction.Action) return nil }