add remove command

cmd/remove.go

@@ -0,0 +1,58 @@
+/*
+ Copyright 2020 Qiniu Cloud (qiniu.com)
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package cmd
+
+import (
+	"fmt"
+	"os"
+
+	log "github.com/sirupsen/logrus"
+
+	"github.com/qiniu/goc/pkg/cover"
+	"github.com/spf13/cobra"
+)
+
+var removeCmd = &cobra.Command{
+	Use:   "remove",
+	Short: "Remove the specified service from the register center.",
+	Long:  `Remove the specified service from the register center, after that, goc profile will not collect coverage data from this service anymore`,
+	Example: `
+# Remove the service 'mongo' from the default register center http://127.0.0.1:7777.
+goc remove --service=mongo
+
+# Remove the service 'http://127.0.0.1:53' from the specified register center.
+goc remove --address="http://127.0.0.1:53" --center=http://192.168.1.1:8080
+`,
+	Run: func(cmd *cobra.Command, args []string) {
+		p := cover.ProfileParam{
+			Service: svrList,
+			Address: addrList,
+		}
+		res, err := cover.NewWorker(center).Remove(p)
+		if err != nil {
+			log.Fatalf("call host %v failed, err: %v, response: %v", center, err, string(res))
+		}
+		fmt.Fprint(os.Stdout, string(res))
+	},
+}
+
+func init() {
+	addBasicFlags(removeCmd.Flags())
+	removeCmd.Flags().StringSliceVarP(&svrList, "service", "", nil, "service name to clear profile, see 'goc list' for all services.")
+	removeCmd.Flags().StringSliceVarP(&addrList, "address", "", nil, "address to clear profile, see 'goc list' for all addresses.")
+	rootCmd.AddCommand(removeCmd)
+}

pkg/cover/client.go

@@ -34,6 +34,7 @@ import (
 type Action interface {
 	Profile(param ProfileParam) ([]byte, error)
 	Clear(param ProfileParam) ([]byte, error)
+	Remove(param ProfileParam) ([]byte, error)
 	InitSystem() ([]byte, error)
 	ListServices() ([]byte, error)
 	RegisterService(svr ServiceUnderTest) ([]byte, error)
@@ -50,6 +51,8 @@ const (
 	CoverServicesListAPI = "/v1/cover/list"
 	//CoverRegisterServiceAPI register a service into service center
 	CoverRegisterServiceAPI = "/v1/cover/register"
+	//CoverServicesRemoveAPI remove one services from the service center
+	CoverServicesRemoveAPI = "/v1/cover/remove"
 )
 
 type client struct {
@@ -128,6 +131,22 @@ func (c *client) Clear(param ProfileParam) ([]byte, error) {
 	return resp, err
 }
 
+func (c *client) Remove(param ProfileParam) ([]byte, error) {
+	u := fmt.Sprintf("%s%s", c.Host, CoverServicesRemoveAPI)
+	if len(param.Service) != 0 && len(param.Address) != 0 {
+		return nil, fmt.Errorf("use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately")
+	}
+
+	// the json.Marshal function can return two types of errors: UnsupportedTypeError or UnsupportedValueError
+	// so no need to check here
+	body, _ := json.Marshal(param)
+	_, resp, err := c.do("POST", u, "application/json", bytes.NewReader(body))
+	if err != nil && isNetworkError(err) {
+		_, resp, err = c.do("POST", u, "application/json", bytes.NewReader(body))
+	}
+	return resp, err
+}
+
 func (c *client) InitSystem() ([]byte, error) {
 	u := fmt.Sprintf("%s%s", c.Host, CoverInitSystemAPI)
 	_, body, err := c.do("POST", u, "", nil)

pkg/cover/client_test.go

@@ -17,6 +17,7 @@
 package cover
 
 import (
+	"fmt"
 	"net/http/httptest"
 	"os"
 	"testing"
@@ -209,3 +210,36 @@ func TestClientClearWithInvalidParam(t *testing.T) {
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately")
 }
+
+func TestClientRemove(t *testing.T) {
+	// remove by invalid param
+	p := ProfileParam{
+		Service: []string{"goc"},
+		Address: []string{"http://127.0.0.1:777"},
+	}
+	c := &client{
+		client: http.DefaultClient,
+	}
+	_, err := c.Remove(p)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately")
+
+	// remove by valid param
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintln(w, "Hello, client")
+	}))
+	defer ts.Close()
+
+	c.Host = ts.URL
+	p = ProfileParam{
+		Address: []string{"http://127.0.0.1:777"},
+	}
+	res, err := c.Remove(p)
+	assert.NoError(t, err)
+	assert.Equal(t, string(res), "Hello, client\n")
+
+	// remove from a invalid center
+	c.Host = "http://127.0.0.1:11111"
+	_, err = c.Remove(p)
+	assert.Error(t, err)
+}

pkg/cover/server.go

@@ -90,6 +90,7 @@ func (s *server) Route(w io.Writer) *gin.Engine {
 		v1.POST("/cover/clear", s.clear)
 		v1.POST("/cover/init", s.initSystem)
 		v1.GET("/cover/list", s.listServices)
+		v1.POST("/cover/remove", s.removeServices)
 	}
 
 	return r
@@ -265,6 +266,28 @@ func (s *server) initSystem(c *gin.Context) {
 	c.JSON(http.StatusOK, "")
 }
 
+func (s *server) removeServices(c *gin.Context) {
+	var body ProfileParam
+	if err := c.ShouldBind(&body); err != nil {
+		c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
+		return
+	}
+	svrsUnderTest := s.Store.GetAll()
+	filterAddrList, err := filterAddrs(body.Service, body.Address, true, svrsUnderTest)
+	if err != nil {
+		c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
+		return
+	}
+	for _, addr := range filterAddrList {
+		err := s.Store.Remove(addr)
+		if err != nil {
+			c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
+			return
+		}
+		fmt.Fprintf(c.Writer, "Register service %s removed from the center.", addr)
+	}
+}
+
 func convertProfile(p []byte) ([]*cover.Profile, error) {
 	// Annoyingly, ParseProfiles only accepts a filename, so we have to write the bytes to disk
 	// so it can read them back.

pkg/cover/server_test.go

@@ -27,6 +27,11 @@ func (m *MockStore) Add(s ServiceUnderTest) error {
 	return args.Error(0)
 }
 
+func (m *MockStore) Remove(a string) error {
+	args := m.Called(a)
+	return args.Error(0)
+}
+
 func (m *MockStore) Get(name string) []string {
 	args := m.Called(name)
 	return args.Get(0).([]string)
@@ -225,6 +230,69 @@ func TestClearService(t *testing.T) {
 	assert.Contains(t, w.Body.String(), "use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately")
 }
 
+func TestRemoveServices(t *testing.T) {
+	testObj := new(MockStore)
+	testObj.On("GetAll").Return(map[string][]string{"foo": {"test1", "test2"}})
+	testObj.On("Remove", "test1").Return(nil)
+
+	server := &server{
+		Store: testObj,
+	}
+	router := server.Route(os.Stdout)
+
+	// remove with invalid request
+	w := httptest.NewRecorder()
+	req, _ := http.NewRequest("POST", "/v1/cover/remove", nil)
+	req.Header.Set("Content-Type", "application/json")
+	router.ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusExpectationFailed, w.Code)
+	assert.Contains(t, w.Body.String(), "invalid request")
+
+	// remove service
+	p := ProfileParam{
+		Address: []string{"test1"},
+	}
+	encoded, err := json.Marshal(p)
+	assert.NoError(t, err)
+	w = httptest.NewRecorder()
+	req, _ = http.NewRequest("POST", "/v1/cover/remove", bytes.NewBuffer(encoded))
+	req.Header.Set("Content-Type", "application/json")
+	router.ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.Contains(t, w.Body.String(), "Register service test1 removed from the center.")
+
+	// remove service with non-exist address
+	testObj.On("Remove", "test2").Return(fmt.Errorf("no service found"))
+	p = ProfileParam{
+		Address: []string{"test2"},
+	}
+	encoded, err = json.Marshal(p)
+	assert.NoError(t, err)
+	w = httptest.NewRecorder()
+	req, _ = http.NewRequest("POST", "/v1/cover/remove", bytes.NewBuffer(encoded))
+	req.Header.Set("Content-Type", "application/json")
+	router.ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusExpectationFailed, w.Code)
+	assert.Contains(t, w.Body.String(), "no service found")
+
+	// clear profile with service and address set at at the same time
+	p = ProfileParam{
+		Service: []string{"goc"},
+		Address: []string{"http://127.0.0.1:3333"},
+	}
+	encoded, err = json.Marshal(p)
+	assert.NoError(t, err)
+	w = httptest.NewRecorder()
+	req, _ = http.NewRequest("POST", "/v1/cover/remove", bytes.NewBuffer(encoded))
+	req.Header.Set("Content-Type", "application/json")
+	router.ServeHTTP(w, req)
+	assert.Equal(t, http.StatusExpectationFailed, w.Code)
+	assert.Contains(t, w.Body.String(), "use 'service' flag and 'address' flag at the same time may cause ambiguity, please use them separately")
+}
+
 func TestInitService(t *testing.T) {
 	testObj := new(MockStore)
 	testObj.On("Init").Return(fmt.Errorf("lala error"))

pkg/cover/store.go

@@ -20,11 +20,13 @@ import (
 	"bufio"
 	"errors"
 	"fmt"
-	log "github.com/sirupsen/logrus"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
 	"sync"
+
+	log "github.com/sirupsen/logrus"
 )
 
 var ErrServiceAlreadyRegistered = errors.New("service already registered")
@@ -46,8 +48,8 @@ type Store interface {
 	// Set stores the services information into internal state
 	Set(services map[string][]string)
 
-	// Remove the service from the store
-	Remove(s Service) error
+	// Remove the service from the store by address
+	Remove(addr string) error
 }
 
 // fileStore holds the registered services into memory and persistent to a local file
@@ -102,6 +104,43 @@ func (l *fileStore) GetAll() map[string][]string {
 	return l.memoryStore.GetAll()
 }
 
+// Remove the service from the memory store and the file store
+func (l *fileStore) Remove(addr string) error {
+	err := l.memoryStore.Remove(addr)
+	if err != nil {
+		return err
+	}
+
+	content, err := ioutil.ReadFile(l.persistentFile)
+	if err != nil {
+		return fmt.Errorf("failed to open file, path: %s, err: %v", l.persistentFile, err)
+	}
+
+	newServices := ""
+	for _, line := range strings.Split(string(content), "\n") {
+		// addr(ip:port) is unique, so no need to check name
+		if strings.Contains(line, addr) {
+			// if the service match the string in the store, skip
+		} else {
+			newServices += line + "\n"
+		}
+	}
+
+	// write back to file
+	f, err := os.OpenFile(l.persistentFile, os.O_TRUNC|os.O_WRONLY|os.O_CREATE, 0600)
+	if err != nil {
+		return fmt.Errorf("failed to open file, path: %s, err: %v", l.persistentFile, err)
+	}
+	defer f.Close()
+
+	_, err = f.WriteString(newServices)
+	if err != nil {
+		return err
+	}
+
+	return f.Sync()
+}
+
 // Init cleanup all the registered service information
 // and the local persistent file
 func (l *fileStore) Init() error {
@@ -249,18 +288,33 @@ func (l *memoryStore) Set(services map[string][]string) {
 	l.servicesMap = services
 }
 
-func (l *memoryStore) Remove(s Service) error {
+// Remove one service from the memory store
+// if service is not fount, return "no service found" error
+func (l *memoryStore) Remove(removeAddr string) error {
 	l.mu.Lock()
 	defer l.mu.Unlock()
 
-	services := l.servicesMap[s.Name]
-	newServices := make([]string, 0)
-	for _, addr := range services {
-		if addr != s.Address {
-			newServices = append(newServices, addr)
+	flag := false
+	for name, addrs := range l.servicesMap {
+		newAddrs := make([]string, 0)
+		for _, addr := range addrs {
+			if removeAddr != addr {
+				newAddrs = append(newAddrs, addr)
+			} else {
+				flag = true
+			}
+		}
+		// if no services left, remove by name
+		if len(newAddrs) == 0 {
+			delete(l.servicesMap, name)
+		} else {
+			l.servicesMap[name] = newAddrs
 		}
 	}
-	l.servicesMap[s.Name] = newServices
+
+	if !flag {
+		return fmt.Errorf("no service found")
+	}
 
 	return nil
 }

pkg/cover/store_test.go

@@ -17,8 +17,10 @@
 package cover
 
 import (
-	"github.com/stretchr/testify/assert"
+	"fmt"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestLocalStore(t *testing.T) {
@@ -69,3 +71,60 @@ func TestLocalStore(t *testing.T) {
 		t.Error("local store init failed")
 	}
 }
+
+func TestMemoryStoreRemove(t *testing.T) {
+	store := NewMemoryStore()
+	s1 := ServiceUnderTest{Name: "test", Address: "http://127.0.0.1:8900"}
+	s2 := ServiceUnderTest{Name: "test2", Address: "http://127.0.0.1:8901"}
+	s3 := ServiceUnderTest{Name: "test2", Address: "http://127.0.0.1:8902"}
+
+	_ = store.Add(s1)
+	_ = store.Add(s2)
+	_ = store.Add(s3)
+
+	ss1 := store.Get("test")
+	assert.Equal(t, 1, len(ss1))
+	err := store.Remove("http://127.0.0.1:8900")
+	assert.NoError(t, err)
+	ss1 = store.Get("test")
+	assert.Nil(t, ss1)
+
+	ss2 := store.Get("test2")
+	assert.Equal(t, 2, len(ss2))
+	err = store.Remove("http://127.0.0.1:8901")
+	assert.NoError(t, err)
+	ss2 = store.Get("test2")
+	assert.Equal(t, 1, len(ss2))
+
+	err = store.Remove("http")
+	assert.Error(t, err, fmt.Errorf("no service found"))
+}
+
+func TestFileStoreRemove(t *testing.T) {
+	store, _ := NewFileStore("_svrs_address.txt")
+	_ = store.Init()
+	s1 := ServiceUnderTest{Name: "test", Address: "http://127.0.0.1:8900"}
+	s2 := ServiceUnderTest{Name: "test2", Address: "http://127.0.0.1:8901"}
+	s3 := ServiceUnderTest{Name: "test2", Address: "http://127.0.0.1:8902"}
+
+	_ = store.Add(s1)
+	_ = store.Add(s2)
+	_ = store.Add(s3)
+
+	ss1 := store.Get("test")
+	assert.Equal(t, 1, len(ss1))
+	err := store.Remove("http://127.0.0.1:8900")
+	assert.NoError(t, err)
+	ss1 = store.Get("test")
+	assert.Nil(t, ss1)
+
+	ss2 := store.Get("test2")
+	assert.Equal(t, 2, len(ss2))
+	err = store.Remove("http://127.0.0.1:8901")
+	assert.NoError(t, err)
+	ss2 = store.Get("test2")
+	assert.Equal(t, 1, len(ss2))
+
+	err = store.Remove("http")
+	assert.Error(t, err, fmt.Errorf("no service found"))
+}

tests/remove.bats

@@ -0,0 +1,62 @@
+#!/usr/bin/env bats
+# Copyright 2020 Qiniu Cloud (七牛云)
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load util.sh
+
+setup_file() {
+    # run centered server
+    goc server 3>&- &
+    GOC_PID=$!
+    sleep 2
+    goc init
+
+    # run covered goc
+    gocc server --port=:60001 --debug 3>&- &
+    GOCC_PID=$!
+    sleep 1
+
+    WORKDIR=$PWD
+    cd samples/run_for_several_seconds
+    gocc build --center=http://127.0.0.1:60001
+    ./simple-project 3>&- &
+    SAMPLE_PID=$!
+    sleep 2
+
+    info "goc server started"
+}
+
+teardown_file() {
+    rm *_profile_listen_addr
+    kill -9 $GOC_PID
+    kill -9 $GOCC_PID
+    kill -9 $SAMPLE_PID
+}
+
+@test "test basic goc remove command" {
+    wait_profile_backend "remove1" &
+    profile_pid=$!
+
+    run goc list --center=http://127.0.0.1:60001;
+    info remove1 output: $output
+    [ "$status" -eq 0 ]
+    [[ "$output" =~ "simple-project" ]]
+
+    run gocc remove --address="simple-project" --debug --debugcisyncfile ci-sync.bak;
+    info remove1 output: $output
+    [ "$status" -eq 0 ]
+    [ "$output" = "" ]
+
+    wait $profile_pid
+}

tests/run-ci-actions.sh

@@ -45,4 +45,6 @@ bats -t agent.bats
 
 bats -t merge.bats
 
+bats -t remove.bats
+
 bash <(curl -s https://codecov.io/bash) -f 'filtered*' -F e2e-$GOVERSION