From f96a73f219bbcbf3bf0a47f2251dd3a0e0a8de75 Mon Sep 17 00:00:00 2001 From: tongjingran Date: Fri, 10 Jul 2020 15:17:14 +0800 Subject: [PATCH] change server name register to server and process duplicate flag values --- cmd/profile.go | 27 +++++++------ go.mod | 2 +- go.sum | 1 - pkg/cover/client.go | 27 ++++++------- pkg/cover/client_test.go | 52 +++++++++++++++++++++++-- pkg/cover/cover.go | 1 + pkg/cover/instrument.go | 4 +- pkg/cover/server.go | 83 ++++++++++++++++++++++++---------------- pkg/cover/store.go | 10 +++-- pkg/cover/store_test.go | 21 ++++++++++ 10 files changed, 160 insertions(+), 68 deletions(-) diff --git a/cmd/profile.go b/cmd/profile.go index 865c116..37f549d 100644 --- a/cmd/profile.go +++ b/cmd/profile.go @@ -35,20 +35,23 @@ var profileCmd = &cobra.Command{ # Get coverage counter from default register center http://127.0.0.1:7777, the result output to stdout. goc profile -# Get coverage counter from default register center, the result output to specified file. -goc profile -o ./coverage.cov - -# Get coverage counter from specified register center, the result output to specified file. -goc profile --center=http://192.168.1.1:8080 -o ./coverage.cov - # Get coverage counter from specified register center, the result output to specified file. goc profile --center=http://192.168.1.1:8080 --output=./coverage.cov + +# Get coverage counter of several specified services +goc profile --service=service1,service2 --service=service3 + +# Get coverage counter of several specified address +goc profile --address=address1,address2 --address=address3 + +# Force to get coverage counter, ignore any internal error +goc profile --force `, Run: func(cmd *cobra.Command, args []string) { p := cover.ProfileParam{ Force: force, - Name: name, - Address: address, + Service: svrList, + Address: addrList, } res, err := cover.NewWorker(center).Profile(p) if err != nil { @@ -73,12 +76,14 @@ goc profile --center=http://192.168.1.1:8080 --output=./coverage.cov var output string var force bool +var svrList []string +var addrList []string func init() { profileCmd.Flags().StringVarP(&output, "output", "o", "", "download cover profile") - profileCmd.Flags().StringVarP(&name, "name", "n", "", "name list to get cover profile") - profileCmd.Flags().StringVarP(&address, "address", "a", "", "address list to get cover proflie") - profileCmd.Flags().BoolVarP(&force, "force", "f", false, "force") + profileCmd.Flags().StringSliceVarP(&svrList, "service", "", nil, "service to get cover profile") + profileCmd.Flags().StringSliceVarP(&addrList, "address", "", nil, "address to get cover profile") + profileCmd.Flags().BoolVarP(&force, "force", "f", false, "force to get cover profile, ignore any internal error") addBasicFlags(profileCmd.Flags()) rootCmd.AddCommand(profileCmd) } diff --git a/go.mod b/go.mod index 67c6e35..1c38ded 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,6 @@ require ( golang.org/x/net v0.0.0-20200301022130-244492dfa37a golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d golang.org/x/tools v0.0.0-20200329025819-fd4102a86c65 - k8s.io/kubernetes v1.13.0 // indirect + k8s.io/kubernetes v1.13.0 k8s.io/test-infra v0.0.0-20200511080351-8ac9dbfab055 ) diff --git a/go.sum b/go.sum index aa894bf..d6bec85 100644 --- a/go.sum +++ b/go.sum @@ -464,7 +464,6 @@ github.com/klauspost/pgzip v1.2.1/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQ github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/konsorten/go-windows-terminal-sequences v1.0.2 h1:DB17ag19krx9CFsz4o3enTrPXyIXCl+2iCXH/aMAp9s= github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= -github.com/konsorten/go-windows-terminal-sequences v1.0.3 h1:CE8S1cTafDpPvMhIxNJKvHsGVBgn1xWYf1NbHQhywc8= github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= diff --git a/pkg/cover/client.go b/pkg/cover/client.go index 456e668..c526a65 100644 --- a/pkg/cover/client.go +++ b/pkg/cover/client.go @@ -17,8 +17,7 @@ package cover import ( - "bytes" - "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -26,6 +25,7 @@ import ( "net" "net/http" "net/url" + "strconv" "strings" ) @@ -75,8 +75,6 @@ func (c *client) RegisterService(srv Service) ([]byte, error) { if strings.TrimSpace(srv.Name) == "" { return nil, fmt.Errorf("invalid service name") } - srvPath := strings.Split(srv.Name, "/") - srv.Name = srvPath[len(srvPath)-1] u := fmt.Sprintf("%s%s?name=%s&address=%s", c.Host, CoverRegisterServiceAPI, srv.Name, srv.Address) res, err := c.do("POST", u, nil) return res, err @@ -93,17 +91,17 @@ func (c *client) ListServices() ([]byte, error) { } func (c *client) Profile(param ProfileParam) ([]byte, error) { - log.Printf("param:%+v", param) - u := fmt.Sprintf("%s%s", c.Host, CoverProfileAPI) - args, err := json.Marshal(param) - if err != nil { - return nil, err + u := fmt.Sprintf("%s%s?force=%s", c.Host, CoverProfileAPI, strconv.FormatBool(param.Force)) + for _, svr := range param.Service { + u = u + "&service=" + svr } - profile, err := c.do("POST", u, bytes.NewReader(args)) + for _, addr := range param.Address { + u = u + "&address=" + addr + } + profile, err := c.do("GET", u, nil) if err != nil && isNetworkError(err) { - profile, err = c.do("POST", u, bytes.NewReader(args)) + profile, err = c.do("GET", u, nil) } - return profile, err } @@ -135,7 +133,10 @@ func (c *client) do(method, url string, body io.Reader) ([]byte, error) { if err != nil { return nil, err } - + if res.StatusCode != http.StatusOK { + err = errors.New(string(responseBody)) + return nil, err + } return responseBody, nil } diff --git a/pkg/cover/client_test.go b/pkg/cover/client_test.go index a950bc7..e242b42 100644 --- a/pkg/cover/client_test.go +++ b/pkg/cover/client_test.go @@ -22,7 +22,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "strings" ) func TestClientAction(t *testing.T) { @@ -32,14 +31,59 @@ func TestClientAction(t *testing.T) { // regsiter service into goc server var src Service - src.Name = "/home/goctest123/_package/newest/goc" + src.Name = "goc" src.Address = "http://127.0.0.1:7777" res, err := client.RegisterService(src) - srvPath := strings.Split(src.Name, "/") - src.Name = srvPath[len(srvPath)-1] assert.NoError(t, err) assert.Contains(t, string(res), "success") + // get porfile from goc server + profileItems := []struct { + param ProfileParam + err string + }{ + { + param: ProfileParam{Force: false, Service: []string{src.Name}, Address: []string{src.Address}}, + err: "invalid param", + }, + { + param: ProfileParam{Force: false, Address: []string{src.Address, "http://unknown.com"}}, + err: "not found", + }, + { + param: ProfileParam{Force: true, Address: []string{src.Address, "http://unknown.com"}}, + err: "no profile", + }, + { + param: ProfileParam{Force: true, Service: []string{src.Name, "unknownSvr"}}, + err: "no profile", + }, + { + param: ProfileParam{Force: false, Service: []string{src.Name, "unknownSvr"}}, + err: "not found", + }, + { + param: ProfileParam{}, + err: "connection refused", + }, + { + param: ProfileParam{Service: []string{src.Name, src.Name}}, + err: "connection refused", + }, + { + param: ProfileParam{Address: []string{src.Address, src.Address}}, + err: "connection refused", + }, + } + for _, item := range profileItems { + res, err = client.Profile(item.param) + if err != nil { + assert.Contains(t, err.Error(), item.err) + } else { + assert.Contains(t, string(res), item.err) + } + } + // do list and check service res, err = client.ListServices() assert.NoError(t, err) diff --git a/pkg/cover/cover.go b/pkg/cover/cover.go index 201dd57..921ec11 100644 --- a/pkg/cover/cover.go +++ b/pkg/cover/cover.go @@ -41,6 +41,7 @@ import ( var ( ErrCoverPkgFailed = errors.New("fail to inject code to project") ErrCoverListFailed = errors.New("fail to list package dependencies") + ErrStoreDuplicated = errors.New("storage duplicated") ) // TestCover is a collection of all counters diff --git a/pkg/cover/instrument.go b/pkg/cover/instrument.go index 4248d85..78a75dd 100644 --- a/pkg/cover/instrument.go +++ b/pkg/cover/instrument.go @@ -54,6 +54,7 @@ import ( "strings" "sync/atomic" "testing" + "path/filepath" {{range $i, $pkgCover := .DepsCover}} _cover{{$i}} {{$pkgCover.Package.ImportPath | printf "%q"}} @@ -212,7 +213,8 @@ func registerHandlers() { } func registerSelf(address string) ([]byte, error) { - req, err := http.NewRequest("POST", fmt.Sprintf("%s/v1/cover/register?name=%s&address=%s", {{.Center | printf "%q"}}, os.Args[0], address), nil) + selfName := filepath.Base(os.Args[0]) + req, err := http.NewRequest("POST", fmt.Sprintf("%s/v1/cover/register?name=%s&address=%s", {{.Center | printf "%q"}}, selfName, address), nil) if err != nil { log.Fatalf("http.NewRequest failed: %v", err) return nil, err diff --git a/pkg/cover/server.go b/pkg/cover/server.go index 8f48132..4939b73 100644 --- a/pkg/cover/server.go +++ b/pkg/cover/server.go @@ -26,12 +26,11 @@ import ( "net/url" "os" - "encoding/json" "github.com/gin-gonic/gin" log "github.com/sirupsen/logrus" "golang.org/x/tools/cover" "k8s.io/test-infra/gopherage/pkg/cov" - "strings" + "strconv" ) // DefaultStore implements the IPersistence interface @@ -69,7 +68,7 @@ func GocServer(w io.Writer) *gin.Engine { v1 := r.Group("/v1") { v1.POST("/cover/register", registerService) - v1.POST("/cover/profile", profile) + v1.GET("/cover/profile", profile) v1.POST("/cover/clear", clear) v1.POST("/cover/init", initSystem) v1.GET("/cover/list", listServices) @@ -86,9 +85,9 @@ type Service struct { // ProfileParam is param of profile API (TODO) type ProfileParam struct { - Force bool `form:"force"` - Name string `form:"name" json:"name"` - Address string `form:"address" json:"address"` + Force bool `form:"force"` + Service []string `form:"service" json:"service"` + Address []string `form:"address" json:"address"` } //listServices list all the registered services @@ -129,69 +128,76 @@ func registerService(c *gin.Context) { } func profile(c *gin.Context) { - respByte, err := ioutil.ReadAll(c.Request.Body) + force, err := strconv.ParseBool(c.Query("force")) if err != nil { - c.JSON(http.StatusInternalServerError, err.Error()) - return - } - var param ProfileParam - json.Unmarshal(respByte, ¶m) - if param.Name != "" && param.Address != "" { c.JSON(http.StatusExpectationFailed, gin.H{"error": "invalid param"}) return } - nameList := strings.Split(param.Name, "&") - addrList := strings.Split(param.Address, "&") + svrList := c.QueryArray("service") + addrList := c.QueryArray("address") svrsAll := DefaultStore.GetAll() svrsUnderTest := make(map[string][]string) - if param.Name == "" && param.Address == "" { + if len(svrList) != 0 && len(addrList) != 0 { + c.JSON(http.StatusExpectationFailed, gin.H{"error": "invalid param"}) + return + } + if len(svrList) == 0 && len(addrList) == 0 { svrsUnderTest = svrsAll } else { - if param.Name != "" { - for _, name := range nameList { - miss := true - for svr, addrs := range svrsAll { - if svr == name { - svrsUnderTest[svr] = addrs - miss = false - } + if len(svrList) != 0 { + for _, name := range svrList { + if addr, ok := svrsAll[name]; ok { + svrsUnderTest[name] = addr + continue } - if miss && !param.Force { + if !force { c.JSON(http.StatusNotFound, fmt.Sprintf("service [%s] not found!", name)) return } } } - if param.Address != "" { + if len(addrList) != 0 { + I: for _, addr := range addrList { - miss := true for svr, addrs := range svrsAll { + if contains(svrsUnderTest[svr], addr) { + continue I + } for _, a := range addrs { if a == addr { svrsUnderTest[svr] = append(svrsUnderTest[svr], a) - miss = false + continue I } } } - if miss && !param.Force { + if !force { c.JSON(http.StatusNotFound, fmt.Sprintf("address [%s] not found!", addr)) return } } } } - var mergedProfiles = make([][]*cover.Profile, len(svrsUnderTest)) + var mergedProfiles = make([][]*cover.Profile, 0) for _, svrs := range svrsUnderTest { for _, addr := range svrs { pp, err := NewWorker(addr).Profile(ProfileParam{}) if err != nil { - c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()}) - return + if !force { + + c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()}) + return + } else { + continue + } } profile, err := convertProfile(pp) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) - return + if !force { + c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + return + } else { + continue + } } mergedProfiles = append(mergedProfiles, profile) } @@ -253,3 +259,12 @@ func convertProfile(p []byte) ([]*cover.Profile, error) { return cover.ParseProfiles(tf.Name()) } + +func contains(arr []string, str string) bool { + for _, element := range arr { + if str == element { + return true + } + } + return false +} diff --git a/pkg/cover/store.go b/pkg/cover/store.go index a18aa9a..d079d28 100644 --- a/pkg/cover/store.go +++ b/pkg/cover/store.go @@ -23,6 +23,7 @@ import ( "strings" "sync" + "errors" log "github.com/sirupsen/logrus" ) @@ -71,8 +72,11 @@ func NewFileStore() Store { // Add adds the given service to file Store func (l *fileStore) Add(s Service) error { - l.memoryStore.Add(s) - + err := l.memoryStore.Add(s) + log.Println(errors.Is(err, ErrStoreDuplicated)) + if errors.Is(err, ErrStoreDuplicated) { + return nil + } // persistent to local store l.mu.Lock() defer l.mu.Unlock() @@ -193,7 +197,7 @@ func (l *memoryStore) Add(s Service) error { for _, addr := range addrs { if addr == s.Address { log.Printf("service registered already, name: %s, address: %s", s.Name, s.Address) - return nil + return ErrStoreDuplicated } } addrs = append(addrs, s.Address) diff --git a/pkg/cover/store_test.go b/pkg/cover/store_test.go index f4bd8aa..512fd87 100644 --- a/pkg/cover/store_test.go +++ b/pkg/cover/store_test.go @@ -17,6 +17,7 @@ package cover import ( + "github.com/stretchr/testify/assert" "testing" ) @@ -42,6 +43,8 @@ func TestLocalStore(t *testing.T) { localStore.Add(tc2) localStore.Add(tc3) localStore.Add(tc4) + err := localStore.Add(tc1) + assert.NoError(t, err) addrs := localStore.Get(tc1.Name) if len(addrs) != 2 { t.Error("unexpected result") @@ -62,3 +65,21 @@ func TestLocalStore(t *testing.T) { t.Error("local store init failed") } } + +func TestMemoryStore(t *testing.T) { + memoryStore := NewMemoryStore() + var tc1 = Service{ + Name: "a", + Address: "http://127.0.0.1", + } + err := memoryStore.Add(tc1) + if err != nil { + t.Error("add server to memory store failed") + } + addrs := memoryStore.Get(tc1.Name) + if len(addrs) != 1 { + t.Error("memory store check failed") + } + err = memoryStore.Add(tc1) + assert.Equal(t, err, ErrStoreDuplicated) +}