mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-24 20:24:09 +00:00
Merge pull request #49167 from nicksardo/gce-static-unit-test
Automatic merge from submit-queue (batch tested with PRs 45813, 49594, 49443, 49167, 47539) GCE: Adding unit test for ensureStaticIP **What this PR does / why we need it**: Entry into unit testing GCE loadbalancer code by testing `ensureStaticIP` which had a bug in 1.7.0. @bowei @freehan @MrHohn @dnardo @thockin, any thoughts and comments on how we could unit test LB code moving forward? I think there are many areas we can split functions into smaller ones for easier testing - firewallNeedsUpdate being an example of that. However, it seems to me that we still need to mock our GCP calls for some functions that heavily revolve around API calls. A dream goal would be to have a unit test that can call EnsureLoadBalancer. Now that we have shared resources between different services and ingresses (firewalls, instance groups, [future features]), being able to setup different scenarios without depending on E2E tests would be awesome. However, I'm not sure how reachable that goal would be. Most importantly, let's not make things worse. If you have advice on anti-patterns to avoid, please speak up. ```release-note NONE ```
This commit is contained in:
commit
ceedc7813c
@ -14,6 +14,7 @@ go_library(
|
||||
"doc.go",
|
||||
"gce.go",
|
||||
"gce_addresses.go",
|
||||
"gce_addresses_fakes.go",
|
||||
"gce_annotations.go",
|
||||
"gce_backendservice.go",
|
||||
"gce_cert.go",
|
||||
@ -25,6 +26,7 @@ go_library(
|
||||
"gce_healthchecks.go",
|
||||
"gce_instancegroup.go",
|
||||
"gce_instances.go",
|
||||
"gce_interfaces.go",
|
||||
"gce_loadbalancer.go",
|
||||
"gce_loadbalancer_external.go",
|
||||
"gce_loadbalancer_internal.go",
|
||||
@ -81,6 +83,7 @@ go_test(
|
||||
srcs = [
|
||||
"gce_disks_test.go",
|
||||
"gce_healthchecks_test.go",
|
||||
"gce_loadbalancer_external_test.go",
|
||||
"gce_test.go",
|
||||
],
|
||||
library = ":go_default_library",
|
||||
|
77
pkg/cloudprovider/providers/gce/gce_addresses_fakes.go
Normal file
77
pkg/cloudprovider/providers/gce/gce_addresses_fakes.go
Normal file
@ -0,0 +1,77 @@
|
||||
/*
|
||||
Copyright 2017 The Kubernetes Authors.
|
||||
|
||||
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 gce
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
compute "google.golang.org/api/compute/v1"
|
||||
"google.golang.org/api/googleapi"
|
||||
)
|
||||
|
||||
type FakeCloudAddressService struct {
|
||||
count int
|
||||
// reservedAddrs tracks usage of IP addresses
|
||||
// Key is the IP address as a string
|
||||
reservedAddrs map[string]bool
|
||||
// addrsByRegionAndName
|
||||
// Outer key is for region string; inner key is for address name.
|
||||
addrsByRegionAndName map[string]map[string]*compute.Address
|
||||
}
|
||||
|
||||
func NewFakeCloudAddressService() *FakeCloudAddressService {
|
||||
return &FakeCloudAddressService{
|
||||
reservedAddrs: make(map[string]bool),
|
||||
addrsByRegionAndName: make(map[string]map[string]*compute.Address),
|
||||
}
|
||||
}
|
||||
|
||||
func (cas *FakeCloudAddressService) ReserveRegionAddress(addr *compute.Address, region string) error {
|
||||
if addr.Address == "" {
|
||||
addr.Address = fmt.Sprintf("1.2.3.%d", cas.count)
|
||||
cas.count++
|
||||
}
|
||||
|
||||
if cas.reservedAddrs[addr.Address] {
|
||||
return &googleapi.Error{Code: http.StatusConflict}
|
||||
}
|
||||
|
||||
if _, exists := cas.addrsByRegionAndName[region]; !exists {
|
||||
cas.addrsByRegionAndName[region] = make(map[string]*compute.Address)
|
||||
}
|
||||
|
||||
if _, exists := cas.addrsByRegionAndName[region][addr.Name]; exists {
|
||||
return &googleapi.Error{Code: http.StatusConflict}
|
||||
}
|
||||
|
||||
cas.addrsByRegionAndName[region][addr.Name] = addr
|
||||
cas.reservedAddrs[addr.Address] = true
|
||||
return nil
|
||||
}
|
||||
|
||||
func (cas *FakeCloudAddressService) GetRegionAddress(name, region string) (*compute.Address, error) {
|
||||
if _, exists := cas.addrsByRegionAndName[region]; !exists {
|
||||
return nil, &googleapi.Error{Code: http.StatusNotFound}
|
||||
}
|
||||
|
||||
if addr, exists := cas.addrsByRegionAndName[region][name]; !exists {
|
||||
return nil, &googleapi.Error{Code: http.StatusNotFound}
|
||||
} else {
|
||||
return addr, nil
|
||||
}
|
||||
}
|
27
pkg/cloudprovider/providers/gce/gce_interfaces.go
Normal file
27
pkg/cloudprovider/providers/gce/gce_interfaces.go
Normal file
@ -0,0 +1,27 @@
|
||||
/*
|
||||
Copyright 2017 The Kubernetes Authors.
|
||||
|
||||
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 gce
|
||||
|
||||
import compute "google.golang.org/api/compute/v1"
|
||||
|
||||
// CloudAddressService is an interface for managing addresses
|
||||
type CloudAddressService interface {
|
||||
ReserveRegionAddress(*compute.Address, string) error
|
||||
GetRegionAddress(string, string) (*compute.Address, error)
|
||||
// TODO: Mock `DeleteRegionAddress(name, region string) endpoint
|
||||
// TODO: Mock Global endpoints
|
||||
}
|
@ -140,7 +140,7 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, a
|
||||
// to this forwarding rule, so we can keep it.
|
||||
isUserOwnedIP = false
|
||||
isSafeToReleaseIP = true
|
||||
ipAddress, _, err = gce.ensureStaticIP(loadBalancerName, serviceName.String(), gce.region, fwdRuleIP)
|
||||
ipAddress, _, err = ensureStaticIP(gce, loadBalancerName, serviceName.String(), gce.region, fwdRuleIP)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to ensure static IP %s: %v", fwdRuleIP, err)
|
||||
}
|
||||
@ -161,7 +161,7 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, a
|
||||
// IP from ephemeral to static, or it will just get the IP if it is
|
||||
// already static.
|
||||
existed := false
|
||||
ipAddress, existed, err = gce.ensureStaticIP(loadBalancerName, serviceName.String(), gce.region, fwdRuleIP)
|
||||
ipAddress, existed, err = ensureStaticIP(gce, loadBalancerName, serviceName.String(), gce.region, fwdRuleIP)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to ensure static IP %s: %v", fwdRuleIP, err)
|
||||
}
|
||||
@ -906,7 +906,7 @@ func (gce *GCECloud) projectOwnsStaticIP(name, region string, ipAddress string)
|
||||
return false, nil
|
||||
}
|
||||
|
||||
func (gce *GCECloud) ensureStaticIP(name, serviceName, region, existingIP string) (ipAddress string, created bool, err error) {
|
||||
func ensureStaticIP(s CloudAddressService, name, serviceName, region, existingIP string) (ipAddress string, existing bool, err error) {
|
||||
// If the address doesn't exist, this will create it.
|
||||
// If the existingIP exists but is ephemeral, this will promote it to static.
|
||||
// If the address already exists, this will harmlessly return a StatusConflict
|
||||
@ -921,7 +921,7 @@ func (gce *GCECloud) ensureStaticIP(name, serviceName, region, existingIP string
|
||||
addressObj.Address = existingIP
|
||||
}
|
||||
|
||||
if err = gce.ReserveRegionAddress(addressObj, region); err != nil {
|
||||
if err = s.ReserveRegionAddress(addressObj, region); err != nil {
|
||||
if !isHTTPErrorCode(err, http.StatusConflict) {
|
||||
return "", false, fmt.Errorf("error creating gce static IP address: %v", err)
|
||||
}
|
||||
@ -929,7 +929,7 @@ func (gce *GCECloud) ensureStaticIP(name, serviceName, region, existingIP string
|
||||
existed = true
|
||||
}
|
||||
|
||||
addr, err := gce.GetRegionAddress(name, region)
|
||||
addr, err := s.GetRegionAddress(name, region)
|
||||
if err != nil {
|
||||
return "", false, fmt.Errorf("error getting static IP address: %v", err)
|
||||
}
|
||||
|
@ -0,0 +1,39 @@
|
||||
/*
|
||||
Copyright 2017 The Kubernetes Authors.
|
||||
|
||||
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 gce
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestEnsureStaticIP(t *testing.T) {
|
||||
fcas := NewFakeCloudAddressService()
|
||||
ipName := "some-static-ip"
|
||||
serviceName := ""
|
||||
region := "us-central1"
|
||||
|
||||
// First ensure call
|
||||
ip, existed, err := ensureStaticIP(fcas, ipName, serviceName, region, "")
|
||||
if err != nil || existed || ip == "" {
|
||||
t.Fatalf(`ensureStaticIP(%v, %v, %v, %v, "") = %v, %v, %v; want valid ip, false, nil`, fcas, ipName, serviceName, region, ip, existed, err)
|
||||
}
|
||||
|
||||
// Second ensure call
|
||||
var ipPrime string
|
||||
ipPrime, existed, err = ensureStaticIP(fcas, ipName, serviceName, region, ip)
|
||||
if err != nil || !existed || ip != ipPrime {
|
||||
t.Fatalf(`ensureStaticIP(%v, %v, %v, %v, %v) = %v, %v, %v; want %v, true, nil`, fcas, ipName, serviceName, region, ip, ipPrime, existed, err, ip)
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user