Merge pull request #74746 from oomichi/golint-e2e-framework-gce

hack/.golint_failures

@@ -649,7 +649,6 @@ test/e2e/autoscaling
 test/e2e/chaosmonkey
 test/e2e/common
 test/e2e/framework
-test/e2e/framework/providers/gce
 test/e2e/lifecycle
 test/e2e/lifecycle/bootstrap
 test/e2e/network

test/e2e/framework/providers/gce/firewall.go

@@ -33,14 +33,6 @@ import (
 	"k8s.io/kubernetes/test/e2e/framework"
 )
 
-const (
-	FirewallTimeoutDefault = 3 * time.Minute
-	FirewallTestTcpTimeout = time.Duration(1 * time.Second)
-	// Set ports outside of 30000-32767, 80 and 8080 to avoid being whitelisted by the e2e cluster
-	FirewallTestHttpPort = int32(29999)
-	FirewallTestUdpPort  = int32(29998)
-)
-
 // MakeFirewallNameForLBService return the expected firewall name for a LB service.
 // This should match the formatting of makeFirewallName() in pkg/cloudprovider/providers/gce/gce_loadbalancer.go
 func MakeFirewallNameForLBService(name string) string {
@@ -69,6 +61,8 @@ func ConstructFirewallForLBService(svc *v1.Service, nodeTag string) *compute.Fir
 	return &fw
 }
 
+// MakeHealthCheckFirewallNameForLBService returns the firewall name used by the GCE load
+// balancers for performing health checks.
 func MakeHealthCheckFirewallNameForLBService(clusterID, name string, isNodesHealthCheck bool) string {
 	return gcecloud.MakeHealthCheckFirewallName(clusterID, name, isNodesHealthCheck)
 }
