From 4143a2b33970ce02b4a456f17e911cbadbbdb733 Mon Sep 17 00:00:00 2001 From: Kubernetes Prow Robot Date: Fri, 9 Apr 2021 05:49:05 -0700 Subject: [PATCH] Merge pull request #3989 from brett-elliott/useragent Set cluster autoscaler-specific user agent. --- .../gce/autoscaling_gce_client.go | 3 +- .../gce/autoscaling_gce_client_test.go | 23 ++++-- .../cloudprovider/gce/gce_cloud_provider.go | 2 +- .../cloudprovider/gce/gce_manager.go | 4 +- .../cloudprovider/gce/gce_manager_test.go | 2 +- .../packet/packet_manager_rest_test.go | 6 +- .../packet/packet_node_group_test.go | 19 ++--- .../config/autoscaling_options.go | 2 + cluster-autoscaler/main.go | 2 + cluster-autoscaler/utils/test/test_utils.go | 72 ++++++++++++++++--- 10 files changed, 105 insertions(+), 30 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go index ffc8e046274f..9ae30feb07a7 100644 --- a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go +++ b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go @@ -78,11 +78,12 @@ type autoscalingGceClientV1 struct { } // NewAutoscalingGceClientV1 creates a new client for communicating with GCE v1 API. -func NewAutoscalingGceClientV1(client *http.Client, projectId string) (*autoscalingGceClientV1, error) { +func NewAutoscalingGceClientV1(client *http.Client, projectId string, userAgent string) (*autoscalingGceClientV1, error) { gceService, err := gce.New(client) if err != nil { return nil, err } + gceService.UserAgent = userAgent return &autoscalingGceClientV1{ projectId: projectId, diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go index 1cc442d15b18..15c34a457fd5 100644 --- a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go +++ b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go @@ -28,9 +28,9 @@ import ( gce_api "google.golang.org/api/compute/v1" ) -func newTestAutoscalingGceClient(t *testing.T, projectId, url string) *autoscalingGceClientV1 { +func newTestAutoscalingGceClient(t *testing.T, projectId, url, userAgent string) *autoscalingGceClientV1 { client := &http.Client{} - gceClient, err := NewAutoscalingGceClientV1(client, projectId) + gceClient, err := NewAutoscalingGceClientV1(client, projectId, userAgent) if !assert.NoError(t, err) { t.Fatalf("fatal error: %v", err) } @@ -63,7 +63,7 @@ const operationDoneResponse = `{ func TestWaitForOp(t *testing.T) { server := test_util.NewHttpServerMock() defer server.Close() - g := newTestAutoscalingGceClient(t, "project1", server.URL) + g := newTestAutoscalingGceClient(t, "project1", server.URL, "") g.operationPollInterval = 1 * time.Millisecond g.operationWaitTimeout = 500 * time.Millisecond @@ -81,7 +81,7 @@ func TestWaitForOp(t *testing.T) { func TestWaitForOpTimeout(t *testing.T) { server := test_util.NewHttpServerMock() defer server.Close() - g := newTestAutoscalingGceClient(t, "project1", server.URL) + g := newTestAutoscalingGceClient(t, "project1", server.URL, "") // The values here are higher than in other tests since we're aiming for timeout. // Lower values make this fragile and flakey. @@ -97,3 +97,18 @@ func TestWaitForOpTimeout(t *testing.T) { err := g.waitForOp(operation, projectId, zoneB) assert.Error(t, err) } + +func TestUserAgent(t *testing.T) { + server := test_util.NewHttpServerMock(test_util.MockFieldUserAgent, test_util.MockFieldResponse) + defer server.Close() + g := newTestAutoscalingGceClient(t, "project1", server.URL, "testuseragent") + + g.operationPollInterval = 10 * time.Millisecond + g.operationWaitTimeout = 49 * time.Millisecond + + server.On("handle", "/project1/zones/us-central1-b/operations/operation-1505728466148-d16f5197").Return("testuseragent", operationRunningResponse).Maybe() + + operation := &gce_api.Operation{Name: "operation-1505728466148-d16f5197"} + + g.waitForOp(operation, projectId, zoneB) +} diff --git a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go index 4161e842f22d..a168bcbbfd56 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go @@ -350,7 +350,7 @@ func BuildGCE(opts config.AutoscalingOptions, do cloudprovider.NodeGroupDiscover defer config.Close() } - manager, err := CreateGceManager(config, do, opts.Regional) + manager, err := CreateGceManager(config, do, opts.Regional, opts.UserAgent) if err != nil { klog.Fatalf("Failed to create GCE Manager: %v", err) } diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager.go b/cluster-autoscaler/cloudprovider/gce/gce_manager.go index dfb9f89c3ec4..b004f13010ff 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_manager.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_manager.go @@ -113,7 +113,7 @@ type gceManagerImpl struct { } // CreateGceManager constructs GceManager object. -func CreateGceManager(configReader io.Reader, discoveryOpts cloudprovider.NodeGroupDiscoveryOptions, regional bool) (GceManager, error) { +func CreateGceManager(configReader io.Reader, discoveryOpts cloudprovider.NodeGroupDiscoveryOptions, regional bool, userAgent string) (GceManager, error) { // Create Google Compute Engine token. var err error tokenSource := google.ComputeTokenSource("") @@ -163,7 +163,7 @@ func CreateGceManager(configReader io.Reader, discoveryOpts cloudprovider.NodeGr // Create Google Compute Engine service. client := oauth2.NewClient(oauth2.NoContext, tokenSource) client.Timeout = httpTimeout - gceService, err := NewAutoscalingGceClientV1(client, projectId) + gceService, err := NewAutoscalingGceClientV1(client, projectId, userAgent) if err != nil { return nil, err } diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go index b0f27ebb2387..224e575aadc8 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go @@ -325,7 +325,7 @@ func buildListInstanceGroupManagersResponse(listInstanceGroupManagerResponsePart } func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceManagerImpl { - gceService := newTestAutoscalingGceClient(t, projectId, testServerURL) + gceService := newTestAutoscalingGceClient(t, projectId, testServerURL, "") // Override wait for op timeouts. gceService.operationWaitTimeout = 50 * time.Millisecond diff --git a/cluster-autoscaler/cloudprovider/packet/packet_manager_rest_test.go b/cluster-autoscaler/cloudprovider/packet/packet_manager_rest_test.go index 3cf2232e290f..a0bac3525ab6 100644 --- a/cluster-autoscaler/cloudprovider/packet/packet_manager_rest_test.go +++ b/cluster-autoscaler/cloudprovider/packet/packet_manager_rest_test.go @@ -76,7 +76,7 @@ func newTestPacketManagerRest(t *testing.T, url string) *packetManagerRest { } func TestListPacketDevices(t *testing.T) { var m *packetManagerRest - server := NewHttpServerMock() + server := NewHttpServerMock(MockFieldContentType, MockFieldResponse) defer server.Close() if len(os.Getenv("PACKET_AUTH_TOKEN")) > 0 { // If auth token set in env, hit the actual Packet API @@ -84,7 +84,9 @@ func TestListPacketDevices(t *testing.T) { } else { // Set up a mock Packet API m = newTestPacketManagerRest(t, server.URL) - server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return(listPacketDevicesResponse).Times(2) + t.Logf("server URL: %v", server.URL) + t.Logf("default packetManagerNodePool baseURL: %v", m.packetManagerNodePools["default"].baseURL) + server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return("application/json", listPacketDevicesResponse).Times(2) } _, err := m.listPacketDevices() diff --git a/cluster-autoscaler/cloudprovider/packet/packet_node_group_test.go b/cluster-autoscaler/cloudprovider/packet/packet_node_group_test.go index dba5a5647611..b25cc45332b3 100644 --- a/cluster-autoscaler/cloudprovider/packet/packet_node_group_test.go +++ b/cluster-autoscaler/cloudprovider/packet/packet_node_group_test.go @@ -38,7 +38,7 @@ const getPacketDeviceResponsePool3 = ` func TestIncreaseDecreaseSize(t *testing.T) { var m *packetManagerRest - server := NewHttpServerMock() + server := NewHttpServerMock(MockFieldContentType, MockFieldResponse) defer server.Close() assert.Equal(t, true, true) if len(os.Getenv("PACKET_AUTH_TOKEN")) > 0 { @@ -47,14 +47,15 @@ func TestIncreaseDecreaseSize(t *testing.T) { } else { // Set up a mock Packet API m = newTestPacketManagerRest(t, server.URL) - server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return(listPacketDevicesResponse).Times(4) - server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return(listPacketDevicesResponseAfterIncreasePool3).Times(3) - server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return(listPacketDevicesResponseAfterIncreasePool2).Times(1) - server.On("handle", "/devices/0f5609af-1c27-451b-8edd-a1283f2c9440").Return(getPacketDeviceResponsePool2).Times(1) - server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return(listPacketDevicesResponseAfterIncreasePool2).Times(4) - server.On("handle", "/devices/8fa90049-e715-4794-ba31-81c1c78cee84").Return(getPacketDeviceResponsePool3).Times(1) - server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return(listPacketDevicesResponseAfterIncreasePool2).Times(2) - server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return(listPacketDevicesResponse).Times(2) + server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return("application/json", listPacketDevicesResponse).Times(3) + server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return("application/json", createPacketDeviceResponsePool3).Times(1) + server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return("application/json", listPacketDevicesResponseAfterIncreasePool3).Times(2) + server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return("application/json", createPacketDeviceResponsePool2).Times(1) + server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return("application/json", listPacketDevicesResponseAfterIncreasePool2).Times(3) + server.On("handle", "/devices/0f5609af-1c27-451b-8edd-a1283f2c9440").Return("application/json", deletePacketDeviceResponsePool2).Times(1) + server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return("application/json", listPacketDevicesResponseAfterIncreasePool3).Times(3) + server.On("handle", "/devices/8fa90049-e715-4794-ba31-81c1c78cee84").Return("application/json", deletePacketDeviceResponsePool3).Times(1) + server.On("handle", "/projects/"+m.packetManagerNodePools["default"].projectID+"/devices").Return("application/json", listPacketDevicesResponse).Times(3) } clusterUpdateLock := sync.Mutex{} ngPool2 := &packetNodeGroup{ diff --git a/cluster-autoscaler/config/autoscaling_options.go b/cluster-autoscaler/config/autoscaling_options.go index 82d28e557a49..d47f4b97ee73 100644 --- a/cluster-autoscaler/config/autoscaling_options.go +++ b/cluster-autoscaler/config/autoscaling_options.go @@ -151,4 +151,6 @@ type AutoscalingOptions struct { ClusterAPICloudConfigAuthoritative bool // Enable or disable cordon nodes functionality before terminating the node during downscale process CordonNodeBeforeTerminate bool + // User agent to use for HTTP calls. + UserAgent string } diff --git a/cluster-autoscaler/main.go b/cluster-autoscaler/main.go index 63d89e5a2af9..65612bbe462e 100644 --- a/cluster-autoscaler/main.go +++ b/cluster-autoscaler/main.go @@ -177,6 +177,7 @@ var ( enableProfiling = flag.Bool("profiling", false, "Is debug/pprof endpoint enabled") clusterAPICloudConfigAuthoritative = flag.Bool("clusterapi-cloud-config-authoritative", false, "Treat the cloud-config flag authoritatively (do not fallback to using kubeconfig flag). ClusterAPI only") cordonNodeBeforeTerminate = flag.Bool("cordon-node-before-terminating", false, "Should CA cordon nodes before terminating during downscale process") + userAgent = flag.String("user-agent", "cluster-autoscaler", "User agent used for HTTP calls.") ) func createAutoscalingOptions() config.AutoscalingOptions { @@ -249,6 +250,7 @@ func createAutoscalingOptions() config.AutoscalingOptions { AWSUseStaticInstanceList: *awsUseStaticInstanceList, ClusterAPICloudConfigAuthoritative: *clusterAPICloudConfigAuthoritative, CordonNodeBeforeTerminate: *cordonNodeBeforeTerminate, + UserAgent: *userAgent, } } diff --git a/cluster-autoscaler/utils/test/test_utils.go b/cluster-autoscaler/utils/test/test_utils.go index 14a819f088de..c0f38bcfc0af 100644 --- a/cluster-autoscaler/utils/test/test_utils.go +++ b/cluster-autoscaler/utils/test/test_utils.go @@ -18,17 +18,16 @@ package test import ( "fmt" - "time" - "net/http" "net/http/httptest" + "strings" + "time" + "github.com/stretchr/testify/mock" apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - - "github.com/stretchr/testify/mock" ) // BuildTestPod creates a pod with specified resources. @@ -213,19 +212,54 @@ func boolptr(val bool) *bool { // instances, err := g.GetManagedInstances() // // Check if expected calls were executed. // mock.AssertExpectationsForObjects(t, server) +// +// Note: to provide a content type, you may pass in the desired +// fields: +// server := NewHttpServerMock(MockFieldContentType, MockFieldResponse) +// ... +// server.On("handle", "/project1/zones/us-central1-b/listManagedInstances").Return("", "").Once() +// The order of the return objects must match that of the HttpServerMockField constants passed to NewHttpServerMock() type HttpServerMock struct { mock.Mock *httptest.Server + fields []HttpServerMockField } +// HttpServerMockField specifies a type of field. +type HttpServerMockField int + +const ( + // MockFieldResponse represents a string response. + MockFieldResponse HttpServerMockField = iota + // MockFieldStatusCode represents an integer HTTP response code. + MockFieldStatusCode + // MockFieldContentType represents a string content type. + MockFieldContentType + // MockFieldUserAgent represents a string user agent. + MockFieldUserAgent +) + // NewHttpServerMock creates new HttpServerMock. -func NewHttpServerMock() *HttpServerMock { - httpServerMock := &HttpServerMock{} +func NewHttpServerMock(fields ...HttpServerMockField) *HttpServerMock { + if len(fields) == 0 { + fields = []HttpServerMockField{MockFieldResponse} + } + foundResponse := false + for _, field := range fields { + if field == MockFieldResponse { + foundResponse = true + break + } + } + if !foundResponse { + panic("Must use MockFieldResponse.") + } + httpServerMock := &HttpServerMock{fields: fields} mux := http.NewServeMux() mux.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) { - result := httpServerMock.handle(req.URL.Path) - w.Write([]byte(result)) + result := httpServerMock.handle(req, w, httpServerMock) + _, _ = w.Write([]byte(result)) }) server := httptest.NewServer(mux) @@ -233,7 +267,25 @@ func NewHttpServerMock() *HttpServerMock { return httpServerMock } -func (l *HttpServerMock) handle(url string) string { +func (l *HttpServerMock) handle(req *http.Request, w http.ResponseWriter, serverMock *HttpServerMock) string { + url := req.URL.Path + var response string args := l.Called(url) - return args.String(0) + for i, field := range l.fields { + switch field { + case MockFieldResponse: + response = args.String(i) + case MockFieldContentType: + w.Header().Set("Content-Type", args.String(i)) + case MockFieldStatusCode: + w.WriteHeader(args.Int(i)) + case MockFieldUserAgent: + gotUserAgent := req.UserAgent() + expectedUserAgent := args.String(i) + if !strings.Contains(gotUserAgent, expectedUserAgent) { + panic(fmt.Sprintf("Error handling URL %s, expected user agent %s but got %s.", url, expectedUserAgent, gotUserAgent)) + } + } + } + return response }