Merge pull request #67 from tongjingran/66

cmd profile support get by name or address list

cmd/profile.go

@@ -35,17 +35,25 @@ 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. You can get all available service names from command 'goc list'. Use 'service' and 'address' flag at the same time is illegal.
+goc profile --service=service1,service2,service3
+
+# Get coverage counter of several specified addresses. You can get all available addresses from command 'goc list'. Use 'service' and 'address' flag at the same time is illegal.
+goc profile --address=address1,address2,address3
+
+# Force to get the coverage counter of all the available services you want.
+goc profile --force
 `,
 	Run: func(cmd *cobra.Command, args []string) {
-		res, err := cover.NewWorker(center).Profile()
+		p := cover.ProfileParam{
+			Force:   force,
+			Service: svrList,
+			Address: addrList,
+		}
+		res, err := cover.NewWorker(center).Profile(p)
 		if err != nil {
 			log.Fatalf("call host %v failed, err: %v, response: %v", center, err, string(res))
 		}
@@ -67,9 +75,15 @@ 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().StringSliceVarP(&svrList, "service", "", nil, "get the cover profile of these services, you can get all available service names from command `goc list`, use this flag and 'address' flag at the same time is illegal.")
+	profileCmd.Flags().StringSliceVarP(&addrList, "address", "", nil, "get the cover profile of these addresses, you can get all available addresses from command `goc list`, use this flag and 'service' flag at the same time is illegal.")
+	profileCmd.Flags().BoolVarP(&force, "force", "f", false, "force to get the coverage counter of all the available services you want")
 	addBasicFlags(profileCmd.Flags())
 	rootCmd.AddCommand(profileCmd)
 }

go.mod

@@ -19,5 +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
 	k8s.io/test-infra v0.0.0-20200511080351-8ac9dbfab055
 )

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=

pkg/cover/client.go

@@ -24,12 +24,13 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"strconv"
 	"strings"
 )
 
 // Action provides methods to contact with the covered service under test
 type Action interface {
-	Profile() ([]byte, error)
+	Profile(param ProfileParam) ([]byte, error)
 	Clear() ([]byte, error)
 	InitSystem() ([]byte, error)
 	ListServices() ([]byte, error)
@@ -88,13 +89,22 @@ func (c *client) ListServices() ([]byte, error) {
 	return services, err
 }
 
-func (c *client) Profile() ([]byte, error) {
+func (c *client) Profile(param ProfileParam) ([]byte, error) {
-	u := fmt.Sprintf("%s%s", c.Host, CoverProfileAPI)
+	u := fmt.Sprintf("%s%s?force=%s", c.Host, CoverProfileAPI, strconv.FormatBool(param.Force))
+	if len(param.Service) != 0 && len(param.Address) != 0 {
+		return nil, fmt.Errorf("use 'service' and 'address' flag at the same time is illegal")
+	}
+
+	for _, svr := range param.Service {
+		u = u + "&service=" + svr
+	}
+	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("GET", u, nil)
 	}
-
 	return profile, err
 }
 
@@ -126,7 +136,6 @@ func (c *client) do(method, url string, body io.Reader) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-
 	return responseBody, nil
 }
 

pkg/cover/client_test.go

@@ -22,17 +22,33 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"net/http"
 )
 
 func TestClientAction(t *testing.T) {
+	// mock goc server
 	ts := httptest.NewServer(GocServer(os.Stdout))
 	defer ts.Close()
 	var client = NewWorker(ts.URL)
 
+	// mock profile server
+	profileMockResponse := "mode: count\nmockService/main.go:30.13,48.33 13 1"
+	profileSuccessMockSvr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(profileMockResponse))
+	}))
+	defer profileSuccessMockSvr.Close()
+
+	profileErrMockSvr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte("error"))
+	}))
+	defer profileErrMockSvr.Close()
+
 	// regsiter service into goc server
 	var src Service
-	src.Name = "goc"
+	src.Name = "serviceSuccess"
-	src.Address = "http://127.0.0.1:7777"
+	src.Address = profileSuccessMockSvr.URL
 	res, err := client.RegisterService(src)
 	assert.NoError(t, err)
 	assert.Contains(t, string(res), "success")
@@ -43,6 +59,72 @@ func TestClientAction(t *testing.T) {
 	assert.Contains(t, string(res), src.Address)
 	assert.Contains(t, string(res), src.Name)
 
+	// get porfile from goc server
+	profileItems := []struct {
+		service Service
+		param   ProfileParam
+		res     string
+	}{
+		{
+			service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:   ProfileParam{Force: false, Service: []string{"serviceOK"}, Address: []string{profileSuccessMockSvr.URL}},
+			res:     "use 'service' and 'address' flag at the same time is illegal",
+		},
+		{
+			service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:   ProfileParam{},
+			res:     profileMockResponse,
+		},
+		{
+			service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:   ProfileParam{Service: []string{"serviceOK"}},
+			res:     profileMockResponse,
+		},
+		{
+			service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:   ProfileParam{Address: []string{profileSuccessMockSvr.URL}},
+			res:     profileMockResponse,
+		},
+		{
+			service: Service{Name: "serviceOK", Address: profileSuccessMockSvr.URL},
+			param:   ProfileParam{Service: []string{"unknown"}},
+			res:     "service [unknown] not found",
+		},
+		{
+			service: Service{Name: "serviceErr", Address: profileErrMockSvr.URL},
+			res:     "bad mode line: error",
+		},
+		{
+			service: Service{Name: "serviceErr", Address: profileErrMockSvr.URL},
+			param:   ProfileParam{Force: true},
+			res:     "no profiles",
+		},
+		{
+			service: Service{Name: "serviceNotExist", Address: "http://172.0.0.2:7777"},
+			res:     "connection refused",
+		},
+		{
+			service: Service{Name: "serviceNotExist", Address: "http://172.0.0.2:7777"},
+			param:   ProfileParam{Force: true},
+			res:     "no profiles",
+		},
+	}
+	for _, item := range profileItems {
+		// init server
+		res, err := client.InitSystem()
+		assert.NoError(t, err)
+		// register server
+		res, err = client.RegisterService(item.service)
+		assert.NoError(t, err)
+		assert.Contains(t, string(res), "success")
+		res, err = client.Profile(item.param)
+		if err != nil {
+			assert.Equal(t, err.Error(), item.res)
+		} else {
+			assert.Contains(t, string(res), item.res)
+		}
+	}
+
 	// init system and check service again
 	res, err = client.InitSystem()
 	assert.NoError(t, err)

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

pkg/cover/server.go

@@ -30,6 +30,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/tools/cover"
 	"k8s.io/test-infra/gopherage/pkg/cov"
+	"strconv"
 )
 
 // DefaultStore implements the IPersistence interface
@@ -82,6 +83,13 @@ type Service struct {
 	Address string `form:"address" json:"address" binding:"required"`
 }
 
+// ProfileParam is param of profile API (TODO)
+type ProfileParam struct {
+	Force   bool     `form:"force"`
+	Service []string `form:"service" json:"service"`
+	Address []string `form:"address" json:"address"`
+}
+
 //listServices list all the registered services
 func listServices(c *gin.Context) {
 	services := DefaultStore.GetAll()
@@ -111,26 +119,49 @@ func registerService(c *gin.Context) {
 		log.Printf("the registed host %s of service %s is different with the real one %s, here we choose the real one", service.Name, host, realIP)
 		service.Address = fmt.Sprintf("http://%s:%s", realIP, port)
 	}
-	if err := DefaultStore.Add(service); err != nil {
+
-		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+	address := DefaultStore.Get(service.Name)
-		return
+	if !contains(address, service.Address) {
+		if err := DefaultStore.Add(service); err != nil {
+			c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+			return
+		}
 	}
 
 	c.JSON(http.StatusOK, gin.H{"result": "success"})
+	return
 }
 
 func profile(c *gin.Context) {
-	svrsUnderTest := DefaultStore.GetAll()
+	force, err := strconv.ParseBool(c.Query("force"))
-	var mergedProfiles = make([][]*cover.Profile, len(svrsUnderTest))
+	if err != nil {
-	for _, addrs := range svrsUnderTest {
+		c.JSON(http.StatusExpectationFailed, gin.H{"error": "invalid param"})
-		for _, addr := range addrs {
+		return
-			pp, err := NewWorker(addr).Profile()
+	}
+	svrList := c.QueryArray("service")
+	addrList := c.QueryArray("address")
+	svrsAll := DefaultStore.GetAll()
+	svrsUnderTest, err := getSvrUnderTest(svrList, addrList, force, svrsAll)
+	if err != nil {
+		c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
+	}
+
+	var mergedProfiles = make([][]*cover.Profile, 0)
+	for _, svrs := range svrsUnderTest {
+		for _, addr := range svrs {
+			pp, err := NewWorker(addr).Profile(ProfileParam{})
 			if err != nil {
+				if force {
+					continue
+				}
 				c.JSON(http.StatusExpectationFailed, gin.H{"error": err.Error()})
 				return
 			}
 			profile, err := convertProfile(pp)
 			if err != nil {
+				if force {
+					continue
+				}
 				c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 				return
 			}
@@ -194,3 +225,58 @@ 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
+}
+
+// getSvrUnderTest get service map by service and address list
+func getSvrUnderTest(svrList, addrList []string, force bool, svrsAll map[string][]string) (svrsUnderTest map[string][]string, err error) {
+	svrsUnderTest = map[string][]string{}
+	if len(svrList) != 0 && len(addrList) != 0 {
+		return nil, fmt.Errorf("use this flag and 'address' flag at the same time is illegal")
+	}
+	// Return all servers when all param is nil
+	if len(svrList) == 0 && len(addrList) == 0 {
+		return svrsAll, nil
+	} else {
+		// Add matched services to map
+		if len(svrList) != 0 {
+			for _, name := range svrList {
+				if addr, ok := svrsAll[name]; ok {
+					svrsUnderTest[name] = addr
+					continue // jump to match the next service
+				}
+				if !force {
+					return nil, fmt.Errorf("service [%s] not found", name)
+				}
+			}
+		}
+		// Add matched addresses to map
+		if len(addrList) != 0 {
+		I:
+			for _, addr := range addrList {
+				for svr, addrs := range svrsAll {
+					if contains(svrsUnderTest[svr], addr) {
+						continue I // The address is duplicate, jump over
+					}
+					for _, a := range addrs {
+						if a == addr {
+							svrsUnderTest[svr] = append(svrsUnderTest[svr], a)
+							continue I // jump to match the next address
+						}
+					}
+				}
+				if !force {
+					return nil, fmt.Errorf("address [%s] not found", addr)
+				}
+			}
+		}
+	}
+	return svrsUnderTest, nil
+}

pkg/cover/server_test.go

@@ -0,0 +1,60 @@
+package cover
+
+import (
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestContains(t *testing.T) {
+	assert.Equal(t, contains([]string{"a", "b"}, "a"), true)
+	assert.Equal(t, contains([]string{"a", "b"}, "c"), false)
+}
+
+func TestGetSvrUnderTest(t *testing.T) {
+	svrAll := map[string][]string{
+		"service1": {"http://127.0.0.1:7777", "http://127.0.0.1:8888"},
+		"service2": {"http://127.0.0.1:9999"},
+	}
+	items := []struct {
+		svrList  []string
+		addrList []string
+		force    bool
+		err      string
+		svrRes   map[string][]string
+	}{
+		{
+			svrList:  []string{"service1"},
+			addrList: []string{"http://127.0.0.1:7777"},
+			err:      "use this flag and 'address' flag at the same time is illegal",
+		},
+		{
+			svrRes: svrAll,
+		},
+		{
+			svrList: []string{"service1", "unknown"},
+			err:     "service [unknown] not found",
+		},
+		{
+			svrList: []string{"service1", "service1", "service2", "unknown"},
+			force:   true,
+			svrRes:  svrAll,
+		},
+		{
+			addrList: []string{"http://127.0.0.1:7777", "http://127.0.0.2:7777"},
+			err:      "address [http://127.0.0.2:7777] not found",
+		},
+		{
+			addrList: []string{"http://127.0.0.1:7777", "http://127.0.0.1:7777", "http://127.0.0.1:9999", "http://127.0.0.2:7777"},
+			force:    true,
+			svrRes:   map[string][]string{"service1": {"http://127.0.0.1:7777"}, "service2": {"http://127.0.0.1:9999"}},
+		},
+	}
+	for _, item := range items {
+		svrs, err := getSvrUnderTest(item.svrList, item.addrList, item.force, svrAll)
+		if err != nil {
+			assert.Equal(t, err.Error(), item.err)
+		} else {
+			assert.Equal(t, svrs, item.svrRes)
+		}
+	}
+}