diff --git a/api/docs/openapi.yaml b/api/docs/openapi.yaml index d9206c8303..eafed7acbd 100644 --- a/api/docs/openapi.yaml +++ b/api/docs/openapi.yaml @@ -12873,6 +12873,8 @@ components: type: string Port: type: integer + PortName: + type: string ServiceName: type: string type: object diff --git a/api/docs/swagger.yaml b/api/docs/swagger.yaml index 5e8e7fa834..4a219a60e4 100644 --- a/api/docs/swagger.yaml +++ b/api/docs/swagger.yaml @@ -1933,6 +1933,8 @@ definitions: type: string Port: type: integer + PortName: + type: string ServiceName: type: string type: object diff --git a/api/http/handler/stacks/create_compose_stack.go b/api/http/handler/stacks/create_compose_stack.go index febb9314e6..861fe6c632 100644 --- a/api/http/handler/stacks/create_compose_stack.go +++ b/api/http/handler/stacks/create_compose_stack.go @@ -169,7 +169,7 @@ func (handler *Handler) createComposeStackFromFileContent(w http.ResponseWriter, handler.FileService, handler.StackDeployer) - stack, httpErr := stackbuilders.Build(r.Context(), handler.DataStore, composeStackBuilder, &stackPayload, endpoint, userID) + stack, httpErr := stackbuilders.BuildAndAsyncDeploy(r.Context(), handler.DataStore, composeStackBuilder, &stackPayload, endpoint, userID) if httpErr != nil { return httpErr } @@ -321,7 +321,7 @@ func (handler *Handler) createComposeStackFromGitRepository(w http.ResponseWrite handler.SourceScheduler, handler.StackDeployer) - stack, httpErr := stackbuilders.Build(r.Context(), handler.DataStore, composeStackBuilder, &stackPayload, endpoint, userID) + stack, httpErr := stackbuilders.BuildAndAsyncDeploy(r.Context(), handler.DataStore, composeStackBuilder, &stackPayload, endpoint, userID) if httpErr != nil { return httpErr } @@ -407,7 +407,7 @@ func (handler *Handler) createComposeStackFromFileUpload(w http.ResponseWriter, handler.FileService, handler.StackDeployer) - stack, httpErr := stackbuilders.Build(r.Context(), handler.DataStore, composeStackBuilder, &stackPayload, endpoint, userID) + stack, httpErr := stackbuilders.BuildAndAsyncDeploy(r.Context(), handler.DataStore, composeStackBuilder, &stackPayload, endpoint, userID) if httpErr != nil { return httpErr } diff --git a/api/http/handler/stacks/create_kubernetes_stack.go b/api/http/handler/stacks/create_kubernetes_stack.go index d7f99895ee..74da7456ef 100644 --- a/api/http/handler/stacks/create_kubernetes_stack.go +++ b/api/http/handler/stacks/create_kubernetes_stack.go @@ -186,7 +186,7 @@ func (handler *Handler) createKubernetesStackFromFileContent(w http.ResponseWrit } } - if _, err := stackbuilders.Build(r.Context(), handler.DataStore, k8sStackBuilder, &stackPayload, endpoint, userID); err != nil { + if _, err := stackbuilders.BuildAndDeploy(r.Context(), handler.DataStore, k8sStackBuilder, &stackPayload, endpoint, userID); err != nil { return err } @@ -270,7 +270,7 @@ func (handler *Handler) createKubernetesStackFromGitRepository(w http.ResponseWr handler.KubernetesDeployer, user) - if _, err := stackbuilders.Build(r.Context(), handler.DataStore, k8sStackBuilder, &stackPayload, endpoint, userID); err != nil { + if _, err := stackbuilders.BuildAndDeploy(r.Context(), handler.DataStore, k8sStackBuilder, &stackPayload, endpoint, userID); err != nil { return err } @@ -315,7 +315,7 @@ func (handler *Handler) createKubernetesStackFromManifestURL(w http.ResponseWrit handler.KubernetesDeployer, user) - if _, err := stackbuilders.Build(r.Context(), handler.DataStore, k8sStackBuilder, &stackPayload, endpoint, userID); err != nil { + if _, err := stackbuilders.BuildAndDeploy(r.Context(), handler.DataStore, k8sStackBuilder, &stackPayload, endpoint, userID); err != nil { return err } diff --git a/api/http/handler/stacks/create_swarm_stack.go b/api/http/handler/stacks/create_swarm_stack.go index 4f46b1c7c3..ed6eaf4106 100644 --- a/api/http/handler/stacks/create_swarm_stack.go +++ b/api/http/handler/stacks/create_swarm_stack.go @@ -98,7 +98,7 @@ func (handler *Handler) createSwarmStackFromFileContent(w http.ResponseWriter, r handler.FileService, handler.StackDeployer) - stack, httpErr := stackbuilders.Build(r.Context(), handler.DataStore, swarmStackBuilder, &stackPayload, endpoint, userID) + stack, httpErr := stackbuilders.BuildAndAsyncDeploy(r.Context(), handler.DataStore, swarmStackBuilder, &stackPayload, endpoint, userID) if httpErr != nil { return httpErr } @@ -253,7 +253,7 @@ func (handler *Handler) createSwarmStackFromGitRepository(w http.ResponseWriter, handler.SourceScheduler, handler.StackDeployer) - stack, httpErr := stackbuilders.Build(r.Context(), handler.DataStore, swarmStackBuilder, &stackPayload, endpoint, userID) + stack, httpErr := stackbuilders.BuildAndAsyncDeploy(r.Context(), handler.DataStore, swarmStackBuilder, &stackPayload, endpoint, userID) if httpErr != nil { return httpErr } @@ -353,7 +353,7 @@ func (handler *Handler) createSwarmStackFromFileUpload(w http.ResponseWriter, r handler.FileService, handler.StackDeployer) - stack, httpErr := stackbuilders.Build(r.Context(), handler.DataStore, swarmStackBuilder, &stackPayload, endpoint, userID) + stack, httpErr := stackbuilders.BuildAndAsyncDeploy(r.Context(), handler.DataStore, swarmStackBuilder, &stackPayload, endpoint, userID) if httpErr != nil { return httpErr } diff --git a/api/http/handler/stacks/deploy_gate.go b/api/http/handler/stacks/deploy_gate.go deleted file mode 100644 index 88c1f16f9f..0000000000 --- a/api/http/handler/stacks/deploy_gate.go +++ /dev/null @@ -1,30 +0,0 @@ -package stacks - -type deployGate struct { - start chan struct{} - abort chan struct{} -} - -func newDeployGate() *deployGate { - return &deployGate{ - start: make(chan struct{}), - abort: make(chan struct{}), - } -} - -func (gate deployGate) wait() bool { - select { - case <-gate.start: - return true - case <-gate.abort: - return false - } -} - -func (gate deployGate) startDeploy() { - close(gate.start) -} - -func (gate deployGate) abortDeploy() { - close(gate.abort) -} diff --git a/api/http/handler/stacks/stack_deploy_inline_test.go b/api/http/handler/stacks/stack_deploy_inline_test.go new file mode 100644 index 0000000000..af3d266175 --- /dev/null +++ b/api/http/handler/stacks/stack_deploy_inline_test.go @@ -0,0 +1,105 @@ +package stacks + +import ( + "context" + "errors" + "testing" + + portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/datastore" + "github.com/portainer/portainer/api/stacks/deployments" + "github.com/portainer/portainer/api/stacks/stackutils" + + "github.com/stretchr/testify/require" +) + +type stubStackDeploymentConfiger struct { + deployments.StackDeploymentConfiger + deployErr error +} + +func (s *stubStackDeploymentConfiger) Deploy(_ context.Context) error { return s.deployErr } + +func TestStackDeployInline(t *testing.T) { + t.Parallel() + + t.Run("successful deploy persists Active status and calls postDeploy with a nil error", func(t *testing.T) { + t.Parallel() + + _, store := datastore.MustNewTestStore(t, true, false) + stack := &portainer.Stack{ID: 1, Status: portainer.StackStatusActive} + stackutils.PrepareStackStatusForDeployment(stack) + require.NoError(t, store.Stack().Create(stack)) + + handler := &Handler{DataStore: store} + + var postDeployCalled bool + var postDeployErr error + postDeploy := func(_ context.Context, err error) { + postDeployCalled = true + postDeployErr = err + } + + httpErr := handler.stackDeployInline(stack, &stubStackDeploymentConfiger{}, postDeploy) + require.Nil(t, httpErr) + + updated, err := store.Stack().Read(stack.ID) + require.NoError(t, err) + require.Equal(t, portainer.StackStatusActive, updated.Status) + require.Len(t, updated.DeploymentStatus, 2, "expected the persisted Deploying entry followed by an Active entry") + require.Equal(t, portainer.StackStatusDeploying, updated.DeploymentStatus[0].Status) + require.Equal(t, portainer.StackStatusActive, updated.DeploymentStatus[1].Status) + + require.True(t, postDeployCalled) + require.NoError(t, postDeployErr) + }) + + t.Run("failed deploy transitions status to Error, calls postDeploy with the error, and returns an error", func(t *testing.T) { + t.Parallel() + + _, store := datastore.MustNewTestStore(t, true, false) + stack := &portainer.Stack{ID: 1, Status: portainer.StackStatusActive} + stackutils.PrepareStackStatusForDeployment(stack) + require.NoError(t, store.Stack().Create(stack)) + + handler := &Handler{DataStore: store} + + deployErr := errors.New("failed to apply resources") + + var postDeployCalled bool + var postDeployErr error + postDeploy := func(_ context.Context, err error) { + postDeployCalled = true + postDeployErr = err + } + + httpErr := handler.stackDeployInline(stack, &stubStackDeploymentConfiger{deployErr: deployErr}, postDeploy) + require.NotNil(t, httpErr) + + updated, err := store.Stack().Read(stack.ID) + require.NoError(t, err) + require.Equal(t, portainer.StackStatusError, updated.Status, "status should transition to Error so the stack doesn't get stuck showing Deploying and can be retried") + require.Len(t, updated.DeploymentStatus, 2, "expected the persisted Deploying entry followed by an Error entry") + require.Equal(t, portainer.StackStatusDeploying, updated.DeploymentStatus[0].Status) + lastEntry := updated.DeploymentStatus[1] + require.Equal(t, portainer.StackStatusError, lastEntry.Status) + require.Equal(t, deployErr.Error(), lastEntry.Message) + + require.True(t, postDeployCalled) + require.Equal(t, deployErr, postDeployErr) + }) + + t.Run("postDeploy may be nil", func(t *testing.T) { + t.Parallel() + + _, store := datastore.MustNewTestStore(t, true, false) + stack := &portainer.Stack{ID: 1, Status: portainer.StackStatusActive} + stackutils.PrepareStackStatusForDeployment(stack) + require.NoError(t, store.Stack().Create(stack)) + + handler := &Handler{DataStore: store} + + httpErr := handler.stackDeployInline(stack, &stubStackDeploymentConfiger{}, nil) + require.Nil(t, httpErr) + }) +} diff --git a/api/http/handler/stacks/stack_update.go b/api/http/handler/stacks/stack_update.go index a7f03ef7e9..a785ef1530 100644 --- a/api/http/handler/stacks/stack_update.go +++ b/api/http/handler/stacks/stack_update.go @@ -105,7 +105,9 @@ func (handler *Handler) stackUpdate(w http.ResponseWriter, r *http.Request) *htt var stack *portainer.Stack var reconcileSourceID portainer.SourceID - err = handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { + var deploymentConfig deployments.StackDeploymentConfiger + var postDeploy postDeployFunc + if err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { preStack, err := tx.Stack().Read(portainer.StackID(stackID)) if err == nil && preStack.WorkflowID != 0 { securityContext, scErr := security.RetrieveRestrictedRequestContext(r) @@ -118,32 +120,50 @@ func (handler *Handler) stackUpdate(w http.ResponseWriter, r *http.Request) *htt } var httpErr *httperror.HandlerError - stack, httpErr = handler.updateStackInTx(tx, r, portainer.StackID(stackID), portainer.EndpointID(endpointID)) + stack, deploymentConfig, postDeploy, httpErr = handler.updateStackInTx(tx, r, portainer.StackID(stackID), portainer.EndpointID(endpointID)) if httpErr != nil { return httpErr } return nil - }) - if err == nil { + }); err != nil { + return response.TxResponse(w, stack, err) + } + + if deploymentConfig == nil { + return response.JSON(w, stack) + } + + if stack.Type == portainer.KubernetesStack { + if httpErr := handler.stackDeployInline(stack, deploymentConfig, postDeploy); httpErr != nil { + return httpErr + } + if err := handler.SourceScheduler.Reconcile(reconcileSourceID); err != nil { log.Warn().Err(err).Msg("source scheduler reconcile failed after stack update") } + return response.JSON(w, stack) } - return response.TxResponse(w, stack, err) + if err := handler.SourceScheduler.Reconcile(reconcileSourceID); err != nil { + log.Warn().Err(err).Msg("source scheduler reconcile failed after stack update") + } + + go stackDeploy(handler.DataStore, stack.ID, deploymentConfig, postDeploy) + + return response.JSON(w, stack) } -func (handler *Handler) updateStackInTx(tx dataservices.DataStoreTx, r *http.Request, stackID portainer.StackID, endpointID portainer.EndpointID) (*portainer.Stack, *httperror.HandlerError) { +func (handler *Handler) updateStackInTx(tx dataservices.DataStoreTx, r *http.Request, stackID portainer.StackID, endpointID portainer.EndpointID) (*portainer.Stack, deployments.StackDeploymentConfiger, postDeployFunc, *httperror.HandlerError) { stack, err := tx.Stack().Read(stackID) if tx.IsErrObjectNotFound(err) { - return nil, httperror.NotFound("Unable to find a stack with the specified identifier inside the database", err) + return nil, nil, nil, httperror.NotFound("Unable to find a stack with the specified identifier inside the database", err) } else if err != nil { - return nil, httperror.InternalServerError("Unable to find a stack with the specified identifier inside the database", err) + return nil, nil, nil, httperror.InternalServerError("Unable to find a stack with the specified identifier inside the database", err) } if stack.Status == portainer.StackStatusDeploying { - return nil, httperror.Conflict("Unable to update stack", errors.New("Stack deployment is already in progress")) + return nil, nil, nil, httperror.Conflict("Unable to update stack", errors.New("Stack deployment is already in progress")) } if endpointID != 0 && endpointID != stack.EndpointID { @@ -152,89 +172,89 @@ func (handler *Handler) updateStackInTx(tx dataservices.DataStoreTx, r *http.Req endpoint, err := tx.Endpoint().Endpoint(stack.EndpointID) if tx.IsErrObjectNotFound(err) { - return nil, httperror.NotFound("Unable to find the environment associated to the stack inside the database", err) + return nil, nil, nil, httperror.NotFound("Unable to find the environment associated to the stack inside the database", err) } else if err != nil { - return nil, httperror.InternalServerError("Unable to find the environment associated to the stack inside the database", err) + return nil, nil, nil, httperror.InternalServerError("Unable to find the environment associated to the stack inside the database", err) } if err := handler.requestBouncer.AuthorizedEndpointOperation(r, endpoint); err != nil { - return nil, httperror.Forbidden("Permission denied to access environment", err) + return nil, nil, nil, httperror.Forbidden("Permission denied to access environment", err) } securityContext, err := security.RetrieveRestrictedRequestContext(r) if err != nil { - return nil, httperror.InternalServerError("Unable to retrieve info from request context", err) + return nil, nil, nil, httperror.InternalServerError("Unable to retrieve info from request context", err) } //only check resource control when it is a DockerSwarmStack or a DockerComposeStack if stack.Type == portainer.DockerSwarmStack || stack.Type == portainer.DockerComposeStack { resourceControl, err := tx.ResourceControl().ResourceControlByResourceIDAndType(stackutils.ResourceControlID(stack.EndpointID, stack.Name), portainer.StackResourceControl) if err != nil { - return nil, httperror.InternalServerError("Unable to retrieve a resource control associated to the stack", err) + return nil, nil, nil, httperror.InternalServerError("Unable to retrieve a resource control associated to the stack", err) } if access, err := handler.userCanAccessStack(securityContext, resourceControl); err != nil { - return nil, httperror.InternalServerError("Unable to verify user authorizations to validate stack access", err) + return nil, nil, nil, httperror.InternalServerError("Unable to verify user authorizations to validate stack access", err) } else if !access { - return nil, httperror.Forbidden("Access denied to resource", httperrors.ErrResourceAccessDenied) + return nil, nil, nil, httperror.Forbidden("Access denied to resource", httperrors.ErrResourceAccessDenied) } } if canManage, err := handler.userCanManageStacks(securityContext, endpoint); err != nil { - return nil, httperror.InternalServerError("Unable to verify user authorizations to validate stack deletion", err) + return nil, nil, nil, httperror.InternalServerError("Unable to verify user authorizations to validate stack deletion", err) } else if !canManage { errMsg := "Stack editing is disabled for non-admin users" - return nil, httperror.Forbidden(errMsg, errors.New(errMsg)) + return nil, nil, nil, httperror.Forbidden(errMsg, errors.New(errMsg)) } - deployGate := newDeployGate() - if err := handler.updateAndDeployStack(tx, r, stack, endpoint, deployGate); err != nil { - return nil, err + deploymentConfig, postDeploy, httpErr := handler.updateAndDeployStack(tx, r, stack, endpoint) + if httpErr != nil { + return nil, nil, nil, httpErr } user, err := tx.User().Read(securityContext.UserID) if err != nil { - return nil, httperror.BadRequest("Cannot find context user", errors.Wrap(err, "failed to fetch the user")) + return nil, nil, nil, httperror.BadRequest("Cannot find context user", errors.Wrap(err, "failed to fetch the user")) } stack.UpdatedBy = user.Username stack.UpdateDate = time.Now().Unix() + + userContext := source.NewUserContext(securityContext.User, securityContext.UserMemberships) + stackutils.PrepareStackStatusForDeployment(stack) if err := tx.Stack().Update(stack.ID, stack); err != nil { - deployGate.abortDeploy() - return nil, httperror.InternalServerError("Unable to persist the stack changes inside the database", err) + return nil, nil, nil, httperror.InternalServerError("Unable to persist the stack changes inside the database", err) } - deployGate.startDeploy() - - userContext := source.NewUserContext(securityContext.User, securityContext.UserMemberships) if err := fillStackGitConfig(tx, userContext, stack); err != nil { - return nil, httperror.InternalServerError("Unable to load git config for stack", err) + return nil, nil, nil, httperror.InternalServerError("Unable to load git config for stack", err) } - return stack, nil + return stack, deploymentConfig, postDeploy, nil } -func (handler *Handler) updateAndDeployStack(tx dataservices.DataStoreTx, r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint, gate *deployGate) *httperror.HandlerError { +func (handler *Handler) updateAndDeployStack(tx dataservices.DataStoreTx, r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint) (deployments.StackDeploymentConfiger, postDeployFunc, *httperror.HandlerError) { switch stack.Type { case portainer.DockerSwarmStack: stack.Name = handler.SwarmStackManager.NormalizeStackName(stack.Name) - return handler.updateSwarmStack(tx, r, stack, endpoint, gate) + return handler.updateSwarmStack(tx, r, stack, endpoint) case portainer.DockerComposeStack: stack.Name = handler.ComposeStackManager.NormalizeStackName(stack.Name) - return handler.updateComposeStack(tx, r, stack, endpoint, gate) + return handler.updateComposeStack(tx, r, stack, endpoint) case portainer.KubernetesStack: - return handler.updateKubernetesStack(tx, r, stack, endpoint, gate) + return handler.updateKubernetesStack(tx, r, stack, endpoint) } - return httperror.InternalServerError("Unsupported stack", errors.Errorf("unsupported stack type: %v", stack.Type)) + return nil, nil, httperror.InternalServerError("Unsupported stack", errors.Errorf("unsupported stack type: %v", stack.Type)) } -func (handler *Handler) updateComposeStack(tx dataservices.DataStoreTx, r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint, gate *deployGate) *httperror.HandlerError { +func (handler *Handler) updateComposeStack(tx dataservices.DataStoreTx, r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint) (deployments.StackDeploymentConfiger, postDeployFunc, *httperror.HandlerError) { + // Must not be git based stack. stop the auto update job if there is any if stack.AutoUpdate != nil { stack.AutoUpdate = nil } @@ -244,7 +264,7 @@ func (handler *Handler) updateComposeStack(tx dataservices.DataStoreTx, r *http. var payload updateComposeStackPayload if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { - return httperror.BadRequest("Invalid request payload", err) + return nil, nil, httperror.BadRequest("Invalid request payload", err) } payload.RepullImageAndRedeploy = payload.RepullImageAndRedeploy || payload.PullImage @@ -254,7 +274,7 @@ func (handler *Handler) updateComposeStack(tx dataservices.DataStoreTx, r *http. oldWorkflowID := stack.WorkflowID stack.WorkflowID = 0 if err := workflows.DeleteIfSingleArtifact(tx, oldWorkflowID); err != nil { - return httperror.InternalServerError("Unable to remove git workflow records from database", err) + return nil, nil, httperror.InternalServerError("Unable to remove git workflow records from database", err) } } @@ -264,13 +284,13 @@ func (handler *Handler) updateComposeStack(tx dataservices.DataStoreTx, r *http. log.Warn().Err(rollbackErr).Msg("rollback stack file error") } - return httperror.InternalServerError("Unable to persist updated Compose file on disk", err) + return nil, nil, httperror.InternalServerError("Unable to persist updated Compose file on disk", err) } // Create compose deployment config securityContext, err := security.RetrieveRestrictedRequestContext(r) if err != nil { - return httperror.InternalServerError("Unable to retrieve info from request context", err) + return nil, nil, httperror.InternalServerError("Unable to retrieve info from request context", err) } composeDeploymentConfig, err := deployments.CreateComposeStackDeploymentConfigTx(tx, securityContext, @@ -286,7 +306,7 @@ func (handler *Handler) updateComposeStack(tx dataservices.DataStoreTx, r *http. log.Warn().Err(rollbackErr).Msg("rollback stack file error") } - return httperror.InternalServerError(err.Error(), err) + return nil, nil, httperror.InternalServerError(err.Error(), err) } if stack.Option != nil { @@ -310,12 +330,11 @@ func (handler *Handler) updateComposeStack(tx dataservices.DataStoreTx, r *http. } } - go stackDeploy(handler.DataStore, stack.ID, composeDeploymentConfig, gate, postDeploy) - - return nil + return composeDeploymentConfig, postDeploy, nil } -func (handler *Handler) updateSwarmStack(tx dataservices.DataStoreTx, r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint, gate *deployGate) *httperror.HandlerError { +func (handler *Handler) updateSwarmStack(tx dataservices.DataStoreTx, r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint) (deployments.StackDeploymentConfiger, postDeployFunc, *httperror.HandlerError) { + // Must not be git based stack. stop the auto update job if there is any if stack.AutoUpdate != nil { stack.AutoUpdate = nil } @@ -325,7 +344,7 @@ func (handler *Handler) updateSwarmStack(tx dataservices.DataStoreTx, r *http.Re var payload updateSwarmStackPayload if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { - return httperror.BadRequest("Invalid request payload", err) + return nil, nil, httperror.BadRequest("Invalid request payload", err) } payload.RepullImageAndRedeploy = payload.RepullImageAndRedeploy || payload.PullImage stack.Env = payload.Env @@ -334,7 +353,7 @@ func (handler *Handler) updateSwarmStack(tx dataservices.DataStoreTx, r *http.Re oldWorkflowID := stack.WorkflowID stack.WorkflowID = 0 if err := workflows.DeleteIfSingleArtifact(tx, oldWorkflowID); err != nil { - return httperror.InternalServerError("Unable to remove git workflow records from database", err) + return nil, nil, httperror.InternalServerError("Unable to remove git workflow records from database", err) } } @@ -344,13 +363,13 @@ func (handler *Handler) updateSwarmStack(tx dataservices.DataStoreTx, r *http.Re log.Warn().Err(rollbackErr).Msg("rollback stack file error") } - return httperror.InternalServerError("Unable to persist updated Compose file on disk", err) + return nil, nil, httperror.InternalServerError("Unable to persist updated Compose file on disk", err) } // Create swarm deployment config securityContext, err := security.RetrieveRestrictedRequestContext(r) if err != nil { - return httperror.InternalServerError("Unable to retrieve info from request context", err) + return nil, nil, httperror.InternalServerError("Unable to retrieve info from request context", err) } swarmDeploymentConfig, err := deployments.CreateSwarmStackDeploymentConfigTx(tx, securityContext, @@ -365,7 +384,7 @@ func (handler *Handler) updateSwarmStack(tx dataservices.DataStoreTx, r *http.Re log.Warn().Err(rollbackErr).Msg("rollback stack file error") } - return httperror.InternalServerError(err.Error(), err) + return nil, nil, httperror.InternalServerError(err.Error(), err) } if stack.Option != nil { @@ -389,16 +408,10 @@ func (handler *Handler) updateSwarmStack(tx dataservices.DataStoreTx, r *http.Re } } - go stackDeploy(handler.DataStore, stack.ID, swarmDeploymentConfig, gate, postDeploy) - - return nil + return swarmDeploymentConfig, postDeploy, nil } -func stackDeploy(dataStore dataservices.DataStore, stackID portainer.StackID, stackDeploymentConfig deployments.StackDeploymentConfiger, gate *deployGate, postDeploy postDeployFunc) { - // Wait until stack update payload is persisted - if !gate.wait() { - return - } +func stackDeploy(dataStore dataservices.DataStore, stackID portainer.StackID, stackDeploymentConfig deployments.StackDeploymentConfiger, postDeploy postDeployFunc) { ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute) defer cancel() @@ -464,3 +477,44 @@ func stackDeploy(dataStore dataservices.DataStore, stackID portainer.StackID, st postDeploy(ctx, deployErr) } } + +// stackDeployInline must be called with no transaction held, and with the stack's status already +// transitioned to Deploying and persisted (see updateStackInTx's Kubernetes branch). +func (handler *Handler) stackDeployInline(stack *portainer.Stack, stackDeploymentConfig deployments.StackDeploymentConfiger, postDeploy postDeployFunc) *httperror.HandlerError { + deployCtx, cancel := context.WithTimeout(context.Background(), stackutils.InlineDeployTimeout) + defer cancel() + + if deployErr := stackDeploymentConfig.Deploy(deployCtx); deployErr != nil { + stackutils.UpdateStackStatusFromDeploymentResult(stack, deployErr) + + if err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { + return tx.Stack().Update(stack.ID, stack) + }); err != nil { + log.Error().Err(err). + AnErr("deploy_error", deployErr). + Int("stack_id", int(stack.ID)). + Str("context", "stackDeployInline"). + Msg("Failed to persist stack deployment status after failed inline deploy") + } + + if postDeploy != nil { + postDeploy(deployCtx, deployErr) + } + + return httperror.InternalServerError("Failed to deploy stack", deployErr) + } + + stackutils.UpdateStackStatusFromDeploymentResult(stack, nil) + + if err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { + return tx.Stack().Update(stack.ID, stack) + }); err != nil { + return httperror.InternalServerError("Failed to persist stack deployment status", err) + } + + if postDeploy != nil { + postDeploy(deployCtx, nil) + } + + return nil +} diff --git a/api/http/handler/stacks/stack_update_git_redeploy.go b/api/http/handler/stacks/stack_update_git_redeploy.go index 1098d11e37..4c7118058a 100644 --- a/api/http/handler/stacks/stack_update_git_redeploy.go +++ b/api/http/handler/stacks/stack_update_git_redeploy.go @@ -231,7 +231,6 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request) stack.UpdatedBy = user.Username stack.UpdateDate = time.Now().Unix() - stackutils.PrepareStackStatusForDeployment(stack) postDeploy := func(ctx context.Context, deployErr error) { if deployErr == nil { @@ -260,11 +259,23 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request) } } - deployGate := newDeployGate() - if err := handler.deployStack(r, stack, payload.RepullImageAndRedeploy, endpoint, deployGate, postDeploy); err != nil { - return err + stackutils.PrepareStackStatusForDeployment(stack) + + deploymentConfig, httpErr := handler.deployStack(r, stack, payload.RepullImageAndRedeploy, endpoint) + if httpErr != nil { + return httpErr } + if stack.Type == portainer.KubernetesStack { + // Inline deployment workflow for Kubernetes stacks + if err := handler.deployKubernetesStackInline(deploymentConfig, stack, securityContext, gitConfig, sourceID, postDeploy); err != nil { + return err + } + + return response.JSON(w, stack) + } + + // Async deployment workflow for Swarm and Compose stacks if err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { if err := tx.Stack().Update(stack.ID, stack); err != nil { return err @@ -280,16 +291,118 @@ func (handler *Handler) stackGitRedeploy(w http.ResponseWriter, r *http.Request) return fillStackGitConfig(tx, userContext, stack) }); err != nil { - deployGate.abortDeploy() - return httperror.InternalServerError("Unable to persist the stack changes inside the database", errors.Wrap(err, "failed to update the stack")) } - deployGate.startDeploy() + if deploymentConfig != nil { + go stackDeploy(handler.DataStore, stack.ID, deploymentConfig, postDeploy) + } return response.JSON(w, stack) } +// deployStack builds the deployment config without deploying; the caller deploys it (sync for +// Kubernetes, async for Swarm/Compose). +func (handler *Handler) deployStack(r *http.Request, stack *portainer.Stack, pullImage bool, endpoint *portainer.Endpoint) (deployments.StackDeploymentConfiger, *httperror.HandlerError) { + switch stack.Type { + case portainer.DockerSwarmStack: + securityContext, err := security.RetrieveRestrictedRequestContext(r) + if err != nil { + return nil, httperror.InternalServerError("Unable to retrieve info from request context", err) + } + + prune := stack.Option != nil && stack.Option.Prune + + deploymentConfiger, err := deployments.CreateSwarmStackDeploymentConfigTx(handler.DataStore, securityContext, stack, endpoint, handler.FileService, handler.StackDeployer, prune, pullImage) + if err != nil { + return nil, httperror.InternalServerError(err.Error(), err) + } + + return deploymentConfiger, nil + + case portainer.DockerComposeStack: + securityContext, err := security.RetrieveRestrictedRequestContext(r) + if err != nil { + return nil, httperror.InternalServerError("Unable to retrieve info from request context", err) + } + + prune := stack.Option != nil && stack.Option.Prune + + deploymentConfiger, err := deployments.CreateComposeStackDeploymentConfigTx(handler.DataStore, securityContext, stack, endpoint, handler.FileService, handler.StackDeployer, prune, pullImage, true) + if err != nil { + return nil, httperror.InternalServerError(err.Error(), err) + } + + return deploymentConfiger, nil + + case portainer.KubernetesStack: + tokenData, err := security.RetrieveTokenData(r) + if err != nil { + return nil, httperror.BadRequest("Failed to retrieve user token data", err) + } + + user := &portainer.User{ + ID: tokenData.ID, + Username: tokenData.Username, + } + + appLabel := k.KubeAppLabels{ + StackID: int(stack.ID), + StackName: stack.Name, + Owner: tokenData.Username, + Kind: "git", + } + + return deployments.CreateKubernetesStackDeploymentConfig(stack, handler.KubernetesDeployer, appLabel, user, endpoint), nil + } + + return nil, httperror.InternalServerError("Unsupported stack", errors.Errorf("unsupported stack type: %v", stack.Type)) +} + +// deployKubernetesStackInline deploys an already-built Kubernetes deployment config synchronously +// and returns any deploy failure to the caller, instead of the goroutine used for Swarm/Compose. +func (handler *Handler) deployKubernetesStackInline(deploymentConfiger deployments.StackDeploymentConfiger, stack *portainer.Stack, securityContext *security.RestrictedRequestContext, gitConfig *gittypes.RepoConfig, sourceID portainer.SourceID, postDeploy postDeployFunc) *httperror.HandlerError { + handler.stackCreationMutex.Lock() + defer handler.stackCreationMutex.Unlock() + + deployCtx, cancel := context.WithTimeout(context.Background(), stackutils.InlineDeployTimeout) + defer cancel() + + if err := deploymentConfiger.Deploy(deployCtx); err != nil { + if postDeploy != nil { + postDeploy(deployCtx, err) + } + + return httperror.InternalServerError("Failed to deploy stack", err) + } + + if err := handler.DataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { + stackutils.UpdateStackStatusFromDeploymentResult(stack, nil) + if err := tx.Stack().Update(stack.ID, stack); err != nil { + return err + } + + userContext := source.NewUserContext(securityContext.User, securityContext.UserMemberships) + if err := saveStackGitConfig(tx, userContext, stack.WorkflowID, stack.ID, sourceID, 0, gitConfig); err != nil { + return err + } + + if err := workflows.SaveSourceStatus(tx, userContext, sourceID, nil); err != nil { + return err + } + + return fillStackGitConfig(tx, userContext, stack) + }); err != nil { + return httperror.InternalServerError("Unable to persist the stack changes inside the database", errors.Wrap(err, "failed to update the stack")) + } + + if postDeploy != nil { + postDeploy(deployCtx, nil) + } + + return nil +} + func resolveGitAuthFromRedeployPayload(gitConfig *gittypes.RepoConfig, payload stackGitRedeployPayload) gittypes.GitAuthentication { auth := gittypes.GitAuthentication{} if gitConfig.Authentication != nil { @@ -303,67 +416,3 @@ func resolveGitAuthFromRedeployPayload(gitConfig *gittypes.RepoConfig, payload s } return auth } - -func (handler *Handler) deployStack(r *http.Request, stack *portainer.Stack, pullImage bool, endpoint *portainer.Endpoint, gate *deployGate, postDeploy postDeployFunc) *httperror.HandlerError { - var deploymentConfiger deployments.StackDeploymentConfiger - - switch stack.Type { - case portainer.DockerSwarmStack: - // Create swarm deployment config - securityContext, err := security.RetrieveRestrictedRequestContext(r) - if err != nil { - return httperror.InternalServerError("Unable to retrieve info from request context", err) - } - - prune := stack.Option != nil && stack.Option.Prune - - deploymentConfiger, err = deployments.CreateSwarmStackDeploymentConfigTx(handler.DataStore, securityContext, stack, endpoint, handler.FileService, handler.StackDeployer, prune, pullImage) - if err != nil { - return httperror.InternalServerError(err.Error(), err) - } - - case portainer.DockerComposeStack: - // Create compose deployment config - securityContext, err := security.RetrieveRestrictedRequestContext(r) - if err != nil { - return httperror.InternalServerError("Unable to retrieve info from request context", err) - } - - prune := stack.Option != nil && stack.Option.Prune - - deploymentConfiger, err = deployments.CreateComposeStackDeploymentConfigTx(handler.DataStore, securityContext, stack, endpoint, handler.FileService, handler.StackDeployer, prune, pullImage, true) - if err != nil { - return httperror.InternalServerError(err.Error(), err) - } - - case portainer.KubernetesStack: - handler.stackCreationMutex.Lock() - defer handler.stackCreationMutex.Unlock() - - tokenData, err := security.RetrieveTokenData(r) - if err != nil { - return httperror.BadRequest("Failed to retrieve user token data", err) - } - - user := &portainer.User{ - ID: tokenData.ID, - Username: tokenData.Username, - } - - appLabel := k.KubeAppLabels{ - StackID: int(stack.ID), - StackName: stack.Name, - Owner: tokenData.Username, - Kind: "git", - } - - deploymentConfiger = deployments.CreateKubernetesStackDeploymentConfig(stack, handler.KubernetesDeployer, appLabel, user, endpoint) - - default: - return httperror.InternalServerError("Unsupported stack", errors.Errorf("unsupported stack type: %v", stack.Type)) - } - - go stackDeploy(handler.DataStore, stack.ID, deploymentConfiger, gate, postDeploy) - - return nil -} diff --git a/api/http/handler/stacks/stack_update_git_redeploy_test.go b/api/http/handler/stacks/stack_update_git_redeploy_test.go index 1bcf8a0e7b..228a21bf69 100644 --- a/api/http/handler/stacks/stack_update_git_redeploy_test.go +++ b/api/http/handler/stacks/stack_update_git_redeploy_test.go @@ -1,12 +1,40 @@ package stacks import ( + "context" + "errors" + "net/http" + "os" + "sync" "testing" + portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/dataservices" + "github.com/portainer/portainer/api/dataservices/source" + "github.com/portainer/portainer/api/datastore" + "github.com/portainer/portainer/api/filesystem" gittypes "github.com/portainer/portainer/api/git/types" + "github.com/portainer/portainer/api/http/security" + "github.com/portainer/portainer/api/stacks/stackutils" + httperror "github.com/portainer/portainer/pkg/libhttp/error" "github.com/stretchr/testify/require" ) +type stubKubernetesDeployer struct { + portainer.KubernetesDeployer + deployErr error +} + +func (f *stubKubernetesDeployer) Deploy(_ context.Context, _ portainer.UserID, _ *portainer.Endpoint, _ []string, _ string) (string, error) { + return "", f.deployErr +} + +func mockDeployKubernetesStackInlineRequest() *http.Request { + req := mockCreateStackRequestWithSecurityContext(http.MethodPut, "/stacks/1/git/redeploy", nil) + + return req.WithContext(security.StoreTokenData(req, &portainer.TokenData{ID: 1, Username: "admin", Role: portainer.AdministratorRole})) +} + func TestResolveGitAuthFromRedeployPayload(t *testing.T) { t.Parallel() @@ -80,3 +108,150 @@ func TestResolveGitAuthFromRedeployPayload(t *testing.T) { }) } } + +func setupDeployKubernetesStackInlineTest(t *testing.T, deployErr error, initialStatus portainer.StackStatus) (*Handler, *portainer.Stack, *gittypes.RepoConfig, portainer.SourceID, *security.RestrictedRequestContext) { + t.Helper() + + var manifest = `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-config + namespace: default +data: + key: value +` + var configHash = "testhash" + tempDir := t.TempDir() + require.NoError(t, os.WriteFile(filesystem.JoinPaths(tempDir, "manifest.yml"), []byte(manifest), 0o644)) + + _, store := datastore.MustNewTestStore(t, true, false) + + adminUserContext := source.InsecureNewAdminContext() + + src := &portainer.Source{ + Type: portainer.SourceTypeGit, + Git: &gittypes.GitSource{URL: "https://example.com/repo.git"}, + } + require.NoError(t, store.Source().Create(adminUserContext, src)) + + stack := &portainer.Stack{ + ID: 1, + Name: "k8s-stack", + Type: portainer.KubernetesStack, + EndpointID: 1, + Namespace: "default", + ProjectPath: tempDir, + EntryPoint: "manifest.yml", + Status: initialStatus, + } + + wf := &portainer.Workflow{ + Artifacts: []portainer.Artifact{{ + StackID: stack.ID, + Files: []portainer.ArtifactFile{{SourceID: src.ID, Ref: "refs/heads/main", Hash: configHash}}, + }}, + } + require.NoError(t, store.Workflow().Create(wf)) + stack.WorkflowID = wf.ID + + require.NoError(t, store.Stack().Create(stack)) + + handler := &Handler{ + DataStore: store, + KubernetesDeployer: &stubKubernetesDeployer{deployErr: deployErr}, + stackCreationMutex: &sync.Mutex{}, + } + + gitConfig := &gittypes.RepoConfig{URL: src.Git.URL, ReferenceName: "refs/heads/main", ConfigHash: configHash} + securityContext := &security.RestrictedRequestContext{ + IsAdmin: true, + UserID: 1, + User: &portainer.User{ID: 1, Role: portainer.AdministratorRole}, + } + + return handler, stack, gitConfig, src.ID, securityContext +} + +func TestDeployKubernetesStackInline(t *testing.T) { + t.Parallel() + + t.Run("successful redeploys persist Active status inline, with no goroutine to wait on, and reset DeploymentStatus on each attempt", func(t *testing.T) { + t.Parallel() + + handler, stack, gitConfig, sourceID, securityContext := setupDeployKubernetesStackInlineTest(t, nil, portainer.StackStatusActive) + req := mockDeployKubernetesStackInlineRequest() + + var postDeployCalls int + postDeploy := func(_ context.Context, _ error) { + postDeployCalls++ + } + + deployOnce := func() *httperror.HandlerError { + stackutils.PrepareStackStatusForDeployment(stack) + deploymentConfig, httpErr := handler.deployStack(req, stack, false, &portainer.Endpoint{}) + require.Nil(t, httpErr) + return handler.deployKubernetesStackInline(deploymentConfig, stack, securityContext, gitConfig, sourceID, postDeploy) + } + + httpErr := deployOnce() + require.Nil(t, httpErr) + + var updated *portainer.Stack + require.NoError(t, handler.DataStore.ViewTx(func(tx dataservices.DataStoreTx) error { + var err error + updated, err = tx.Stack().Read(stack.ID) + return err + })) + require.Equal(t, portainer.StackStatusActive, updated.Status) + require.Len(t, updated.DeploymentStatus, 2, "expected a Deploying entry followed by an Active entry") + require.Equal(t, portainer.StackStatusActive, updated.DeploymentStatus[1].Status) + require.Equal(t, 1, postDeployCalls, "postDeploy should be called after a successful inline deploy") + + httpErr = deployOnce() + require.Nil(t, httpErr) + + require.NoError(t, handler.DataStore.ViewTx(func(tx dataservices.DataStoreTx) error { + var err error + updated, err = tx.Stack().Read(stack.ID) + return err + })) + require.Equal(t, portainer.StackStatusActive, updated.Status) + require.Len(t, updated.DeploymentStatus, 2, "DeploymentStatus should be reset on each redeploy, not accumulate across redeploys") + require.Equal(t, portainer.StackStatusActive, updated.DeploymentStatus[1].Status) + require.Equal(t, 2, postDeployCalls) + }) + + t.Run("failed deploy returns an error, leaves the stack untouched, and still calls postDeploy with the error", func(t *testing.T) { + t.Parallel() + + deployErr := errors.New("failed to apply resources") + handler, stack, gitConfig, sourceID, securityContext := setupDeployKubernetesStackInlineTest(t, deployErr, portainer.StackStatusActive) + req := mockDeployKubernetesStackInlineRequest() + + var postDeployCalled bool + var postDeployErr error + postDeploy := func(_ context.Context, err error) { + postDeployCalled = true + postDeployErr = err + } + + stackutils.PrepareStackStatusForDeployment(stack) + deploymentConfig, httpErr := handler.deployStack(req, stack, false, &portainer.Endpoint{}) + require.Nil(t, httpErr) + + httpErr = handler.deployKubernetesStackInline(deploymentConfig, stack, securityContext, gitConfig, sourceID, postDeploy) + require.NotNil(t, httpErr) + + var unchanged *portainer.Stack + require.NoError(t, handler.DataStore.ViewTx(func(tx dataservices.DataStoreTx) error { + var err error + unchanged, err = tx.Stack().Read(stack.ID) + return err + })) + require.Equal(t, portainer.StackStatusActive, unchanged.Status, "status should be untouched since nothing should be persisted on deploy failure") + require.Empty(t, unchanged.DeploymentStatus, "no deployment status entry should be recorded on failure") + + require.True(t, postDeployCalled, "postDeploy should be called on failure too, consistent with the file-based inline deploy path") + require.ErrorIs(t, postDeployErr, deployErr) + }) +} diff --git a/api/http/handler/stacks/stack_update_kubernetes_test.go b/api/http/handler/stacks/stack_update_kubernetes_test.go new file mode 100644 index 0000000000..08d6d1c2f7 --- /dev/null +++ b/api/http/handler/stacks/stack_update_kubernetes_test.go @@ -0,0 +1,159 @@ +package stacks + +import ( + "bytes" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strconv" + "testing" + + portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/datastore" + "github.com/portainer/portainer/api/filesystem" + "github.com/portainer/portainer/api/http/security" + "github.com/portainer/portainer/api/internal/testhelpers" + "github.com/portainer/portainer/api/kubernetes/cli" + + "github.com/portainer/portainer/api/dataservices" + "github.com/segmentio/encoding/json" + "github.com/stretchr/testify/require" +) + +const kubernetesFileStackTestManifest = `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-config + namespace: default +data: + key: value +` + +const kubernetesFileStackUpdatedTestManifest = `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-config + namespace: default +data: + key: updated-value +` + +func mockUpdateKubernetesStackFileContentRequest(stackID portainer.StackID, endpointID portainer.EndpointID, payload []byte) *http.Request { + target := fmt.Sprintf("/stacks/%d?endpointId=%d", stackID, endpointID) + req := mockCreateStackRequestWithSecurityContext(http.MethodPut, target, bytes.NewBuffer(payload)) + + return req.WithContext(security.StoreTokenData(req, &portainer.TokenData{ID: 1, Username: "admin", Role: portainer.AdministratorRole})) +} + +// KubernetesClientFactory must be non-nil: updateKubernetesStack calls GetPrivilegedKubeClient on +// it unconditionally, which panics on a nil receiver. +func setupStackUpdateKubernetesTest(t *testing.T, deployErr error) (*Handler, *portainer.Stack, *portainer.Endpoint) { + t.Helper() + + _, store := datastore.MustNewTestStore(t, false, true) + + testDataPath := filesystem.JoinPaths(t.TempDir()) + fileService, err := filesystem.NewService(testDataPath, "") + require.NoError(t, err, "error init file service") + + _, err = mockCreateUser(store) + require.NoError(t, err, "error creating user") + + endpoint, err := mockCreateEndpoint(store) + require.NoError(t, err, "error creating endpoint") + + stack := &portainer.Stack{ + ID: 1, + Name: "k8s-file-stack", + Type: portainer.KubernetesStack, + EndpointID: endpoint.ID, + EntryPoint: "manifest.yml", + Namespace: "default", + Status: portainer.StackStatusActive, + } + stack.ProjectPath = fileService.GetStackProjectPath(strconv.Itoa(int(stack.ID))) + require.NoError(t, store.Stack().Create(stack)) + + _, err = fileService.StoreStackFileFromBytes( + strconv.Itoa(int(stack.ID)), + stack.EntryPoint, + []byte(kubernetesFileStackTestManifest), + ) + require.NoError(t, err, "error storing stack file") + + clientFactory, err := cli.NewClientFactory(nil, nil, store, "test-instance", "", "") + require.NoError(t, err, "error creating kubernetes client factory") + + handler := NewHandler(testhelpers.NewTestRequestBouncer(), nil) + handler.DataStore = store + handler.FileService = fileService + handler.KubernetesDeployer = &stubKubernetesDeployer{deployErr: deployErr} + handler.KubernetesClientFactory = clientFactory + + return handler, stack, endpoint +} + +func TestStackUpdate_KubernetesFileContent(t *testing.T) { + t.Parallel() + + t.Run("successful deploy persists Active status", func(t *testing.T) { + t.Parallel() + + handler, stack, endpoint := setupStackUpdateKubernetesTest(t, nil) + + payload := kubernetesFileStackUpdatePayload{ + StackFileContent: kubernetesFileStackUpdatedTestManifest, + StackName: stack.Name, + } + jsonPayload, err := json.Marshal(payload) + require.NoError(t, err) + + req := mockUpdateKubernetesStackFileContentRequest(stack.ID, endpoint.ID, jsonPayload) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + + var updated *portainer.Stack + require.NoError(t, handler.DataStore.ViewTx(func(tx dataservices.DataStoreTx) error { + var err error + updated, err = tx.Stack().Read(stack.ID) + return err + })) + require.Equal(t, portainer.StackStatusActive, updated.Status) + require.Len(t, updated.DeploymentStatus, 2, "expected a Deploying entry followed by an Active entry") + require.Equal(t, portainer.StackStatusActive, updated.DeploymentStatus[1].Status) + }) + + t.Run("failed deploy transitions status to Error so the stack isn't stuck showing Deploying", func(t *testing.T) { + t.Parallel() + + deployErr := errors.New("failed to apply resources") + handler, stack, endpoint := setupStackUpdateKubernetesTest(t, deployErr) + + payload := kubernetesFileStackUpdatePayload{ + StackFileContent: kubernetesFileStackUpdatedTestManifest, + StackName: stack.Name, + } + jsonPayload, err := json.Marshal(payload) + require.NoError(t, err) + + req := mockUpdateKubernetesStackFileContentRequest(stack.ID, endpoint.ID, jsonPayload) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + require.Equal(t, http.StatusInternalServerError, rec.Code, rec.Body.String()) + + var updated *portainer.Stack + require.NoError(t, handler.DataStore.ViewTx(func(tx dataservices.DataStoreTx) error { + var err error + updated, err = tx.Stack().Read(stack.ID) + return err + })) + require.Equal(t, portainer.StackStatusError, updated.Status, "status should transition to Error, not stay stuck at Deploying, so the stack can be retried") + require.Len(t, updated.DeploymentStatus, 2, "expected the persisted Deploying entry followed by an Error entry") + require.Equal(t, portainer.StackStatusDeploying, updated.DeploymentStatus[0].Status) + require.Equal(t, portainer.StackStatusError, updated.DeploymentStatus[1].Status) + }) +} diff --git a/api/http/handler/stacks/stack_update_test.go b/api/http/handler/stacks/stack_update_test.go index 745f6a7062..309a4911f5 100644 --- a/api/http/handler/stacks/stack_update_test.go +++ b/api/http/handler/stacks/stack_update_test.go @@ -16,6 +16,7 @@ import ( "github.com/portainer/portainer/api/datastore" "github.com/portainer/portainer/api/filesystem" "github.com/portainer/portainer/api/internal/testhelpers" + "github.com/portainer/portainer/api/stacks/deployments" "github.com/portainer/portainer/api/stacks/stackutils" "github.com/portainer/portainer/pkg/fips" httperror "github.com/portainer/portainer/pkg/libhttp/error" @@ -40,7 +41,7 @@ func Test_updateStackInTx(t *testing.T) { // Execute updateStackInTx within a successful transaction err := setup.store.UpdateTx(func(tx dataservices.DataStoreTx) error { - _, handlerErr := setup.handler.updateStackInTx(tx, setup.req, setup.stack.ID, setup.endpoint.ID) + _, _, _, handlerErr := setup.handler.updateStackInTx(tx, setup.req, setup.stack.ID, setup.endpoint.ID) if handlerErr != nil { return handlerErr } @@ -70,7 +71,7 @@ func Test_updateStackInTx(t *testing.T) { // Execute updateStackInTx within a transaction that we force to fail err := setup.store.UpdateTx(func(tx dataservices.DataStoreTx) error { - updatedStack, handlerErr := setup.handler.updateStackInTx(tx, setup.req, setup.stack.ID, setup.endpoint.ID) + updatedStack, _, _, handlerErr := setup.handler.updateStackInTx(tx, setup.req, setup.stack.ID, setup.endpoint.ID) if handlerErr != nil { return handlerErr } @@ -109,7 +110,7 @@ func Test_updateStackInTx(t *testing.T) { var handlerErr *httperror.HandlerError _ = setup.store.UpdateTx(func(tx dataservices.DataStoreTx) error { - _, handlerErr = setup.handler.updateStackInTx(tx, setup.req, 9999, setup.endpoint.ID) + _, _, _, handlerErr = setup.handler.updateStackInTx(tx, setup.req, 9999, setup.endpoint.ID) return handlerErr }) @@ -132,7 +133,7 @@ func Test_updateStackInTx(t *testing.T) { var handlerErr *httperror.HandlerError _ = setup.store.UpdateTx(func(tx dataservices.DataStoreTx) error { - _, handlerErr = setup.handler.updateStackInTx(tx, setup.req, stack.ID, 2999) // Non-existent endpoint ID + _, _, _, handlerErr = setup.handler.updateStackInTx(tx, setup.req, stack.ID, 2999) // Non-existent endpoint ID return nil }) @@ -162,7 +163,7 @@ func Test_updateStackInTx(t *testing.T) { var handlerErr *httperror.HandlerError _ = setup.store.UpdateTx(func(tx dataservices.DataStoreTx) error { - _, handlerErr = setup.handler.updateStackInTx(tx, setup.req, stack.ID, stack.EndpointID) + _, _, _, handlerErr = setup.handler.updateStackInTx(tx, setup.req, stack.ID, stack.EndpointID) return nil }) @@ -187,7 +188,7 @@ func Test_updateStackInTx(t *testing.T) { var handlerErr *httperror.HandlerError _ = setup.store.UpdateTx(func(tx dataservices.DataStoreTx) error { - _, handlerErr = setup.handler.updateStackInTx(tx, setup.req, stack.ID, stack.EndpointID) + _, _, _, handlerErr = setup.handler.updateStackInTx(tx, setup.req, stack.ID, stack.EndpointID) return nil }) @@ -422,8 +423,11 @@ func Test_updateSwarmStack_Prune(t *testing.T) { deployer := testhelpers.NewTestStackDeployer() setup.handler.StackDeployer = deployer + var deploymentConfig deployments.StackDeploymentConfiger + var postDeploy postDeployFunc err := setup.store.UpdateTx(func(tx dataservices.DataStoreTx) error { - _, handlerErr := setup.handler.updateStackInTx(tx, setup.req, setup.stack.ID, setup.endpoint.ID) + var handlerErr *httperror.HandlerError + _, deploymentConfig, postDeploy, handlerErr = setup.handler.updateStackInTx(tx, setup.req, setup.stack.ID, setup.endpoint.ID) if handlerErr != nil { return handlerErr } @@ -436,7 +440,8 @@ func Test_updateSwarmStack_Prune(t *testing.T) { require.NotNil(t, stored.Option, "stack.Option should not be nil") assert.True(t, stored.Option.Prune, "stack.Option.Prune should be persisted as true") - // Deploy runs asynchronously; wait for the goroutine to call the deployer + go stackDeploy(setup.store, setup.stack.ID, deploymentConfig, postDeploy) + require.Eventually(t, func() bool { return deployer.DeploySwarmCallCount == 1 }, 5*time.Second, 10*time.Millisecond, "DeploySwarmStack should be called exactly once") @@ -461,8 +466,11 @@ func Test_updateComposeStack_Prune(t *testing.T) { deployer := testhelpers.NewTestStackDeployer() setup.handler.StackDeployer = deployer + var deploymentConfig deployments.StackDeploymentConfiger + var postDeploy postDeployFunc err := setup.store.UpdateTx(func(tx dataservices.DataStoreTx) error { - _, handlerErr := setup.handler.updateStackInTx(tx, setup.req, setup.stack.ID, setup.endpoint.ID) + var handlerErr *httperror.HandlerError + _, deploymentConfig, postDeploy, handlerErr = setup.handler.updateStackInTx(tx, setup.req, setup.stack.ID, setup.endpoint.ID) if handlerErr != nil { return handlerErr } @@ -475,7 +483,8 @@ func Test_updateComposeStack_Prune(t *testing.T) { require.NotNil(t, stored.Option, "stack.Option should not be nil") assert.True(t, stored.Option.Prune, "stack.Option.Prune should be persisted as true") - // Deploy runs asynchronously; wait for the goroutine to call the deployer + go stackDeploy(setup.store, setup.stack.ID, deploymentConfig, postDeploy) + require.Eventually(t, func() bool { return deployer.DeployComposeCallCount == 1 }, 5*time.Second, 10*time.Millisecond, "DeployComposeStack should be called exactly once") diff --git a/api/http/handler/stacks/update_kubernetes_stack.go b/api/http/handler/stacks/update_kubernetes_stack.go index 45827ce851..ba30b1adab 100644 --- a/api/http/handler/stacks/update_kubernetes_stack.go +++ b/api/http/handler/stacks/update_kubernetes_stack.go @@ -54,27 +54,27 @@ func (payload *kubernetesGitStackUpdatePayload) Validate(r *http.Request) error return nil } -func (handler *Handler) updateKubernetesStack(tx dataservices.DataStoreTx, r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint, gate *deployGate) *httperror.HandlerError { +func (handler *Handler) updateKubernetesStack(tx dataservices.DataStoreTx, r *http.Request, stack *portainer.Stack, endpoint *portainer.Endpoint) (deployments.StackDeploymentConfiger, postDeployFunc, *httperror.HandlerError) { securityContext, err := security.RetrieveRestrictedRequestContext(r) if err != nil { - return httperror.InternalServerError("Unable to retrieve info from request context", err) + return nil, nil, httperror.InternalServerError("Unable to retrieve info from request context", err) } userContext := source.NewUserContext(securityContext.User, securityContext.UserMemberships) if stack.WorkflowID != 0 { gitConfig, sourceID, err := loadGitConfigForStack(tx, userContext, stack.WorkflowID, stack.ID) if err != nil { - return httperror.InternalServerError("Unable to load git config for stack", err) + return nil, nil, httperror.InternalServerError("Unable to load git config for stack", err) } if gitConfig == nil { - return httperror.InternalServerError("Stack has no git config in source", errors.New("source has no git config")) + return nil, nil, httperror.InternalServerError("Stack has no git config in source", errors.New("source has no git config")) } var payload kubernetesGitStackUpdatePayload if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { - return httperror.BadRequest("Invalid request payload", err) + return nil, nil, httperror.BadRequest("Invalid request payload", err) } gitConfig.ReferenceName = payload.RepositoryReferenceName @@ -100,40 +100,40 @@ func (handler *Handler) updateKubernetesStack(tx dataservices.DataStoreTx, r *ht gitConfig.Authentication.Password, gitConfig.TLSSkipVerify, ); err != nil { - return httperror.InternalServerError("Unable to fetch git repository", err) + return nil, nil, httperror.InternalServerError("Unable to fetch git repository", err) } } else { gitConfig.Authentication = nil } if err := saveStackGitConfig(tx, userContext, stack.WorkflowID, stack.ID, sourceID, 0, gitConfig); err != nil { - return httperror.InternalServerError("Unable to update source git config", err) + return nil, nil, httperror.InternalServerError("Unable to update source git config", err) } - return nil + return nil, nil, nil } var payload kubernetesFileStackUpdatePayload if err := request.DecodeAndValidateJSONPayload(r, &payload); err != nil { - return httperror.BadRequest("Invalid request payload", err) + return nil, nil, httperror.BadRequest("Invalid request payload", err) } tokenData, err := security.RetrieveTokenData(r) if err != nil { - return httperror.BadRequest("Failed to retrieve user token data", err) + return nil, nil, httperror.BadRequest("Failed to retrieve user token data", err) } tempFileDir, _ := os.MkdirTemp("", "kub_file_content") if err := filesystem.WriteToFile(filesystem.JoinPaths(tempFileDir, stack.EntryPoint), []byte(payload.StackFileContent)); err != nil { - return httperror.InternalServerError("Failed to persist deployment file in a temp directory", err) + return nil, nil, httperror.InternalServerError("Failed to persist deployment file in a temp directory", err) } if payload.StackName != stack.Name { stack.Name = payload.StackName if err := handler.DataStore.Stack().Update(stack.ID, stack); err != nil { - return httperror.InternalServerError("Failed to update stack name", err) + return nil, nil, httperror.InternalServerError("Failed to update stack name", err) } } @@ -169,7 +169,7 @@ func (handler *Handler) updateKubernetesStack(tx dataservices.DataStoreTx, r *ht log.Warn().Err(rollbackErr).Msg("rollback stack file error") } - return httperror.InternalServerError("Unable to persist Kubernetes Manifest file on disk", err) + return nil, nil, httperror.InternalServerError("Unable to persist Kubernetes Manifest file on disk", err) } stack.ProjectPath = projectPath @@ -187,7 +187,5 @@ func (handler *Handler) updateKubernetesStack(tx dataservices.DataStoreTx, r *ht } } - go stackDeploy(handler.DataStore, copyStack.ID, k8sDeploymentConfig, gate, postDeploy) - - return nil + return k8sDeploymentConfig, postDeploy, nil } diff --git a/api/stacks/deployments/deploy.go b/api/stacks/deployments/deploy.go index 8af03137a0..5f0d0d1890 100644 --- a/api/stacks/deployments/deploy.go +++ b/api/stacks/deployments/deploy.go @@ -190,6 +190,8 @@ func redeployWhenChangedSecondStage( return errors.WithMessagef(err, "failed to set the deploying status for stack %v", stack.ID) } + previousDeploymentInfo := stack.CurrentDeploymentInfo + stack.CurrentDeploymentInfo = &portainer.StackDeploymentInfo{ RepositoryURL: gitConfig.URL, ReferenceName: gitConfig.ReferenceName, @@ -242,6 +244,9 @@ func redeployWhenChangedSecondStage( } deployErr := redeployStack(stack) + if deployErr != nil { + stack.CurrentDeploymentInfo = previousDeploymentInfo + } if err := datastore.UpdateTx(func(tx dataservices.DataStoreTx) error { stack.UpdateDate = time.Now().Unix() @@ -251,6 +256,10 @@ func redeployWhenChangedSecondStage( return err } + if deployErr != nil { + return nil + } + newHash := gitConfig.ConfigHash return workflows.UpdateArtifactFileForStack(tx, stack.WorkflowID, stack.ID, gitSrc.ID, func(a *portainer.ArtifactFile) { diff --git a/api/stacks/deployments/deploy_test.go b/api/stacks/deployments/deploy_test.go index 39f8496bd9..6a0851c330 100644 --- a/api/stacks/deployments/deploy_test.go +++ b/api/stacks/deployments/deploy_test.go @@ -308,7 +308,7 @@ func setupRedeployStore(t *testing.T, stackType portainer.StackType, stackID por err = store.Source().Create(adminUserContext, src) require.NoError(t, err, "failed to create source") - wf := &portainer.Workflow{Artifacts: []portainer.Artifact{{Files: []portainer.ArtifactFile{{SourceID: src.ID}}}}} + wf := &portainer.Workflow{Artifacts: []portainer.Artifact{{StackID: stackID, Files: []portainer.ArtifactFile{{SourceID: src.ID}}}}} err = store.Workflow().Create(wf) require.NoError(t, err, "failed to create workflow") @@ -352,6 +352,42 @@ func Test_redeployWhenChanged_KubernetesStack(t *testing.T) { require.NoError(t, err) } +type failingDeployer struct { + noopDeployer + deployErr error +} + +func (f failingDeployer) DeployKubernetesStack(_ context.Context, stack *portainer.Stack, endpoint *portainer.Endpoint, user *portainer.User) error { + return f.deployErr +} + +func Test_redeployWhenChanged_KubernetesStack_DeployFailure_DoesNotAdvanceArtifactHash(t *testing.T) { + t.Parallel() + + store, stackID := setupRedeployStore(t, portainer.KubernetesStack, 7) + deployErr := errors.New("failed to apply resources") + + err := RedeployWhenChanged(t.Context(), stackID, failingDeployer{deployErr: deployErr}, store, testhelpers.NewGitService(nil, "newHash")) + require.NoError(t, err, "a failed deploy is recorded on the stack, not returned as a scheduler error") + + var updated *portainer.Stack + err = store.ViewTx(func(tx dataservices.DataStoreTx) error { + var rerr error + updated, rerr = tx.Stack().Read(stackID) + return rerr + }) + require.NoError(t, err) + assert.Equal(t, portainer.StackStatusError, updated.Status) + assert.Nil(t, updated.CurrentDeploymentInfo, "CurrentDeploymentInfo should stay at its pre-attempt value (nil here) since nothing was actually deployed") + + workflows, err := store.Workflow().ReadAll() + require.NoError(t, err) + require.Len(t, workflows, 1) + require.Len(t, workflows[0].Artifacts, 1) + require.Len(t, workflows[0].Artifacts[0].Files, 1) + assert.Empty(t, workflows[0].Artifacts[0].Files[0].Hash, "artifact hash should stay at its old value (empty, in this fixture) since the deploy failed - the UI's git banner reads this field") +} + func Test_getUserRegistries(t *testing.T) { t.Parallel() _, store := datastore.MustNewTestStore(t, false, true) diff --git a/api/stacks/stackbuilders/director.go b/api/stacks/stackbuilders/director.go index f0a9ea7f1b..b29fc1973f 100644 --- a/api/stacks/stackbuilders/director.go +++ b/api/stacks/stackbuilders/director.go @@ -20,12 +20,13 @@ type stackBuildProcess interface { prepare(ctx context.Context, payload *StackPayload, userID portainer.UserID) error saveStack() (*portainer.Stack, error) deploy(ctx context.Context, endpoint *portainer.Endpoint) error + cleanUp() error // postDeploy runs after a successful deployment: for git builders it enables // auto-update; for other builders it is a no-op. postDeploy(ctx context.Context, stack *portainer.Stack) error } -// Build executes the stack build process. It returns the created stack and any +// BuildAndAsyncDeploy executes the stack build process. It returns the created stack and any // error encountered during the process. The returned error is of type // *httperror.HandlerError, which could be an InternalServerError depending on // the error encountered during the stack build process. @@ -33,9 +34,11 @@ type stackBuildProcess interface { // The stack is saved to DB with Status=Deploying and returned immediately. // Deployment runs in a background goroutine. The caller must poll // GET /stacks/{id} to track completion. -func Build(ctx context.Context, dataStore dataservices.DataStore, builder stackBuildProcess, payload *StackPayload, endpoint *portainer.Endpoint, userID portainer.UserID) (*portainer.Stack, *httperror.HandlerError) { +func BuildAndAsyncDeploy(ctx context.Context, dataStore dataservices.DataStore, builder stackBuildProcess, payload *StackPayload, endpoint *portainer.Endpoint, userID portainer.UserID) (*portainer.Stack, *httperror.HandlerError) { builder.setGeneralInfo(payload, endpoint) + defer func() { _ = builder.cleanUp() }() + if err := builder.prepare(ctx, payload, userID); err != nil { return nil, httperror.InternalServerError("Failed to prepare stack", err) } @@ -50,6 +53,56 @@ func Build(ctx context.Context, dataStore dataservices.DataStore, builder stackB return stack, nil } +// BuildAndDeploy is like BuildAndAsyncDeploy, but deploys inline and returns any deploy failure to +// the caller. +// +// The async wrapper hid real failures: an apply that errors before even reaching the cluster +// (e.g. a missing namespace) only recorded its error on the stack, in the background +// (BE-13174). The Applications List reads live from the cluster rather than from stack records, so +// surfacing it there would mean reworking that list's data source - and its delete and other +// actions - to understand stack records too. Deploying inline and returning the error is the +// simpler, safer fix. +// +// Separately, Kubernetes doesn't need Portainer's own async wrapper: kubectl apply returns as soon +// as the API server accepts the manifest, and the cluster reconciles it asynchronously on its own. +// Swarm and Compose deploys can run long (image pulls, etc.), so they keep using +// BuildAndAsyncDeploy. +func BuildAndDeploy(ctx context.Context, dataStore dataservices.DataStore, builder stackBuildProcess, payload *StackPayload, endpoint *portainer.Endpoint, userID portainer.UserID) (*portainer.Stack, *httperror.HandlerError) { + builder.setGeneralInfo(payload, endpoint) + + defer func() { _ = builder.cleanUp() }() + + if err := builder.prepare(ctx, payload, userID); err != nil { + return nil, httperror.InternalServerError("Failed to prepare stack", err) + } + + // Not tied to ctx: canceling on client disconnect could orphan cluster resources with no stack record. + deployCtx, cancel := context.WithTimeout(context.Background(), stackutils.InlineDeployTimeout) + defer cancel() + + if err := builder.deploy(deployCtx, endpoint); err != nil { + return nil, httperror.InternalServerError("Failed to deploy stack", err) + } + + stack, err := builder.saveStack() + if err != nil { + return nil, httperror.InternalServerError("Failed to save stack", err) + } + + if err := dataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { + stackutils.UpdateStackStatusFromDeploymentResult(stack, nil) + return tx.Stack().Update(stack.ID, stack) + }); err != nil { + return stack, httperror.InternalServerError("Failed to persist stack deployment status", err) + } + + if err := builder.postDeploy(ctx, stack); err != nil { + return stack, httperror.InternalServerError("Failed to run post-deployment hook", err) + } + + return stack, nil +} + func deploy(dataStore dataservices.DataStore, builder stackBuildProcess, stackID portainer.StackID, endpoint *portainer.Endpoint) { backgroundCtx := context.Background() ctx, cancel := context.WithTimeout(backgroundCtx, 15*time.Minute) diff --git a/api/stacks/stackbuilders/director_test.go b/api/stacks/stackbuilders/director_test.go index a5e10570ff..0c414f3d98 100644 --- a/api/stacks/stackbuilders/director_test.go +++ b/api/stacks/stackbuilders/director_test.go @@ -59,6 +59,10 @@ func (s *stubBuilder) postDeploy(_ context.Context, _ *portainer.Stack) error { return nil } +func (s *stubBuilder) cleanUp() error { + return nil +} + // Helpers func waitForStackStatus(t *testing.T, store *datastore.Store, id portainer.StackID, wantStatus portainer.StackStatus) *portainer.Stack { @@ -78,33 +82,33 @@ func waitForStackStatus(t *testing.T, store *datastore.Store, id portainer.Stack // Tests -func TestBuild_SaveError_ErrUnauthorized_ReturnsInternalServerError(t *testing.T) { +func TestBuildAndAsyncDeploy_SaveError_ErrUnauthorized_ReturnsInternalServerError(t *testing.T) { t.Parallel() builder := &stubBuilder{saveErr: httperrors.ErrUnauthorized} - _, herr := Build(t.Context(), nil, builder, &StackPayload{}, &portainer.Endpoint{}, 0) + _, herr := BuildAndAsyncDeploy(t.Context(), nil, builder, &StackPayload{}, &portainer.Endpoint{}, 0) require.NotNil(t, herr) assert.Equal(t, http.StatusInternalServerError, herr.StatusCode) } -func TestBuild_SaveError_ReturnsInternalServerError(t *testing.T) { +func TestBuildAndAsyncDeploy_SaveError_ReturnsInternalServerError(t *testing.T) { t.Parallel() builder := &stubBuilder{saveErr: errors.New("db error")} - _, herr := Build(t.Context(), nil, builder, &StackPayload{}, &portainer.Endpoint{}, 0) + _, herr := BuildAndAsyncDeploy(t.Context(), nil, builder, &StackPayload{}, &portainer.Endpoint{}, 0) require.NotNil(t, herr) assert.Equal(t, http.StatusInternalServerError, herr.StatusCode) } -func TestBuild_SpawnAsync_DeploySuccess_UpdatesStackStatusToActive(t *testing.T) { +func TestBuildAndAsyncDeploy_SpawnAsync_DeploySuccess_UpdatesStackStatusToActive(t *testing.T) { t.Parallel() _, store := datastore.MustNewTestStore(t, true, false) stack := &portainer.Stack{ID: 1} builder := &stubBuilder{store: store, savedStack: stack} - _, herr := Build(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) + _, herr := BuildAndAsyncDeploy(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) require.Nil(t, herr) updated := waitForStackStatus(t, store, stack.ID, portainer.StackStatusActive) @@ -115,14 +119,14 @@ func TestBuild_SpawnAsync_DeploySuccess_UpdatesStackStatusToActive(t *testing.T) assert.Equal(t, portainer.StackStatusActive, updated.DeploymentStatus[1].Status) } -func TestBuild_SpawnAsync_DeployFailure_UpdatesStackStatusToError(t *testing.T) { +func TestBuildAndAsyncDeploy_SpawnAsync_DeployFailure_UpdatesStackStatusToError(t *testing.T) { t.Parallel() deployErr := errors.New("failed to pull image nginx:999") _, store := datastore.MustNewTestStore(t, true, false) stack := &portainer.Stack{ID: 1} builder := &stubBuilder{store: store, savedStack: stack, deployErr: deployErr} - _, herr := Build(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) + _, herr := BuildAndAsyncDeploy(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) require.Nil(t, herr) updated := waitForStackStatus(t, store, stack.ID, portainer.StackStatusError) @@ -135,13 +139,13 @@ func TestBuild_SpawnAsync_DeployFailure_UpdatesStackStatusToError(t *testing.T) assert.Equal(t, deployErr.Error(), lastEntry.Message) } -func TestBuild_SpawnAsync_PostDeployHook_CalledOnSuccess(t *testing.T) { +func TestBuildAndAsyncDeploy_SpawnAsync_PostDeployHook_CalledOnSuccess(t *testing.T) { t.Parallel() _, store := datastore.MustNewTestStore(t, true, false) stack := &portainer.Stack{ID: 1} builder := &stubBuilder{store: store, savedStack: stack} - _, herr := Build(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) + _, herr := BuildAndAsyncDeploy(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) require.Nil(t, herr) waitForStackStatus(t, store, stack.ID, portainer.StackStatusActive) @@ -149,16 +153,73 @@ func TestBuild_SpawnAsync_PostDeployHook_CalledOnSuccess(t *testing.T) { require.Eventually(t, builder.hookCalled.Load, 5*time.Second, 10*time.Millisecond, "post-deploy hook should be called after a successful deployment") } -func TestBuild_SpawnAsync_PostDeployHook_NotCalledOnDeployFailure(t *testing.T) { +func TestBuildAndAsyncDeploy_SpawnAsync_PostDeployHook_NotCalledOnDeployFailure(t *testing.T) { t.Parallel() _, store := datastore.MustNewTestStore(t, true, false) stack := &portainer.Stack{ID: 1} builder := &stubBuilder{store: store, savedStack: stack, deployErr: errors.New("failed to deploy")} - _, herr := Build(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) + _, herr := BuildAndAsyncDeploy(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) require.Nil(t, herr) waitForStackStatus(t, store, stack.ID, portainer.StackStatusError) require.False(t, builder.hookCalled.Load(), "post-deploy hook should not be called after a failed deployment") } + +func TestBuildAndDeploy_DeployFailure_ReturnsErrorWithoutPersistingStack(t *testing.T) { + t.Parallel() + _, store := datastore.MustNewTestStore(t, true, false) + stack := &portainer.Stack{ID: 1} + deployErr := errors.New("failed to apply resources") + builder := &stubBuilder{store: store, savedStack: stack, deployErr: deployErr} + + _, herr := BuildAndDeploy(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) + + require.NotNil(t, herr) + assert.Equal(t, http.StatusInternalServerError, herr.StatusCode) + + _, err := store.Stack().Read(stack.ID) + require.Error(t, err, "a stack that failed to deploy should never be persisted") +} + +func TestBuildAndDeploy_DeploySuccess_PersistsActiveStatusSynchronously(t *testing.T) { + t.Parallel() + _, store := datastore.MustNewTestStore(t, true, false) + stack := &portainer.Stack{ID: 1} + builder := &stubBuilder{store: store, savedStack: stack} + + _, herr := BuildAndDeploy(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) + require.Nil(t, herr) + + updated, err := store.Stack().Read(stack.ID) + require.NoError(t, err) + assert.Equal(t, portainer.StackStatusActive, updated.Status) + require.Len(t, updated.DeploymentStatus, 2) + assert.Equal(t, portainer.StackStatusDeploying, updated.DeploymentStatus[0].Status) + assert.Equal(t, portainer.StackStatusActive, updated.DeploymentStatus[1].Status) +} + +func TestBuildAndDeploy_PostDeployHook_CalledOnSuccess(t *testing.T) { + t.Parallel() + _, store := datastore.MustNewTestStore(t, true, false) + stack := &portainer.Stack{ID: 1} + builder := &stubBuilder{store: store, savedStack: stack} + + _, herr := BuildAndDeploy(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) + require.Nil(t, herr) + + assert.True(t, builder.hookCalled.Load(), "post-deploy hook should be called after a successful deployment") +} + +func TestBuildAndDeploy_PostDeployHook_NotCalledOnDeployFailure(t *testing.T) { + t.Parallel() + _, store := datastore.MustNewTestStore(t, true, false) + stack := &portainer.Stack{ID: 1} + builder := &stubBuilder{store: store, savedStack: stack, deployErr: errors.New("failed to deploy")} + + _, herr := BuildAndDeploy(t.Context(), store, builder, &StackPayload{}, &portainer.Endpoint{}, 0) + require.NotNil(t, herr) + + assert.False(t, builder.hookCalled.Load(), "post-deploy hook should not be called after a failed deployment") +} diff --git a/api/stacks/stackbuilders/stack_builder.go b/api/stacks/stackbuilders/stack_builder.go index 0181c4074e..c2d5cf3875 100644 --- a/api/stacks/stackbuilders/stack_builder.go +++ b/api/stacks/stackbuilders/stack_builder.go @@ -52,8 +52,6 @@ func (b *StackBuilder) deploy(ctx context.Context, _ *portainer.Endpoint) error func (b *StackBuilder) postDeploy(_ context.Context, _ *portainer.Stack) error { return nil } func (b *StackBuilder) saveStack() (*portainer.Stack, error) { - defer func() { _ = b.cleanUp() }() - if err := b.dataStore.UpdateTx(func(tx dataservices.DataStoreTx) error { if err := tx.Stack().Create(b.stack); err != nil { return fmt.Errorf("Unable to persist the stack inside the database: %w", err) diff --git a/api/stacks/stackutils/stack_status.go b/api/stacks/stackutils/stack_status.go index 2ce34a75ee..d67d2e6701 100644 --- a/api/stacks/stackutils/stack_status.go +++ b/api/stacks/stackutils/stack_status.go @@ -6,6 +6,9 @@ import ( portainer "github.com/portainer/portainer/api" ) +// InlineDeployTimeout bounds a synchronous, inline stack deploy. +const InlineDeployTimeout = time.Minute + // PrepareStackStatusForDeployment transitions a stack into the deploying state before a deployment begins. // It saves the current status in DeploymentStartStatus so that pre-deployment actions can be determined // (e.g. whether to undeploy before redeploying), sets Status to StackStatusDeploying, and resets the diff --git a/app/react/components/Charts/StatsLineChart.tsx b/app/react/components/Charts/StatsLineChart.tsx index 48b14812c1..b56ddcf7f9 100644 --- a/app/react/components/Charts/StatsLineChart.tsx +++ b/app/react/components/Charts/StatsLineChart.tsx @@ -51,7 +51,7 @@ export function StatsLineChart({ width={75} /> + formatter={(value: unknown) => yAxisFormatter(typeof value === 'number' ? value : 0) } isAnimationActive={false} diff --git a/app/react/portainer/generated-api/portainer/types.gen.ts b/app/react/portainer/generated-api/portainer/types.gen.ts index f1ce2b8aaf..19da2dad68 100644 --- a/app/react/portainer/generated-api/portainer/types.gen.ts +++ b/app/react/portainer/generated-api/portainer/types.gen.ts @@ -1235,6 +1235,7 @@ export type KubernetesK8sIngressPath = { Path?: string; PathType?: string; Port?: number; + PortName?: string; ServiceName?: string; }; diff --git a/app/react/portainer/generated-api/portainer/zod.gen.ts b/app/react/portainer/generated-api/portainer/zod.gen.ts index 1ecb41a8fd..60d2833542 100644 --- a/app/react/portainer/generated-api/portainer/zod.gen.ts +++ b/app/react/portainer/generated-api/portainer/zod.gen.ts @@ -440,6 +440,7 @@ export const zKubernetesK8sIngressPath = z.object({ Path: z.string().optional(), PathType: z.string().optional(), Port: z.int().optional(), + PortName: z.string().optional(), ServiceName: z.string().optional(), }); diff --git a/app/react/portainer/gitops/sources/CreateView/steps/ConfigureGit.tsx b/app/react/portainer/gitops/sources/CreateView/steps/ConfigureGit.tsx index 1eb42c0913..8509840ed5 100644 --- a/app/react/portainer/gitops/sources/CreateView/steps/ConfigureGit.tsx +++ b/app/react/portainer/gitops/sources/CreateView/steps/ConfigureGit.tsx @@ -59,7 +59,7 @@ export function ConfigureGit() { /> {values.git.polling.enabled && ( -
+
setFieldValue('git.polling.interval', value)} diff --git a/app/react/portainer/gitops/sources/ItemView/SettingsTab/EditForm/EditPollingWidget.tsx b/app/react/portainer/gitops/sources/ItemView/SettingsTab/EditForm/EditPollingWidget.tsx index ea608eecc6..cda7617546 100644 --- a/app/react/portainer/gitops/sources/ItemView/SettingsTab/EditForm/EditPollingWidget.tsx +++ b/app/react/portainer/gitops/sources/ItemView/SettingsTab/EditForm/EditPollingWidget.tsx @@ -28,7 +28,7 @@ export function EditPollingWidget() { data-cy="source-polling-switch" /> {values.pollingEnabled && ( -
+
setFieldValue('interval', value)}