Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support deleting data volumes while deleting services #613

Merged
merged 32 commits into from Dec 6, 2018
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
79edc37
support deleting data volumes while deleting services, closes #422
ilgooz Nov 29, 2018
8b3ed89
merge dev
ilgooz Dec 2, 2018
35909fe
service: cleanup
ilgooz Dec 2, 2018
a23d9f6
commands>dev: do not delete service's data by default
ilgooz Dec 2, 2018
9f2f6ec
fix typo
ilgooz Dec 2, 2018
e08107b
interface/grpc/*: fix tests
ilgooz Dec 2, 2018
bc244af
interface/grpc/*: fix tests
ilgooz Dec 2, 2018
cb1a0ad
delete logic about creating volumes
ilgooz Dec 3, 2018
81f79a4
commands>delete: fix delete data flag
NicolasMahe Dec 3, 2018
4c48c78
commands>delete: change the default of delete data dialog to false
NicolasMahe Dec 3, 2018
3f7f55e
protobuf>coreapi: improve docs
NicolasMahe Dec 3, 2018
e54c021
generate pb files & docs
ilgooz Dec 3, 2018
5a52246
commands>delete: have yes, all, keep-data flags. add some tests
ilgooz Dec 4, 2018
23fa50c
merge dev
ilgooz Dec 4, 2018
6480e18
commands>delete: confirm single service deletion, add more tests
ilgooz Dec 4, 2018
3c2dab7
Merge branch 'dev' into feature/auto-remove-volume
NicolasMahe Dec 4, 2018
f75edb7
Lint
krhubert Dec 4, 2018
1799814
Add one test case
krhubert Dec 4, 2018
3998e17
Add delete volume integration test
NicolasMahe Dec 5, 2018
2718c05
Separate dep.extractVolumes and dep.extractVolumesFrom. Update codeba…
NicolasMahe Dec 5, 2018
ee556c5
Simplify logic on data deletion confirmation question
NicolasMahe Dec 5, 2018
659edbc
Fix TestServiceDeletePreRunE
NicolasMahe Dec 5, 2018
d4494c7
Add container.option to newIntegrationContainer to set a longer timeo…
NicolasMahe Dec 5, 2018
af9dfe3
Increase TestIntegrationDeleteVolumes timeout to 1 minute
NicolasMahe Dec 5, 2018
25c5f61
Fix lint on dep.extractVolumes
NicolasMahe Dec 5, 2018
3ade1f6
Comment TestIntegrationDeleteVolumes because it doesn't work on CircleCI
NicolasMahe Dec 5, 2018
ec382b5
Remove option in newIntegrationContainer
NicolasMahe Dec 5, 2018
2374af9
Merge branch 'dev' into feature/auto-remove-volume
NicolasMahe Dec 5, 2018
ac80351
Fix lint
NicolasMahe Dec 5, 2018
f0b15d9
Merge branch 'feature/auto-remove-volume' of https://github.com/mesg-…
NicolasMahe Dec 5, 2018
669d864
Remove survey mocks
NicolasMahe Dec 5, 2018
a0b2da9
Uncomment tests + remove discardOutput call
krhubert Dec 5, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 15 additions & 3 deletions api/delete.go
Expand Up @@ -5,8 +5,10 @@ import (
)

// DeleteService stops and deletes service serviceID.
func (a *API) DeleteService(serviceID string) error {
return newServiceDeletor(a).Delete(serviceID)
// when deleteData is enabled, any persistent data that belongs to
// service and its dependencies also will be deleted.
func (a *API) DeleteService(serviceID string, deleteData bool) error {
return newServiceDeletor(a).Delete(serviceID, deleteData)
}

// serviceDeletor provides functionalities to delete a MESG service.
Expand All @@ -22,7 +24,9 @@ func newServiceDeletor(api *API) *serviceDeletor {
}

// Delete stops and deletes service serviceID.
func (d *serviceDeletor) Delete(serviceID string) error {
// when deleteData is enabled, any persistent data that belongs to
// service and its dependencies also will be deleted.
func (d *serviceDeletor) Delete(serviceID string, deleteData bool) error {
s, err := d.api.db.Get(serviceID)
if err != nil {
return err
Expand All @@ -34,5 +38,13 @@ func (d *serviceDeletor) Delete(serviceID string) error {
if err := s.Stop(); err != nil {
return err
}
// delete volumes first before the service. this way if
// deleting volumes fails, process can be retried by the user again
// because service still will be in the db.
if deleteData {
NicolasMahe marked this conversation as resolved.
Show resolved Hide resolved
if err := s.DeleteVolumes(); err != nil {
return err
}
}
return d.api.db.Delete(serviceID)
}
1 change: 1 addition & 0 deletions api/deploy.go
Expand Up @@ -139,6 +139,7 @@ func (d *serviceDeployer) deploy(r io.Reader) (*service.Service, *importer.Valid
if validationErr != nil {
return nil, validationErr, nil
}

return s, nil, d.api.db.Save(s)
}

Expand Down
4 changes: 2 additions & 2 deletions commands/commands.go
Expand Up @@ -21,8 +21,8 @@ type RootExecutor interface {
// ServiceExecutor is an interface that handles services commands.
type ServiceExecutor interface {
ServiceByID(id string) (*coreapi.Service, error)
ServiceDeleteAll() error
ServiceDelete(ids ...string) error
ServiceDeleteAll(deleteData bool) error
ServiceDelete(deleteData bool, ids ...string) error
ServiceDeploy(path string, statuses chan provider.DeployStatus) (id string, validationError, err error)
ServiceListenEvents(id, eventFilter string) (chan *coreapi.EventData, chan error, error)
ServiceListenResults(id, taskFilter, outputFilter string, tagFilters []string) (chan *coreapi.ResultData, chan error, error)
Expand Down
19 changes: 10 additions & 9 deletions commands/mocks/Executor.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 10 additions & 5 deletions commands/provider/service_provider.go
Expand Up @@ -38,7 +38,7 @@ func (p *ServiceProvider) ServiceByID(id string) (*coreapi.Service, error) {
}

// ServiceDeleteAll deletes all services.
func (p *ServiceProvider) ServiceDeleteAll() error {
func (p *ServiceProvider) ServiceDeleteAll(deleteData bool) error {
rep, err := p.client.ListServices(context.Background(), &coreapi.ListServicesRequest{})
if err != nil {
return err
Expand All @@ -51,7 +51,10 @@ func (p *ServiceProvider) ServiceDeleteAll() error {
wg.Add(len(rep.Services))
for _, s := range rep.Services {
go func(id string) {
_, err := p.client.DeleteService(context.Background(), &coreapi.DeleteServiceRequest{ServiceID: id})
_, err := p.client.DeleteService(context.Background(), &coreapi.DeleteServiceRequest{
ServiceID: id,
DeleteData: deleteData,
})
if err != nil {
errs.Append(err)
}
Expand All @@ -63,11 +66,13 @@ func (p *ServiceProvider) ServiceDeleteAll() error {
}

// ServiceDelete deletes service with given ids.
func (p *ServiceProvider) ServiceDelete(ids ...string) error {
func (p *ServiceProvider) ServiceDelete(deleteData bool, ids ...string) error {
var errs xerrors.Errors
for _, id := range ids {
_, err := p.client.DeleteService(context.Background(), &coreapi.DeleteServiceRequest{ServiceID: id})
if err != nil {
if _, err := p.client.DeleteService(context.Background(), &coreapi.DeleteServiceRequest{
ServiceID: id,
DeleteData: deleteData,
}); err != nil {
errs = append(errs, err)
}
}
Expand Down
25 changes: 20 additions & 5 deletions commands/service_delete.go
Expand Up @@ -13,8 +13,9 @@ import (
type serviceDeleteCmd struct {
baseCmd

all bool
force bool
all bool
force bool
deleteData bool

e ServiceExecutor
}
Expand All @@ -32,6 +33,7 @@ mesg-core service delete --all`,

c.cmd.Flags().BoolVar(&c.all, "all", c.all, "Delete all services")
c.cmd.Flags().BoolVarP(&c.force, "force", "f", c.force, "Force delete all services")
c.cmd.Flags().BoolVarP(&c.deleteData, "delete-data", "d", c.force, "Delete services' persistent data along the way")
ilgooz marked this conversation as resolved.
Show resolved Hide resolved
return c
}

Expand All @@ -41,7 +43,7 @@ func (c *serviceDeleteCmd) preRunE(cmd *cobra.Command, args []string) error {
}

if !c.all || (c.all && c.force) {
return nil
return c.askDeleteData()
ilgooz marked this conversation as resolved.
Show resolved Hide resolved
}

if err := survey.AskOne(&survey.Confirm{
Expand All @@ -54,14 +56,27 @@ func (c *serviceDeleteCmd) preRunE(cmd *cobra.Command, args []string) error {
if !c.force {
return errors.New("can't continue without confirmation")
}

return c.askDeleteData()
}

func (c *serviceDeleteCmd) askDeleteData() error {
if !c.deleteData {
if err := survey.AskOne(&survey.Confirm{
Message: "Do you want to remove service(s)' persistent data as well?",
Default: true,
ilgooz marked this conversation as resolved.
Show resolved Hide resolved
}, &c.deleteData, nil); err != nil {
return err
}
}
return nil
}

func (c *serviceDeleteCmd) runE(cmd *cobra.Command, args []string) error {
var err error
if c.all {
pretty.Progress("Deleting all services...", func() {
err = c.e.ServiceDeleteAll()
err = c.e.ServiceDeleteAll(c.deleteData)
})
if err != nil {
return err
Expand All @@ -75,7 +90,7 @@ func (c *serviceDeleteCmd) runE(cmd *cobra.Command, args []string) error {
// build function to avoid using arg inside progress
fn := func(id string) func() {
return func() {
err = c.e.ServiceDelete(id)
err = c.e.ServiceDelete(c.deleteData, id)
}
}(arg)
pretty.Progress(fmt.Sprintf("Deleting service %q...", arg), fn)
Expand Down
2 changes: 1 addition & 1 deletion commands/service_dev.go
Expand Up @@ -79,7 +79,7 @@ func (c *serviceDevCmd) runE(cmd *cobra.Command, args []string) error {
defer func() {
var err error
pretty.Progress("Removing the service...", func() {
err = c.e.ServiceDelete(id)
err = c.e.ServiceDelete(false, id)
})
if err != nil {
fmt.Printf("%s Removing the service completed with an error: %s\n", pretty.FailSign, err)
Expand Down
1 change: 1 addition & 0 deletions container/container.go
Expand Up @@ -29,6 +29,7 @@ type Container interface {
Status(namespace []string) (StatusType, error)
StopService(namespace []string) (err error)
TasksError(namespace []string) ([]string, error)
DeleteVolume(name string) error
}

// DockerContainer provides high level interactions with Docker API for MESG.
Expand Down
14 changes: 14 additions & 0 deletions container/mocks/Container.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions container/volume.go
@@ -0,0 +1,12 @@
package container

import (
"context"
)

// DeleteVolume deletes a Docker Volume by name.
func (c *DockerContainer) DeleteVolume(name string) error {
ctx, cancel := context.WithTimeout(context.Background(), c.callTimeout)
defer cancel()
return c.client.VolumeRemove(ctx, name, false)
}
20 changes: 20 additions & 0 deletions container/volume_test.go
@@ -0,0 +1,20 @@
package container

import (
"testing"

"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

func TestDeleteVolume(t *testing.T) {
var (
c, m = newTesting(t)
name = "1"
)

m.On("VolumeRemove", mock.Anything, name, false).Once().Return(nil)
require.NoError(t, c.DeleteVolume(name))

m.AssertExpectations(t)
}
1 change: 1 addition & 0 deletions docs/api/core.md
Expand Up @@ -1162,6 +1162,7 @@ Request's data of the `DeleteService` API.
| Field | Type | Description |
| ----- | ---- | ----------- |
| serviceID | [string](#string) | The Service ID. Generated when using the [`DeployService` API](#deployservice). |
| deleteData | [bool](#bool) | When enabled, any persistent data that belongs to service and its dependencies will be also deleted. |



Expand Down
2 changes: 1 addition & 1 deletion interface/grpc/core/delete.go
Expand Up @@ -8,5 +8,5 @@ import (

// DeleteService stops and deletes service serviceID.
func (s *Server) DeleteService(ctx context.Context, request *coreapi.DeleteServiceRequest) (*coreapi.DeleteServiceReply, error) {
return &coreapi.DeleteServiceReply{}, s.api.DeleteService(request.ServiceID)
return &coreapi.DeleteServiceReply{}, s.api.DeleteService(request.ServiceID, request.DeleteData)
}
2 changes: 1 addition & 1 deletion interface/grpc/core/deploy_integration_test.go
Expand Up @@ -17,7 +17,7 @@ func TestIntegrationDeployService(t *testing.T) {
stream := newTestDeployStream(url)

require.Nil(t, server.DeployService(stream))
defer server.api.DeleteService(stream.serviceID)
defer server.api.DeleteService(stream.serviceID, false)

require.Len(t, stream.serviceID, 40)
require.Contains(t, stream.statuses, api.DeployStatus{
Expand Down
10 changes: 5 additions & 5 deletions interface/grpc/core/execute_test.go
Expand Up @@ -21,7 +21,7 @@ func TestExecute(t *testing.T) {
s, validationErr, err := server.api.DeployService(serviceTar(t, taskServicePath))
require.Zero(t, validationErr)
require.NoError(t, err)
defer server.api.DeleteService(s.ID)
defer server.api.DeleteService(s.ID, false)

require.NoError(t, server.api.StartService(s.ID))
defer server.api.StopService(s.ID)
Expand All @@ -42,7 +42,7 @@ func TestExecuteWithInvalidJSON(t *testing.T) {
s, validationErr, err := server.api.DeployService(serviceTar(t, taskServicePath))
require.Zero(t, validationErr)
require.NoError(t, err)
defer server.api.DeleteService(s.ID)
defer server.api.DeleteService(s.ID, false)

_, err = server.ExecuteTask(context.Background(), &coreapi.ExecuteTaskRequest{
ServiceID: s.ID,
Expand All @@ -63,7 +63,7 @@ func TestExecuteWithInvalidTask(t *testing.T) {
s, validationErr, err := server.api.DeployService(serviceTar(t, taskServicePath))
require.Zero(t, validationErr)
require.NoError(t, err)
defer server.api.DeleteService(s.ID)
defer server.api.DeleteService(s.ID, false)

require.NoError(t, server.api.StartService(s.ID))
defer server.api.StopService(s.ID)
Expand Down Expand Up @@ -91,7 +91,7 @@ func TestExecuteWithInvalidTaskInput(t *testing.T) {
s, validationErr, err := server.api.DeployService(serviceTar(t, taskServicePath))
require.Zero(t, validationErr)
require.NoError(t, err)
defer server.api.DeleteService(s.ID)
defer server.api.DeleteService(s.ID, false)

require.NoError(t, server.api.StartService(s.ID))
defer server.api.StopService(s.ID)
Expand All @@ -115,7 +115,7 @@ func TestExecuteWithNonRunningService(t *testing.T) {
s, validationErr, err := server.api.DeployService(serviceTar(t, taskServicePath))
require.Zero(t, validationErr)
require.NoError(t, err)
defer server.api.DeleteService(s.ID)
defer server.api.DeleteService(s.ID, false)

_, err = server.ExecuteTask(context.Background(), &coreapi.ExecuteTaskRequest{
ServiceID: s.ID,
Expand Down
2 changes: 1 addition & 1 deletion interface/grpc/core/get_service_test.go
Expand Up @@ -15,7 +15,7 @@ func TestGetService(t *testing.T) {
s, validationErr, err := server.api.DeployService(serviceTar(t, taskServicePath))
require.Zero(t, validationErr)
require.NoError(t, err)
defer server.api.DeleteService(s.ID)
defer server.api.DeleteService(s.ID, false)

reply, err := server.GetService(context.Background(), &coreapi.GetServiceRequest{
ServiceID: s.ID,
Expand Down
2 changes: 1 addition & 1 deletion interface/grpc/core/list_services_test.go
Expand Up @@ -15,7 +15,7 @@ func TestListServices(t *testing.T) {

stream := newTestDeployStream(url)
require.NoError(t, server.DeployService(stream))
defer server.api.DeleteService(stream.serviceID)
defer server.api.DeleteService(stream.serviceID, false)

reply, err := server.ListServices(context.Background(), &coreapi.ListServicesRequest{})
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion interface/grpc/core/start_test.go
Expand Up @@ -19,7 +19,7 @@ func TestStartService(t *testing.T) {
s, validationErr, err := server.api.DeployService(serviceTar(t, eventServicePath))
require.Zero(t, validationErr)
require.NoError(t, err)
defer server.api.DeleteService(s.ID)
defer server.api.DeleteService(s.ID, false)

_, err = server.StartService(context.Background(), &coreapi.StartServiceRequest{
ServiceID: s.ID,
Expand Down
2 changes: 1 addition & 1 deletion interface/grpc/core/stop_test.go
Expand Up @@ -19,7 +19,7 @@ func TestStopService(t *testing.T) {
s, validationErr, err := server.api.DeployService(serviceTar(t, eventServicePath))
require.Zero(t, validationErr)
require.NoError(t, err)
defer server.api.DeleteService(s.ID)
defer server.api.DeleteService(s.ID, false)

require.NoError(t, server.api.StartService(s.ID))

Expand Down