@@ -114,7 +108,7 @@ func GetClusterName(instancePrefix string) string {
 
 // GetE2eFirewalls returns all firewall rules we create for an e2e cluster.
 // From cluster/gce/util.sh, all firewall rules should be consistent with the ones created by startup scripts.
-func GetE2eFirewalls(masterName, masterTag, nodeTag, network, clusterIpRange string) []*compute.Firewall {
+func GetE2eFirewalls(masterName, masterTag, nodeTag, network, clusterIPRange string) []*compute.Firewall {
 	instancePrefix, err := GetInstancePrefix(masterName)
 	framework.ExpectNoError(err)
 	clusterName := GetClusterName(instancePrefix)
@@ -198,7 +192,7 @@ func GetE2eFirewalls(masterName, masterTag, nodeTag, network, clusterIpRange str
 	})
 	fws = append(fws, &compute.Firewall{
 		Name:         nodeTag + "-all",
-		SourceRanges: []string{clusterIpRange},
+		SourceRanges: []string{clusterIPRange},
 		TargetTags:   []string{nodeTag},
 		Allowed: []*compute.FirewallAllowed{
 			{
@@ -399,6 +393,7 @@ func VerifyFirewallRule(res, exp *compute.Firewall, network string, portsSubset
 	return nil
 }
 
+// WaitForFirewallRule waits for the specified firewall existence
 func WaitForFirewallRule(gceCloud *gcecloud.Cloud, fwName string, exist bool, timeout time.Duration) (*compute.Firewall, error) {
 	framework.Logf("Waiting up to %v for firewall %v exist=%v", timeout, fwName, exist)
 	var fw *compute.Firewall

test/e2e/framework/providers/gce/gce.go

@@ -89,17 +89,20 @@ func factory() (framework.ProviderInterface, error) {
 	return NewProvider(gceCloud), nil
 }
 
+// NewProvider returns a cloud provider interface for GCE
 func NewProvider(gceCloud *gcecloud.Cloud) framework.ProviderInterface {
 	return &Provider{
 		gceCloud: gceCloud,
 	}
 }
 
+// Provider is a structure to handle GCE clouds for e2e testing
 type Provider struct {
 	framework.NullProvider
 	gceCloud *gcecloud.Cloud
 }
 
+// ResizeGroup resizes an instance group
 func (p *Provider) ResizeGroup(group string, size int32) error {
 	// TODO: make this hit the compute API directly instead of shelling out to gcloud.
 	// TODO: make gce/gke implement InstanceGroups, so we can eliminate the per-provider logic
@@ -116,6 +119,7 @@ func (p *Provider) ResizeGroup(group string, size int32) error {
 	return nil
 }
 
+// GetGroupNodes returns a node name for the specified node group
 func (p *Provider) GetGroupNodes(group string) ([]string, error) {
 	// TODO: make this hit the compute API directly instead of shelling out to gcloud.
 	// TODO: make gce/gke implement InstanceGroups, so we can eliminate the per-provider logic
@@ -137,6 +141,7 @@ func (p *Provider) GetGroupNodes(group string) ([]string, error) {
 	return lines, nil
 }
 
+// GroupSize returns the size of an instance group
 func (p *Provider) GroupSize(group string) (int, error) {
 	// TODO: make this hit the compute API directly instead of shelling out to gcloud.
 	// TODO: make gce/gke implement InstanceGroups, so we can eliminate the per-provider logic
@@ -154,6 +159,7 @@ func (p *Provider) GroupSize(group string) (int, error) {
 	return len(re.FindAllString(string(output), -1)), nil
 }
 
+// EnsureLoadBalancerResourcesDeleted ensures that cloud load balancer resources that were created
 func (p *Provider) EnsureLoadBalancerResourcesDeleted(ip, portRange string) error {
 	project := framework.TestContext.CloudConfig.ProjectID
 	region, err := gcecloud.GetGCERegion(framework.TestContext.CloudConfig.Zone)
@@ -190,6 +196,7 @@ func getGCEZoneForGroup(group string) (string, error) {
 	return zone, nil
 }
 
+// DeleteNode deletes a node which is specified as the argument
 func (p *Provider) DeleteNode(node *v1.Node) error {
 	zone := framework.TestContext.CloudConfig.Zone
 	project := framework.TestContext.CloudConfig.ProjectID
@@ -197,6 +204,7 @@ func (p *Provider) DeleteNode(node *v1.Node) error {
 	return p.gceCloud.DeleteInstance(project, zone, node.Name)
 }
 
+// CreatePD creates a persistent volume
 func (p *Provider) CreatePD(zone string) (string, error) {
 	pdName := fmt.Sprintf("%s-%s", framework.TestContext.Prefix, string(uuid.NewUUID()))
 
@@ -215,6 +223,7 @@ func (p *Provider) CreatePD(zone string) (string, error) {
 	return pdName, nil
 }
 
+// DeletePD deletes a persistent volume
 func (p *Provider) DeletePD(pdName string) error {
 	err := p.gceCloud.DeleteDisk(pdName)
 
@@ -229,6 +238,7 @@ func (p *Provider) DeletePD(pdName string) error {
 	return err
 }
 
+// CreatePVSource creates a persistent volume source
 func (p *Provider) CreatePVSource(zone, diskName string) (*v1.PersistentVolumeSource, error) {
 	return &v1.PersistentVolumeSource{
 		GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
@@ -239,11 +249,12 @@ func (p *Provider) CreatePVSource(zone, diskName string) (*v1.PersistentVolumeSo
 	}, nil
 }
 
+// DeletePVSource deletes a persistent volume source
 func (p *Provider) DeletePVSource(pvSource *v1.PersistentVolumeSource) error {
 	return framework.DeletePDWithRetry(pvSource.GCEPersistentDisk.PDName)
 }
 
-// CleanupResources cleans up GCE Service Type=LoadBalancer resources with
+// CleanupServiceResources cleans up GCE Service Type=LoadBalancer resources with
 // the given name. The name is usually the UUID of the Service prefixed with an
 // alpha-numeric character ('a') to work around cloudprovider rules.
 func (p *Provider) CleanupServiceResources(c clientset.Interface, loadBalancerName, region, zone string) {
@@ -301,10 +312,13 @@ func (p *Provider) cleanupGCEResources(c clientset.Interface, loadBalancerName,
 	return
 }
 
+// LoadBalancerSrcRanges contains the ranges of ips used by the GCE load balancers (l4 & L7)
+// for proxying client requests and performing health checks.
 func (p *Provider) LoadBalancerSrcRanges() []string {
 	return gcecloud.LoadBalancerSrcRanges()
 }
 
+// EnableAndDisableInternalLB returns functions for both enabling and disabling internal Load Balancer
 func (p *Provider) EnableAndDisableInternalLB() (enable, disable func(svc *v1.Service)) {
 	enable = func(svc *v1.Service) {
 		svc.ObjectMeta.Annotations = map[string]string{gcecloud.ServiceAnnotationLoadBalancerType: string(gcecloud.LBTypeInternal)}
@@ -351,13 +365,14 @@ func GetNodeTags(c clientset.Interface, cloudConfig framework.CloudConfig) []str
 	return GetInstanceTags(cloudConfig, nodes.Items[0].Name).Items
 }
 
-// IsHTTPErrorCode returns true if the error is a google api
+// IsGoogleAPIHTTPErrorCode returns true if the error is a google api
 // error matching the corresponding HTTP error code.
 func IsGoogleAPIHTTPErrorCode(err error, code int) bool {
 	apiErr, ok := err.(*googleapi.Error)
 	return ok && apiErr.Code == code
 }
 
+// GetGCECloud returns GCE cloud provider
 func GetGCECloud() (*gcecloud.Cloud, error) {
 	p, ok := framework.TestContext.CloudConfig.Provider.(*Provider)
 	if !ok {
@@ -366,6 +381,7 @@ func GetGCECloud() (*gcecloud.Cloud, error) {
 	return p.gceCloud, nil
 }
 
+// GetClusterID returns cluster ID
 func GetClusterID(c clientset.Interface) (string, error) {
 	cm, err := c.CoreV1().ConfigMaps(metav1.NamespaceSystem).Get(gcecloud.UIDConfigMapName, metav1.GetOptions{})
 	if err != nil || cm == nil {

test/e2e/network/firewall.go

@@ -33,6 +33,13 @@ import (
 	. "github.com/onsi/gomega"
 )
 
+const (
+	firewallTestTCPTimeout = time.Duration(1 * time.Second)
+	// Set ports outside of 30000-32767, 80 and 8080 to avoid being whitelisted by the e2e cluster
+	firewallTestHTTPPort = int32(29999)
+	firewallTestUDPPort  = int32(29998)
+)
+
 var _ = SIGDescribe("Firewall rule", func() {
 	var firewall_test_name = "firewall-test"
 	f := framework.NewDefaultFramework(firewall_test_name)
@@ -74,7 +81,7 @@ var _ = SIGDescribe("Firewall rule", func() {
 
 		By("Creating a LoadBalancer type service with ExternalTrafficPolicy=Global")
 		svc := jig.CreateLoadBalancerService(ns, serviceName, framework.LoadBalancerCreateTimeoutDefault, func(svc *v1.Service) {
-			svc.Spec.Ports = []v1.ServicePort{{Protocol: v1.ProtocolTCP, Port: gce.FirewallTestHttpPort}}
+			svc.Spec.Ports = []v1.ServicePort{{Protocol: v1.ProtocolTCP, Port: firewallTestHTTPPort}}
 			svc.Spec.LoadBalancerSourceRanges = firewallTestSourceRanges
 		})
 		defer func() {
@@ -121,7 +128,7 @@ var _ = SIGDescribe("Firewall rule", func() {
 		By(fmt.Sprintf("Creating netexec pods on at most %v nodes", framework.MaxNodesForEndpointsTests))
 		for i, nodeName := range nodesNames {
 			podName := fmt.Sprintf("netexec%v", i)
-			jig.LaunchNetexecPodOnNode(f, nodeName, podName, gce.FirewallTestHttpPort, gce.FirewallTestUdpPort, true)
+			jig.LaunchNetexecPodOnNode(f, nodeName, podName, firewallTestHTTPPort, firewallTestUDPPort, true)
 			defer func() {
 				framework.Logf("Cleaning up the netexec pod: %v", podName)
 				Expect(cs.CoreV1().Pods(ns).Delete(podName, nil)).NotTo(HaveOccurred())
@@ -130,7 +137,7 @@ var _ = SIGDescribe("Firewall rule", func() {
 
 		// Send requests from outside of the cluster because internal traffic is whitelisted
 		By("Accessing the external service ip from outside, all non-master nodes should be reached")
-		Expect(framework.TestHitNodesFromOutside(svcExternalIP, gce.FirewallTestHttpPort, framework.LoadBalancerCreateTimeoutDefault, nodesSet)).NotTo(HaveOccurred())
+		Expect(framework.TestHitNodesFromOutside(svcExternalIP, firewallTestHTTPPort, framework.LoadBalancerCreateTimeoutDefault, nodesSet)).NotTo(HaveOccurred())
 
 		// Check if there are overlapping tags on the firewall that extend beyond just the vms in our cluster
 		// by removing the tag on one vm and make sure it doesn't get any traffic. This is an imperfect
@@ -150,11 +157,11 @@ var _ = SIGDescribe("Firewall rule", func() {
 			nodesSet.Insert(nodesNames[0])
 			gce.SetInstanceTags(cloudConfig, nodesNames[0], zone, removedTags)
 			// Make sure traffic is recovered before exit
-			Expect(framework.TestHitNodesFromOutside(svcExternalIP, gce.FirewallTestHttpPort, framework.LoadBalancerCreateTimeoutDefault, nodesSet)).NotTo(HaveOccurred())
+			Expect(framework.TestHitNodesFromOutside(svcExternalIP, firewallTestHTTPPort, framework.LoadBalancerCreateTimeoutDefault, nodesSet)).NotTo(HaveOccurred())
 		}()
 
 		By("Accessing serivce through the external ip and examine got no response from the node without tags")
-		Expect(framework.TestHitNodesFromOutsideWithCount(svcExternalIP, gce.FirewallTestHttpPort, framework.LoadBalancerCreateTimeoutDefault, nodesSet, 15)).NotTo(HaveOccurred())
+		Expect(framework.TestHitNodesFromOutsideWithCount(svcExternalIP, firewallTestHTTPPort, framework.LoadBalancerCreateTimeoutDefault, nodesSet, 15)).NotTo(HaveOccurred())
 	})
 
 	It("should have correct firewall rules for e2e cluster", func() {
@@ -178,12 +185,12 @@ var _ = SIGDescribe("Firewall rule", func() {
 
 		masterAddresses := framework.GetAllMasterAddresses(cs)
 		for _, masterAddress := range masterAddresses {
-			assertNotReachableHTTPTimeout(masterAddress, ports.InsecureKubeControllerManagerPort, gce.FirewallTestTcpTimeout)
-			assertNotReachableHTTPTimeout(masterAddress, ports.InsecureSchedulerPort, gce.FirewallTestTcpTimeout)
+			assertNotReachableHTTPTimeout(masterAddress, ports.InsecureKubeControllerManagerPort, firewallTestTCPTimeout)
+			assertNotReachableHTTPTimeout(masterAddress, ports.InsecureSchedulerPort, firewallTestTCPTimeout)
 		}
-		assertNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletPort, gce.FirewallTestTcpTimeout)
-		assertNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletReadOnlyPort, gce.FirewallTestTcpTimeout)
-		assertNotReachableHTTPTimeout(nodeAddrs[0], ports.ProxyStatusPort, gce.FirewallTestTcpTimeout)
+		assertNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletPort, firewallTestTCPTimeout)
+		assertNotReachableHTTPTimeout(nodeAddrs[0], ports.KubeletReadOnlyPort, firewallTestTCPTimeout)
+		assertNotReachableHTTPTimeout(nodeAddrs[0], ports.ProxyStatusPort, firewallTestTCPTimeout)
 	})
 })
 

test/e2e/network/ingress.go

@@ -83,13 +83,13 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 	// Slow by design ~10m for each "It" block dominated by loadbalancer setup time
 	// TODO: write similar tests for nginx, haproxy and AWS Ingress.
 	Describe("GCE [Slow] [Feature:Ingress]", func() {
-		var gceController *gce.GCEIngressController
+		var gceController *gce.IngressController
 
 		// Platform specific setup
 		BeforeEach(func() {
 			framework.SkipUnlessProviderIs("gce", "gke")
 			By("Initializing gce controller")
-			gceController = &gce.GCEIngressController{
+			gceController = &gce.IngressController{
 				Ns:     ns,
 				Client: jig.Client,
 				Cloud:  framework.TestContext.CloudConfig,
@@ -111,7 +111,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 			jig.TryDeleteIngress()
 
 			By("Cleaning up cloud resources")
-			Expect(gceController.CleanupGCEIngressController()).NotTo(HaveOccurred())
+			Expect(gceController.CleanupIngressController()).NotTo(HaveOccurred())
 		})
 
 		It("should conform to Ingress spec", func() {
@@ -125,7 +125,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 		})
 
 		It("should create ingress with given static-ip", func() {
-			// ip released when the rest of lb resources are deleted in CleanupGCEIngressController
+			// ip released when the rest of lb resources are deleted in CleanupIngressController
 			ip := gceController.CreateStaticIP(ns)
 			By(fmt.Sprintf("allocated static ip %v: %v through the GCE cloud provider", ns, ip))
 			executeStaticIPHttpsOnlyTest(f, jig, ns, ip)
@@ -401,17 +401,17 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 
 			// Verify that the controller does not create any other resource except instance group.
 			// TODO(59778): Check GCE resources specific to this ingress instead of listing all resources.
-			if len(gceController.ListUrlMaps()) != 0 {
-				framework.Failf("unexpected url maps, expected none, got: %v", gceController.ListUrlMaps())
+			if len(gceController.ListURLMaps()) != 0 {
+				framework.Failf("unexpected url maps, expected none, got: %v", gceController.ListURLMaps())
 			}
 			if len(gceController.ListGlobalForwardingRules()) != 0 {
 				framework.Failf("unexpected forwarding rules, expected none, got: %v", gceController.ListGlobalForwardingRules())
 			}
-			if len(gceController.ListTargetHttpProxies()) != 0 {
-				framework.Failf("unexpected target http proxies, expected none, got: %v", gceController.ListTargetHttpProxies())
+			if len(gceController.ListTargetHTTPProxies()) != 0 {
+				framework.Failf("unexpected target http proxies, expected none, got: %v", gceController.ListTargetHTTPProxies())
 			}
-			if len(gceController.ListTargetHttpsProxies()) != 0 {
-				framework.Failf("unexpected target https proxies, expected none, got: %v", gceController.ListTargetHttpProxies())
+			if len(gceController.ListTargetHTTPSProxies()) != 0 {
+				framework.Failf("unexpected target https proxies, expected none, got: %v", gceController.ListTargetHTTPProxies())
 			}
 			if len(gceController.ListSslCertificates()) != 0 {
 				framework.Failf("unexpected ssl certificates, expected none, got: %v", gceController.ListSslCertificates())
@@ -466,13 +466,13 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 	})
 
 	Describe("GCE [Slow] [Feature:NEG]", func() {
-		var gceController *gce.GCEIngressController
+		var gceController *gce.IngressController
 
 		// Platform specific setup
 		BeforeEach(func() {
 			framework.SkipUnlessProviderIs("gce", "gke")
 			By("Initializing gce controller")
-			gceController = &gce.GCEIngressController{
+			gceController = &gce.IngressController{
 				Ns:     ns,
 				Client: jig.Client,
 				Cloud:  framework.TestContext.CloudConfig,
@@ -494,7 +494,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 			jig.TryDeleteIngress()
 
 			By("Cleaning up cloud resources")
-			Expect(gceController.CleanupGCEIngressController()).NotTo(HaveOccurred())
+			Expect(gceController.CleanupIngressController()).NotTo(HaveOccurred())
 		})
 
 		It("should conform to Ingress spec", func() {
@@ -808,7 +808,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 	})
 
 	Describe("GCE [Slow] [Feature:kubemci]", func() {
-		var gceController *gce.GCEIngressController
+		var gceController *gce.IngressController
 		var ipName, ipAddress string
 
 		// Platform specific setup
@@ -817,7 +817,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 			jig.Class = ingress.MulticlusterIngressClassValue
 			jig.PollInterval = 5 * time.Second
 			By("Initializing gce controller")
-			gceController = &gce.GCEIngressController{
+			gceController = &gce.IngressController{
 				Ns:     ns,
 				Client: jig.Client,
 				Cloud:  framework.TestContext.CloudConfig,
@@ -828,7 +828,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 			// TODO(https://github.com/GoogleCloudPlatform/k8s-multicluster-ingress/issues/19):
 			// Kubemci should reserve a static ip if user has not specified one.
 			ipName = "kubemci-" + string(uuid.NewUUID())
-			// ip released when the rest of lb resources are deleted in CleanupGCEIngressController
+			// ip released when the rest of lb resources are deleted in CleanupIngressController
 			ipAddress = gceController.CreateStaticIP(ipName)
 			By(fmt.Sprintf("allocated static ip %v: %v through the GCE cloud provider", ipName, ipAddress))
 		})
@@ -846,7 +846,7 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 			}
 
 			By("Cleaning up cloud resources")
-			Expect(gceController.CleanupGCEIngressController()).NotTo(HaveOccurred())
+			Expect(gceController.CleanupIngressController()).NotTo(HaveOccurred())
 		})
 
 		It("should conform to Ingress spec", func() {
@@ -1118,7 +1118,7 @@ func detectHttpVersionAndSchemeTest(f *framework.Framework, jig *ingress.TestJig
 	Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("Failed to get %s or %s, response body: %s", version, scheme, resp))
 }
 
-func detectNegAnnotation(f *framework.Framework, jig *ingress.TestJig, gceController *gce.GCEIngressController, ns, name string, negs int) {
+func detectNegAnnotation(f *framework.Framework, jig *ingress.TestJig, gceController *gce.IngressController, ns, name string, negs int) {
 	if err := wait.Poll(5*time.Second, negUpdateTimeout, func() (bool, error) {
 		svc, err := f.ClientSet.CoreV1().Services(ns).Get(name, metav1.GetOptions{})
 		if err != nil {

test/e2e/network/scale/ingress.go

@@ -63,7 +63,7 @@ var (
 type IngressScaleFramework struct {
 	Clientset     clientset.Interface
 	Jig           *ingress.TestJig
-	GCEController *gce.GCEIngressController
+	GCEController *gce.IngressController
 	CloudConfig   framework.CloudConfig
 	Logger        ingress.TestLogger
 
@@ -112,7 +112,7 @@ func (f *IngressScaleFramework) PrepareScaleTest() error {
 	f.Jig = ingress.NewIngressTestJig(f.Clientset)
 	f.Jig.Logger = f.Logger
 	f.Jig.PollInterval = scaleTestPollInterval
-	f.GCEController = &gce.GCEIngressController{
+	f.GCEController = &gce.IngressController{
 		Client: f.Clientset,
 		Cloud:  f.CloudConfig,
 	}
@@ -154,7 +154,7 @@ func (f *IngressScaleFramework) CleanupScaleTest() []error {
 	}
 
 	f.Logger.Infof("Cleaning up cloud resources...")
-	if err := f.GCEController.CleanupGCEIngressControllerWithTimeout(ingressesCleanupTimeout); err != nil {
+	if err := f.GCEController.CleanupIngressControllerWithTimeout(ingressesCleanupTimeout); err != nil {
 		errs = append(errs, err)
 	}