From 57a56c2d68fc71b94a4146808c0b36bfcd5a0ca2 Mon Sep 17 00:00:00 2001 From: lyyyuna Date: Sun, 20 Sep 2020 20:29:17 +0800 Subject: [PATCH] add remove command --- cmd/remove.go | 58 +++++++++++++++++++++++++++++++ pkg/cover/client.go | 19 +++++++++++ pkg/cover/client_test.go | 34 ++++++++++++++++++ pkg/cover/server.go | 23 +++++++++++++ pkg/cover/server_test.go | 68 ++++++++++++++++++++++++++++++++++++ pkg/cover/store.go | 74 ++++++++++++++++++++++++++++++++++------ pkg/cover/store_test.go | 61 ++++++++++++++++++++++++++++++++- tests/remove.bats | 62 +++++++++++++++++++++++++++++++++ tests/run-ci-actions.sh | 2 ++ 9 files changed, 390 insertions(+), 11 deletions(-) create mode 100644 cmd/remove.go create mode 100755 tests/remove.bats diff --git a/cmd/remove.go b/cmd/remove.go new file mode 100644 index 0000000..6d21eea --- /dev/null +++ b/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) +} diff --git a/pkg/cover/client.go b/pkg/cover/client.go index 55f2c08..6e0c82f 100644 --- a/pkg/cover/client.go +++ b/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) diff --git a/pkg/cover/client_test.go b/pkg/cover/client_test.go index 077acba..f858872 100644 --- a/pkg/cover/client_test.go +++ b/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) +} diff --git a/pkg/cover/server.go b/pkg/cover/server.go index 0ad07d9..b8420b6 100644 --- a/pkg/cover/server.go +++ b/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. diff --git a/pkg/cover/server_test.go b/pkg/cover/server_test.go index c144911..4e16af1 100644 --- a/pkg/cover/server_test.go +++ b/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")) diff --git a/pkg/cover/store.go b/pkg/cover/store.go index d549c2f..fc06e53 100644 --- a/pkg/cover/store.go +++ b/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 } diff --git a/pkg/cover/store_test.go b/pkg/cover/store_test.go index 6d5d076..bf3201f 100644 --- a/pkg/cover/store_test.go +++ b/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")) +} diff --git a/tests/remove.bats b/tests/remove.bats new file mode 100755 index 0000000..e03f6f4 --- /dev/null +++ b/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 +} diff --git a/tests/run-ci-actions.sh b/tests/run-ci-actions.sh index 14bef93..cc4beab 100755 --- a/tests/run-ci-actions.sh +++ b/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 \ No newline at end of file