diff --git a/cmd/glbc/main.go b/cmd/glbc/main.go index ff1877770b..9186489714 100644 --- a/cmd/glbc/main.go +++ b/cmd/glbc/main.go @@ -26,7 +26,7 @@ import ( flag "github.com/spf13/pflag" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/ingress-gce/pkg/frontendconfig" - "k8s.io/ingress-gce/pkg/healthchecks" + "k8s.io/ingress-gce/pkg/healthchecksl4" "k8s.io/ingress-gce/pkg/ingparams" "k8s.io/ingress-gce/pkg/l4lb" "k8s.io/ingress-gce/pkg/psc" @@ -279,7 +279,7 @@ func runControllers(ctx *ingctx.ControllerContext) { fwc := firewalls.NewFirewallController(ctx, flags.F.NodePortRanges.Values()) - healthchecks.InitializeL4(ctx.Cloud, ctx) + healthchecksl4.Initialize(ctx.Cloud, ctx) if flags.F.RunL4Controller { l4Controller := l4lb.NewILBController(ctx, stopCh) diff --git a/pkg/healthchecks/interfaces.go b/pkg/healthchecks/interfaces.go index 0b9956be13..60cb8d9f79 100644 --- a/pkg/healthchecks/interfaces.go +++ b/pkg/healthchecks/interfaces.go @@ -17,15 +17,13 @@ limitations under the License. package healthchecks import ( + "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" computealpha "google.golang.org/api/compute/v0.alpha" computebeta "google.golang.org/api/compute/v0.beta" - compute "google.golang.org/api/compute/v1" + "google.golang.org/api/compute/v1" v1 "k8s.io/api/core/v1" "k8s.io/ingress-gce/pkg/translator" "k8s.io/ingress-gce/pkg/utils" - "k8s.io/ingress-gce/pkg/utils/namer" - - "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" ) // HealthCheckProvider is an interface to manage a single GCE health check. @@ -58,19 +56,3 @@ type HealthChecker interface { Delete(name string, scope meta.KeyType) error Get(name string, version meta.Version, scope meta.KeyType) (*translator.HealthCheck, error) } - -// L4HealthChecks defines methods for creating and deleting health checks (and their firewall rules) for l4 services -type L4HealthChecks interface { - // EnsureL4HealthCheck creates health check (and firewall rule) for l4 service - EnsureL4HealthCheck(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) *EnsureL4HealthCheckResult - // DeleteHealthCheck deletes health check (and firewall rule) for l4 service - DeleteHealthCheck(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) -} - -type EnsureL4HealthCheckResult struct { - HCName string - HCLink string - HCFirewallRuleName string - GceResourceInError string - Err error -} diff --git a/pkg/healthchecks/healthchecks_l4.go b/pkg/healthchecksl4/healthchecksl4.go similarity index 84% rename from pkg/healthchecks/healthchecks_l4.go rename to pkg/healthchecksl4/healthchecksl4.go index 19cb49d770..7df47cc70c 100644 --- a/pkg/healthchecks/healthchecks_l4.go +++ b/pkg/healthchecksl4/healthchecksl4.go @@ -14,14 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -package healthchecks +package healthchecksl4 import ( "fmt" "strconv" "sync" - cloudprovider "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -30,6 +29,7 @@ import ( "k8s.io/ingress-gce/pkg/composite" "k8s.io/ingress-gce/pkg/events" "k8s.io/ingress-gce/pkg/firewalls" + "k8s.io/ingress-gce/pkg/healthchecksprovider" "k8s.io/ingress-gce/pkg/utils" "k8s.io/ingress-gce/pkg/utils/namer" "k8s.io/klog/v2" @@ -49,7 +49,7 @@ const ( var ( // instanceLock to prevent duplicate initialization. instanceLock = &sync.Mutex{} - // instance is a singleton instance, created by InitializeL4 + // instance is a singleton instance, created by Initialize instance *l4HealthChecks ) @@ -57,12 +57,13 @@ type l4HealthChecks struct { // sharedResourceLock serializes operations on the healthcheck and firewall // resources shared across multiple Services. sharedResourcesLock sync.Mutex + hcProvider healthChecksProvider cloud *gce.Cloud recorderFactory events.RecorderProducer } -// InitializeL4 creates singleton instance, must be run before L4() func -func InitializeL4(cloud *gce.Cloud, recorderFactory events.RecorderProducer) { +// Initialize creates singleton instance, must be run before GetInstance() func +func Initialize(cloud *gce.Cloud, recorderFactory events.RecorderProducer) { instanceLock.Lock() defer instanceLock.Unlock() @@ -74,25 +75,27 @@ func InitializeL4(cloud *gce.Cloud, recorderFactory events.RecorderProducer) { instance = &l4HealthChecks{ cloud: cloud, recorderFactory: recorderFactory, + hcProvider: healthchecksprovider.NewHealthChecks(cloud, meta.VersionGA), } klog.V(3).Infof("Initialized L4 Healthchecks") } -// FakeL4 creates instance of l4HealthChecks. Use for test only. -func FakeL4(cloud *gce.Cloud, recorderFactory events.RecorderProducer) *l4HealthChecks { +// Fake creates instance of l4HealthChecks. Use for test only. +func Fake(cloud *gce.Cloud, recorderFactory events.RecorderProducer) *l4HealthChecks { instance = &l4HealthChecks{ cloud: cloud, recorderFactory: recorderFactory, + hcProvider: healthchecksprovider.NewHealthChecks(cloud, meta.VersionGA), } return instance } -// L4 returns singleton instance, must be run after InitializeL4 -func L4() *l4HealthChecks { +// GetInstance returns singleton instance, must be run after Initialize +func GetInstance() *l4HealthChecks { return instance } -// EnsureL4HealthCheck and firewall rules exist for the L4 +// EnsureHealthCheck and firewall rules exist for the L4 // LoadBalancer Service. // // The healthcheck and firewall will be shared between different K8s @@ -102,8 +105,7 @@ func L4() *l4HealthChecks { // Firewall rules are always created at in the Global scope (vs // Regional). This means that one Firewall rule is created for // Services of different scope (Global vs Regional). - -func (l4hc *l4HealthChecks) EnsureL4HealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) *EnsureL4HealthCheckResult { +func (l4hc *l4HealthChecks) EnsureHealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) *EnsureL4HealthCheckResult { namespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} hcName := namer.L4HealthCheck(svc.Namespace, svc.Name, sharedHC) @@ -154,7 +156,7 @@ func (l4hc *l4HealthChecks) DeleteHealthCheck(svc *corev1.Service, namer namer.L defer l4hc.sharedResourcesLock.Unlock() } - err := utils.IgnoreHTTPNotFound(l4hc.deleteHealthCheck(hcName, scope)) + err := l4hc.hcProvider.Delete(hcName, scope) if err != nil { // Ignore deletion error due to health check in use by another resource. if !utils.IsInUsedByError(err) { @@ -169,17 +171,11 @@ func (l4hc *l4HealthChecks) DeleteHealthCheck(svc *corev1.Service, namer namer.L } func (l4hc *l4HealthChecks) ensureL4HealthCheckInternal(hcName string, svcName types.NamespacedName, shared bool, path string, port int32, scope meta.KeyType, l4Type utils.L4LBType) (*composite.HealthCheck, string, error) { - selfLink := "" - key, err := composite.CreateKey(l4hc.cloud, hcName, scope) - if err != nil { - return nil, selfLink, fmt.Errorf("Failed to create key for healthcheck with name %s for service %s", hcName, svcName.String()) - } - hc, err := composite.GetHealthCheck(l4hc.cloud, key, meta.VersionGA) + hc, err := l4hc.hcProvider.Get(hcName, scope) if err != nil { - if !utils.IsNotFoundError(err) { - return nil, selfLink, err - } + return nil, "", err } + var region string if scope == meta.Regional { region = l4hc.cloud.Region() @@ -189,14 +185,17 @@ func (l4hc *l4HealthChecks) ensureL4HealthCheckInternal(hcName string, svcName t if hc == nil { // Create the healthcheck klog.V(2).Infof("Creating healthcheck %s for service %s, shared = %v. Expected healthcheck: %v", hcName, svcName, shared, expectedHC) - err = composite.CreateHealthCheck(l4hc.cloud, key, expectedHC) + err = l4hc.hcProvider.Create(expectedHC) + if err != nil { + return nil, "", err + } + selfLink, err := l4hc.hcProvider.SelfLink(expectedHC.Name, scope) if err != nil { - return nil, selfLink, err + return nil, "", err } - selfLink = cloudprovider.SelfLink(meta.VersionGA, l4hc.cloud.ProjectID(), "healthChecks", key) return expectedHC, selfLink, nil } - selfLink = hc.SelfLink + selfLink := hc.SelfLink if !needToUpdateHealthChecks(hc, expectedHC) { // nothing to do klog.V(3).Infof("Healthcheck %v already exists", hcName) @@ -204,7 +203,7 @@ func (l4hc *l4HealthChecks) ensureL4HealthCheckInternal(hcName string, svcName t } mergeHealthChecks(hc, expectedHC) klog.V(2).Infof("Updating healthcheck %s for service %s, updated healthcheck: %v", hcName, svcName, expectedHC) - err = composite.UpdateHealthCheck(l4hc.cloud, key, expectedHC) + err = l4hc.hcProvider.Update(expectedHC.Name, scope, expectedHC) if err != nil { return nil, selfLink, err } @@ -227,14 +226,6 @@ func (l4hc *l4HealthChecks) ensureFirewall(svc *corev1.Service, hcFwName string, return firewalls.EnsureL4LBFirewallForHc(svc, sharedHC, &hcFWRParams, l4hc.cloud, l4hc.recorderFactory.Recorder(svc.Namespace)) } -func (l4hc *l4HealthChecks) deleteHealthCheck(name string, scope meta.KeyType) error { - key, err := composite.CreateKey(l4hc.cloud, name, scope) - if err != nil { - return fmt.Errorf("Failed to create composite key for healthcheck %s - %w", name, err) - } - return composite.DeleteHealthCheck(l4hc.cloud, key, meta.VersionGA) -} - func (l4hc *l4HealthChecks) deleteHealthCheckFirewall(svc *corev1.Service, hcName, hcFwName string, sharedHC bool, l4Type utils.L4LBType) (string, error) { namespacedName := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace} @@ -266,12 +257,12 @@ func (l4hc *l4HealthChecks) healthCheckFirewallSafeToDelete(hcName string, share if l4Type == utils.XLB { scopeToCheck = meta.Global } - key, err := composite.CreateKey(l4hc.cloud, hcName, scopeToCheck) + + hc, err := l4hc.hcProvider.Get(hcName, scopeToCheck) if err != nil { - return false, fmt.Errorf("Failed to create composite key for healthcheck %s - %w", hcName, err) + return false, fmt.Errorf("l4hc.hcProvider.Get(%s, %s) returned error %w, want nil", hcName, scopeToCheck, err) } - _, err = composite.GetHealthCheck(l4hc.cloud, key, meta.VersionGA) - return utils.IsNotFoundError(err), nil + return hc == nil, nil } func (l4hc *l4HealthChecks) deleteFirewall(name string, svc *corev1.Service) error { diff --git a/pkg/healthchecks/healthchecks_l4_test.go b/pkg/healthchecksl4/healthchecksl4_test.go similarity index 99% rename from pkg/healthchecks/healthchecks_l4_test.go rename to pkg/healthchecksl4/healthchecksl4_test.go index f0fccb53e1..448736c8cc 100644 --- a/pkg/healthchecks/healthchecks_l4_test.go +++ b/pkg/healthchecksl4/healthchecksl4_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package healthchecks +package healthchecksl4 import ( "testing" @@ -109,7 +109,7 @@ func TestCompareHealthChecks(t *testing.T) { } } -func TestCreateHealthCheck(t *testing.T) { +func TestNewHealthCheck(t *testing.T) { t.Parallel() namespaceName := types.NamespacedName{Name: "svc", Namespace: "default"} diff --git a/pkg/healthchecksl4/interfaces.go b/pkg/healthchecksl4/interfaces.go new file mode 100644 index 0000000000..a2380480d3 --- /dev/null +++ b/pkg/healthchecksl4/interfaces.go @@ -0,0 +1,33 @@ +package healthchecksl4 + +import ( + "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" + v1 "k8s.io/api/core/v1" + "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/utils" + "k8s.io/ingress-gce/pkg/utils/namer" +) + +// L4HealthChecks defines methods for creating and deleting health checks (and their firewall rules) for l4 services +type L4HealthChecks interface { + // EnsureHealthCheck creates health check (and firewall rule) for l4 service + EnsureHealthCheck(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) *EnsureL4HealthCheckResult + // DeleteHealthCheck deletes health check (and firewall rule) for l4 service + DeleteHealthCheck(svc *v1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, error) +} + +type EnsureL4HealthCheckResult struct { + HCName string + HCLink string + HCFirewallRuleName string + GceResourceInError string + Err error +} + +type healthChecksProvider interface { + Get(name string, scope meta.KeyType) (*composite.HealthCheck, error) + Create(healthCheck *composite.HealthCheck) error + Update(name string, scope meta.KeyType, updatedHealthCheck *composite.HealthCheck) error + Delete(name string, scope meta.KeyType) error + SelfLink(name string, scope meta.KeyType) (string, error) +} diff --git a/pkg/healthchecksprovider/healthchecksprovider.go b/pkg/healthchecksprovider/healthchecksprovider.go new file mode 100644 index 0000000000..de96c0b1fe --- /dev/null +++ b/pkg/healthchecksprovider/healthchecksprovider.go @@ -0,0 +1,86 @@ +package healthchecksprovider + +import ( + "fmt" + + cloudprovider "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" + "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" + "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/utils" + "k8s.io/legacy-cloud-providers/gce" +) + +type HealthChecks struct { + cloud *gce.Cloud + version meta.Version +} + +func NewHealthChecks(cloud *gce.Cloud, version meta.Version) *HealthChecks { + return &HealthChecks{ + cloud: cloud, + version: version, + } +} + +func (hc *HealthChecks) Get(name string, scope meta.KeyType) (*composite.HealthCheck, error) { + key, err := hc.createKey(name, scope) + if err != nil { + return nil, fmt.Errorf("hc.createKey(%s, %s) returned error %w, want nil", name, scope, err) + } + healthCheck, err := composite.GetHealthCheck(hc.cloud, key, hc.version) + if err != nil { + if utils.IsNotFoundError(err) { + return nil, nil + } + return nil, fmt.Errorf("composite.GetHealthCheck(_, %v, %v) returned error %w, want nil", key, meta.VersionGA, err) + } + return healthCheck, nil +} + +func (hc *HealthChecks) Create(healthCheck *composite.HealthCheck) error { + key, err := hc.createKey(healthCheck.Name, healthCheck.Scope) + if err != nil { + return fmt.Errorf("hc.createKey(%s, %s) returned error: %w, want nil", healthCheck.Name, healthCheck.Scope, err) + } + + err = composite.CreateHealthCheck(hc.cloud, key, healthCheck) + if err != nil { + return fmt.Errorf("composite.CreateHealthCheck(_, %s, %v) returned error %w, want nil", key, healthCheck, err) + } + return nil +} + +func (hc *HealthChecks) Update(name string, scope meta.KeyType, updatedHealthCheck *composite.HealthCheck) error { + key, err := hc.createKey(name, scope) + if err != nil { + return fmt.Errorf("hc.createKey(%s, %s) returned error: %w, want nil", name, scope, err) + } + + err = composite.UpdateHealthCheck(hc.cloud, key, updatedHealthCheck) + if err != nil { + return fmt.Errorf("composite.UpdateHealthCheck(_, %s, %v) returned error %w, want nil", key, updatedHealthCheck, err) + } + return nil +} + +func (hc *HealthChecks) Delete(name string, scope meta.KeyType) error { + key, err := hc.createKey(name, scope) + if err != nil { + return fmt.Errorf("hc.createKey(%s, %s) returned error %w, want nil", name, scope, err) + } + + return utils.IgnoreHTTPNotFound(composite.DeleteHealthCheck(hc.cloud, key, hc.version)) +} + +func (hc *HealthChecks) SelfLink(name string, scope meta.KeyType) (string, error) { + key, err := hc.createKey(name, scope) + if err != nil { + return "", fmt.Errorf("hc.createKey(%s, %s) returned error %w, want nil", name, scope, err) + } + + return cloudprovider.SelfLink(meta.VersionGA, hc.cloud.ProjectID(), "healthChecks", key), nil +} + +func (hc *HealthChecks) createKey(name string, scope meta.KeyType) (*meta.Key, error) { + return composite.CreateKey(hc.cloud, name, scope) +} diff --git a/pkg/healthchecksprovider/healthchecksprovider_test.go b/pkg/healthchecksprovider/healthchecksprovider_test.go new file mode 100644 index 0000000000..48dd4b5399 --- /dev/null +++ b/pkg/healthchecksprovider/healthchecksprovider_test.go @@ -0,0 +1,267 @@ +package healthchecksprovider + +import ( + "fmt" + "testing" + + "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/utils" + "k8s.io/legacy-cloud-providers/gce" +) + +func TestCreateHealthCheck(t *testing.T) { + testCases := []struct { + healthCheck *composite.HealthCheck + desc string + }{ + { + desc: "Create regional health check", + healthCheck: &composite.HealthCheck{ + Name: "regional-hc", + Scope: meta.Regional, + }, + }, + { + desc: "Create global health check", + healthCheck: &composite.HealthCheck{ + Name: "global-hc", + Scope: meta.Global, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) + hc := NewHealthChecks(fakeGCE, meta.VersionGA) + + err := hc.Create(tc.healthCheck) + if err != nil { + t.Fatalf("hc.Create(%v), returned error %v, want nil", tc.healthCheck, err) + } + + err = verifyHealthCheckExists(fakeGCE, tc.healthCheck.Name, tc.healthCheck.Scope) + if err != nil { + t.Errorf("verifyHealthCheckExists(_, %s, %s) returned error %v, want nil", tc.healthCheck.Name, tc.healthCheck.Scope, err) + } + }) + } +} + +func TestGetHealthCheck(t *testing.T) { + regionalHealthCheck := &composite.HealthCheck{ + Name: "regional-hc", + Version: meta.VersionGA, + Scope: meta.Regional, + } + globalHealthCheck := &composite.HealthCheck{ + Name: "global-hc", + Version: meta.VersionGA, + Scope: meta.Global, + } + + testCases := []struct { + existingHealthChecks []*composite.HealthCheck + getHCName string + getHCScope meta.KeyType + expectedHealthCheck *composite.HealthCheck + desc string + }{ + { + desc: "Get regional health check", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + getHCName: regionalHealthCheck.Name, + getHCScope: regionalHealthCheck.Scope, + expectedHealthCheck: regionalHealthCheck, + }, + { + desc: "Get global health check", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + getHCName: globalHealthCheck.Name, + getHCScope: globalHealthCheck.Scope, + expectedHealthCheck: globalHealthCheck, + }, + { + desc: "Get non existent global health check", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + getHCName: "non-existent-hc", + getHCScope: meta.Global, + expectedHealthCheck: nil, + }, + { + desc: "Get non existent regional health check", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + getHCName: "non-existent-hc", + getHCScope: meta.Regional, + expectedHealthCheck: nil, + }, + { + desc: "Get existent regional health check, but providing global scope", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + getHCName: regionalHealthCheck.Name, + getHCScope: meta.Global, + expectedHealthCheck: nil, + }, + { + desc: "Get existent global health check, but providing regional scope", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + getHCName: globalHealthCheck.Name, + getHCScope: meta.Regional, + expectedHealthCheck: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) + mustCreateHealthChecks(t, fakeGCE, tc.existingHealthChecks) + hcp := NewHealthChecks(fakeGCE, meta.VersionGA) + + hc, err := hcp.Get(tc.getHCName, tc.getHCScope) + if err != nil { + t.Fatalf("hcp.Get(%v), returned error %v, want nil", tc.getHCName, err) + } + + // Scope field gets removed (but region added), after creating health check + ignoreFields := cmpopts.IgnoreFields(composite.HealthCheck{}, "SelfLink", "Region", "Scope") + if !cmp.Equal(hc, tc.expectedHealthCheck, ignoreFields) { + diff := cmp.Diff(hc, tc.expectedHealthCheck, ignoreFields) + t.Errorf("hcp.Get(s) returned %v, not equal to expectedHealthCheck %v, diff: %v", hc, tc.expectedHealthCheck, diff) + } + }) + } +} + +func TestDeleteHealthCheck(t *testing.T) { + regionalHealthCheck := &composite.HealthCheck{ + Name: "regional-hc", + Version: meta.VersionGA, + Scope: meta.Regional, + } + globalHealthCheck := &composite.HealthCheck{ + Name: "global-hc", + Version: meta.VersionGA, + Scope: meta.Global, + } + + testCases := []struct { + existingHealthChecks []*composite.HealthCheck + deleteHCName string + deleteHCScope meta.KeyType + shouldNotDeleteHealthChecks []*composite.HealthCheck + desc string + }{ + { + desc: "Delete regional health check", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + deleteHCName: regionalHealthCheck.Name, + deleteHCScope: regionalHealthCheck.Scope, + shouldNotDeleteHealthChecks: []*composite.HealthCheck{globalHealthCheck}, + }, + { + desc: "Delete global health check", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + deleteHCName: globalHealthCheck.Name, + deleteHCScope: globalHealthCheck.Scope, + shouldNotDeleteHealthChecks: []*composite.HealthCheck{regionalHealthCheck}, + }, + { + desc: "Delete non existent healthCheck", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + deleteHCName: "non-existent", + deleteHCScope: meta.Regional, + shouldNotDeleteHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + }, + { + desc: "Delete global health check name, but using regional scope", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + deleteHCName: globalHealthCheck.Name, + deleteHCScope: meta.Regional, + shouldNotDeleteHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + }, + { + desc: "Delete regional health check name, but using global scope", + existingHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + deleteHCName: regionalHealthCheck.Name, + deleteHCScope: meta.Global, + shouldNotDeleteHealthChecks: []*composite.HealthCheck{regionalHealthCheck, globalHealthCheck}, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) + mustCreateHealthChecks(t, fakeGCE, tc.existingHealthChecks) + hc := NewHealthChecks(fakeGCE, meta.VersionGA) + + err := hc.Delete(tc.deleteHCName, tc.deleteHCScope) + if err != nil { + t.Fatalf("hc.Delete(%v), returned error %v, want nil", tc.deleteHCName, err) + } + + err = verifyHealthCheckNotExists(fakeGCE, tc.deleteHCName, tc.deleteHCScope) + if err != nil { + t.Errorf("verifyHealthCheckNotExists(_, %s, %s) returned error %v", tc.deleteHCName, tc.deleteHCScope, err) + } + for _, hc := range tc.shouldNotDeleteHealthChecks { + err = verifyHealthCheckExists(fakeGCE, hc.Name, hc.Scope) + if err != nil { + t.Errorf("verifyHealthCheckExists(_, %s, %s) returned error %v", hc.Name, hc.Scope, err) + } + } + }) + } +} + +func verifyHealthCheckExists(cloud *gce.Cloud, name string, scope meta.KeyType) error { + return verifyHealthCheckShouldExist(cloud, name, scope, true) +} + +func verifyHealthCheckNotExists(cloud *gce.Cloud, name string, scope meta.KeyType) error { + return verifyHealthCheckShouldExist(cloud, name, scope, false) +} + +func verifyHealthCheckShouldExist(cloud *gce.Cloud, name string, scope meta.KeyType, shouldExist bool) error { + key, err := composite.CreateKey(cloud, name, scope) + if err != nil { + return fmt.Errorf("hailed to create key for fetching health check %s, err: %w", name, err) + } + _, err = composite.GetHealthCheck(cloud, key, meta.VersionGA) + if err != nil { + if utils.IsNotFoundError(err) { + if shouldExist { + return fmt.Errorf("health check %s in scope %s was not found", name, scope) + } + return nil + } + return fmt.Errorf("composite.GetHealthCheck(_, %v, %v) returned error %w, want nil", key, meta.VersionGA, err) + } + if !shouldExist { + return fmt.Errorf("health Check %s in scope %s exists, expected to be not found", name, scope) + } + return nil +} + +func mustCreateHealthChecks(t *testing.T, cloud *gce.Cloud, hcs []*composite.HealthCheck) { + t.Helper() + + for _, hc := range hcs { + mustCreateHealthCheck(t, cloud, hc) + } +} + +func mustCreateHealthCheck(t *testing.T, cloud *gce.Cloud, hc *composite.HealthCheck) { + t.Helper() + + key, err := composite.CreateKey(cloud, hc.Name, hc.Scope) + if err != nil { + t.Fatalf("composite.CreateKey(_, %s, %s) returned error %v, want nil", hc.Name, hc.Scope, err) + } + err = composite.CreateHealthCheck(cloud, key, hc) + if err != nil { + t.Fatalf("composite.CreateHealthCheck(_, %s, %v) returned error %v, want nil", key, hc, err) + } +} diff --git a/pkg/l4lb/l4controller_test.go b/pkg/l4lb/l4controller_test.go index 377d8f3d5b..5cd19617a3 100644 --- a/pkg/l4lb/l4controller_test.go +++ b/pkg/l4lb/l4controller_test.go @@ -23,8 +23,7 @@ import ( "testing" "time" - "k8s.io/ingress-gce/pkg/healthchecks" - + "k8s.io/ingress-gce/pkg/healthchecksl4" "k8s.io/ingress-gce/pkg/loadbalancers" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" @@ -71,7 +70,7 @@ func newServiceController(t *testing.T, fakeGCE *gce.Cloud) *L4Controller { for _, n := range nodes { ctx.NodeInformer.GetIndexer().Add(n) } - healthchecks.FakeL4(ctx.Cloud, ctx) + healthchecksl4.Fake(ctx.Cloud, ctx) return NewILBController(ctx, stopCh) } diff --git a/pkg/l4lb/l4netlbcontroller_test.go b/pkg/l4lb/l4netlbcontroller_test.go index cb7ae44f8e..476c97a030 100644 --- a/pkg/l4lb/l4netlbcontroller_test.go +++ b/pkg/l4lb/l4netlbcontroller_test.go @@ -42,7 +42,7 @@ import ( "k8s.io/ingress-gce/pkg/annotations" "k8s.io/ingress-gce/pkg/composite" ingctx "k8s.io/ingress-gce/pkg/context" - "k8s.io/ingress-gce/pkg/healthchecks" + "k8s.io/ingress-gce/pkg/healthchecksl4" "k8s.io/ingress-gce/pkg/loadbalancers" "k8s.io/ingress-gce/pkg/test" "k8s.io/ingress-gce/pkg/utils" @@ -240,7 +240,7 @@ func newL4NetLBServiceController() *L4NetLBController { for _, n := range nodes { ctx.NodeInformer.GetIndexer().Add(n) } - healthchecks.FakeL4(ctx.Cloud, ctx) + healthchecksl4.Fake(ctx.Cloud, ctx) return NewL4NetLBController(ctx, stopCh) } @@ -873,7 +873,7 @@ func TestHealthCheckWhenExternalTrafficPolicyWasUpdated(t *testing.T) { // delete shared health check if is created, update service to Cluster and // check that non-shared health check was created hcNameShared := lc.namer.L4HealthCheck(svc.Namespace, svc.Name, true) - healthchecks.FakeL4(lc.ctx.Cloud, lc.ctx).DeleteHealthCheck(svc, lc.namer, true, meta.Regional, utils.XLB) + healthchecksl4.Fake(lc.ctx.Cloud, lc.ctx).DeleteHealthCheck(svc, lc.namer, true, meta.Regional, utils.XLB) // Update ExternalTrafficPolicy to Cluster check if shared HC was created err = updateAndAssertExternalTrafficPolicy(newSvc, lc, v1.ServiceExternalTrafficPolicyTypeCluster, hcNameShared) if err != nil { diff --git a/pkg/loadbalancers/l4.go b/pkg/loadbalancers/l4.go index dcfbf754f6..697b39c559 100644 --- a/pkg/loadbalancers/l4.go +++ b/pkg/loadbalancers/l4.go @@ -31,7 +31,7 @@ import ( "k8s.io/ingress-gce/pkg/composite" "k8s.io/ingress-gce/pkg/firewalls" "k8s.io/ingress-gce/pkg/forwardingrules" - "k8s.io/ingress-gce/pkg/healthchecks" + "k8s.io/ingress-gce/pkg/healthchecksl4" "k8s.io/ingress-gce/pkg/metrics" "k8s.io/ingress-gce/pkg/utils" "k8s.io/ingress-gce/pkg/utils/namer" @@ -51,8 +51,8 @@ type L4 struct { Service *corev1.Service ServicePort utils.ServicePort NamespacedName types.NamespacedName - healthChecks healthchecks.L4HealthChecks forwardingRules ForwardingRulesProvider + healthChecks healthchecksl4.L4HealthChecks } // L4ILBSyncResult contains information about the outcome of an L4 ILB sync. It stores the list of resource name annotations, @@ -83,7 +83,7 @@ func NewL4Handler(params *L4ILBParams) *L4 { namer: params.Namer, recorder: params.Recorder, Service: params.Service, - healthChecks: healthchecks.L4(), + healthChecks: healthchecksl4.GetInstance(), forwardingRules: forwardingrules.New(params.Cloud, meta.VersionGA, scope), } l4.NamespacedName = types.NamespacedName{Name: params.Service.Name, Namespace: params.Service.Namespace} @@ -201,7 +201,7 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service // create healthcheck sharedHC := !helpers.RequestsOnlyLocalTraffic(l4.Service) - hcResult := l4.healthChecks.EnsureL4HealthCheck(l4.Service, l4.namer, sharedHC, meta.Global, utils.ILB, nodeNames) + hcResult := l4.healthChecks.EnsureHealthCheck(l4.Service, l4.namer, sharedHC, meta.Global, utils.ILB, nodeNames) if hcResult.Err != nil { result.GCEResourceInError = hcResult.GceResourceInError diff --git a/pkg/loadbalancers/l4_test.go b/pkg/loadbalancers/l4_test.go index 159f0e47d4..2bc2f6f072 100644 --- a/pkg/loadbalancers/l4_test.go +++ b/pkg/loadbalancers/l4_test.go @@ -23,11 +23,10 @@ import ( "strings" "testing" - "k8s.io/ingress-gce/pkg/healthchecks" - "google.golang.org/api/compute/v1" "k8s.io/ingress-gce/pkg/backends" "k8s.io/ingress-gce/pkg/firewalls" + "k8s.io/ingress-gce/pkg/healthchecksl4" "k8s.io/ingress-gce/pkg/utils" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" @@ -75,7 +74,7 @@ func TestEnsureInternalBackendServiceUpdates(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) bsName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) _, err := l4.backendPool.EnsureL4BackendService(bsName, "", "TCP", string(svc.Spec.SessionAffinity), string(cloud.SchemeInternal), l4.NamespacedName, meta.VersionGA) @@ -125,6 +124,7 @@ func TestEnsureInternalLoadBalancer(t *testing.T) { svc := test.NewL4ILBService(false, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -132,7 +132,7 @@ func TestEnsureInternalLoadBalancer(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -188,6 +188,7 @@ func TestEnsureInternalLoadBalancerTypeChange(t *testing.T) { svc := test.NewL4ILBService(false, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -195,7 +196,7 @@ func TestEnsureInternalLoadBalancerTypeChange(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -235,7 +236,7 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -245,7 +246,7 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) { // Create the expected resources necessary for an Internal Load Balancer sharedHC := !servicehelper.RequestsOnlyLocalTraffic(svc) - hcResult := l4.healthChecks.EnsureL4HealthCheck(l4.Service, l4.namer, sharedHC, meta.Global, utils.ILB, []string{}) + hcResult := l4.healthChecks.EnsureHealthCheck(l4.Service, l4.namer, sharedHC, meta.Global, utils.ILB, []string{}) if hcResult.Err != nil { t.Errorf("Failed to create healthcheck, err %v", hcResult.Err) @@ -277,6 +278,7 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { svc := test.NewL4ILBService(true, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -284,7 +286,7 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { @@ -403,6 +405,7 @@ func TestUpdateResourceLinks(t *testing.T) { svc := test.NewL4ILBService(true, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -410,7 +413,7 @@ func TestUpdateResourceLinks(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { @@ -487,6 +490,7 @@ func TestEnsureInternalLoadBalancerHealthCheckConfigurable(t *testing.T) { svc := test.NewL4ILBService(true, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -494,7 +498,7 @@ func TestEnsureInternalLoadBalancerHealthCheckConfigurable(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { @@ -536,6 +540,7 @@ func TestEnsureInternalLoadBalancerDeleted(t *testing.T) { nodeNames := []string{"test-node-1"} svc := test.NewL4ILBService(false, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -543,7 +548,7 @@ func TestEnsureInternalLoadBalancerDeleted(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -574,6 +579,7 @@ func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) { nodeNames := []string{"test-node-1"} svc := test.NewL4ILBService(false, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -581,7 +587,7 @@ func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -698,6 +704,7 @@ func TestHealthCheckFirewallDeletionWithNetLB(t *testing.T) { func ensureService(fakeGCE *gce.Cloud, namer *namer_util.L4Namer, nodeNames []string, zoneName string, port int, t *testing.T) (*v1.Service, *L4, *L4ILBSyncResult) { svc := test.NewL4ILBService(false, 8080) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -705,7 +712,7 @@ func ensureService(fakeGCE *gce.Cloud, namer *namer_util.L4Namer, nodeNames []st Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, zoneName); err != nil { return nil, nil, &L4ILBSyncResult{Error: fmt.Errorf("Unexpected error when adding nodes %v", err)} @@ -729,6 +736,7 @@ func TestEnsureInternalLoadBalancerWithSpecialHealthCheck(t *testing.T) { nodeNames := []string{"test-node-1"} svc := test.NewL4ILBService(false, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -736,7 +744,7 @@ func TestEnsureInternalLoadBalancerWithSpecialHealthCheck(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -848,7 +856,7 @@ func TestEnsureInternalLoadBalancerErrors(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) //lbName :=l4.namer.L4Backend(params.service.Namespace, params.service.Name) frName := l4.GetFRName() @@ -930,6 +938,7 @@ func TestEnsureInternalLoadBalancerEnableGlobalAccess(t *testing.T) { nodeNames := []string{"test-node-1"} svc := test.NewL4ILBService(false, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -937,7 +946,7 @@ func TestEnsureInternalLoadBalancerEnableGlobalAccess(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -1018,6 +1027,7 @@ func TestEnsureInternalLoadBalancerCustomSubnet(t *testing.T) { svc := test.NewL4ILBService(false, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -1025,7 +1035,7 @@ func TestEnsureInternalLoadBalancerCustomSubnet(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -1122,6 +1132,7 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) { svc := test.NewL4ILBService(false, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -1129,7 +1140,7 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) fwName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) tc := struct { @@ -1183,6 +1194,7 @@ func TestEnsureInternalLoadBalancerModifyProtocol(t *testing.T) { nodeNames := []string{"test-node-1"} svc := test.NewL4ILBService(false, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -1190,7 +1202,7 @@ func TestEnsureInternalLoadBalancerModifyProtocol(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName) if err != nil { @@ -1281,6 +1293,7 @@ func TestEnsureInternalLoadBalancerAllPorts(t *testing.T) { nodeNames := []string{"test-node-1"} svc := test.NewL4ILBService(false, 8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) + l4ilbParams := &L4ILBParams{ Service: svc, Cloud: fakeGCE, @@ -1288,7 +1301,7 @@ func TestEnsureInternalLoadBalancerAllPorts(t *testing.T) { Recorder: record.NewFakeRecorder(100), } l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) diff --git a/pkg/loadbalancers/l4netlb.go b/pkg/loadbalancers/l4netlb.go index cb5f5d3ac3..0dc3c2ff9b 100644 --- a/pkg/loadbalancers/l4netlb.go +++ b/pkg/loadbalancers/l4netlb.go @@ -31,7 +31,7 @@ import ( "k8s.io/ingress-gce/pkg/composite" "k8s.io/ingress-gce/pkg/firewalls" "k8s.io/ingress-gce/pkg/forwardingrules" - "k8s.io/ingress-gce/pkg/healthchecks" + "k8s.io/ingress-gce/pkg/healthchecksl4" "k8s.io/ingress-gce/pkg/metrics" "k8s.io/ingress-gce/pkg/utils" "k8s.io/ingress-gce/pkg/utils/namer" @@ -50,7 +50,7 @@ type L4NetLB struct { Service *corev1.Service ServicePort utils.ServicePort NamespacedName types.NamespacedName - healthChecks healthchecks.L4HealthChecks + healthChecks healthchecksl4.L4HealthChecks forwardingRules ForwardingRulesProvider } @@ -91,7 +91,7 @@ func NewL4NetLB(service *corev1.Service, cloud *gce.Cloud, scope meta.KeyType, n Service: service, NamespacedName: types.NamespacedName{Name: service.Name, Namespace: service.Namespace}, backendPool: backends.NewPool(cloud, namer), - healthChecks: healthchecks.L4(), + healthChecks: healthchecksl4.GetInstance(), forwardingRules: forwardingrules.New(cloud, meta.VersionGA, scope), } portId := utils.ServicePortID{Service: l4netlb.NamespacedName} @@ -124,7 +124,7 @@ func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service) l4netlb.Service = svc sharedHC := !helpers.RequestsOnlyLocalTraffic(svc) - hcResult := l4netlb.healthChecks.EnsureL4HealthCheck(l4netlb.Service, l4netlb.namer, sharedHC, l4netlb.scope, utils.XLB, nodeNames) + hcResult := l4netlb.healthChecks.EnsureHealthCheck(l4netlb.Service, l4netlb.namer, sharedHC, l4netlb.scope, utils.XLB, nodeNames) if hcResult.Err != nil { result.GCEResourceInError = hcResult.GceResourceInError diff --git a/pkg/loadbalancers/l4netlb_test.go b/pkg/loadbalancers/l4netlb_test.go index 29c048cff7..eeea81c8f1 100644 --- a/pkg/loadbalancers/l4netlb_test.go +++ b/pkg/loadbalancers/l4netlb_test.go @@ -21,10 +21,6 @@ import ( "strings" "testing" - "k8s.io/ingress-gce/pkg/firewalls" - "k8s.io/ingress-gce/pkg/flags" - "k8s.io/ingress-gce/pkg/healthchecks" - "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/mock" @@ -34,6 +30,9 @@ import ( servicehelper "k8s.io/cloud-provider/service/helpers" "k8s.io/ingress-gce/pkg/annotations" "k8s.io/ingress-gce/pkg/composite" + "k8s.io/ingress-gce/pkg/firewalls" + "k8s.io/ingress-gce/pkg/flags" + "k8s.io/ingress-gce/pkg/healthchecksl4" "k8s.io/ingress-gce/pkg/metrics" "k8s.io/ingress-gce/pkg/test" "k8s.io/ingress-gce/pkg/utils" @@ -57,7 +56,7 @@ func TestEnsureL4NetLoadBalancer(t *testing.T) { namer := namer_util.NewL4Namer(kubeSystemUID, namer_util.NewNamer(vals.ClusterName, "cluster-fw")) l4netlb := NewL4NetLB(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) - l4netlb.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4netlb.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4netlb.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -108,7 +107,7 @@ func TestDeleteL4NetLoadBalancer(t *testing.T) { namer := namer_util.NewL4Namer(kubeSystemUID, namer_util.NewNamer(vals.ClusterName, "cluster-fw")) l4NetLB := NewL4NetLB(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) - l4NetLB.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4NetLB.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4NetLB.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -211,7 +210,7 @@ func ensureLoadBalancer(port int, vals gce.TestClusterValues, fakeGCE *gce.Cloud emptyNodes := []string{} l4NetLB := NewL4NetLB(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) - l4NetLB.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4NetLB.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) result := l4NetLB.EnsureFrontend(emptyNodes, svc) if result.Error != nil { @@ -354,7 +353,7 @@ func TestMetricsForStandardNetworkTier(t *testing.T) { namer := namer_util.NewL4Namer(kubeSystemUID, namer_util.NewNamer(vals.ClusterName, "cluster-fw")) l4netlb := NewL4NetLB(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) - l4netlb.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4netlb.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4netlb.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err) @@ -401,7 +400,7 @@ func TestEnsureNetLBFirewallDestinations(t *testing.T) { svc := test.NewL4NetLBRBSService(8080) namer := namer_util.NewL4Namer(kubeSystemUID, nil) l4netlb := NewL4NetLB(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100)) - l4netlb.healthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{}) + l4netlb.healthChecks = healthchecksl4.Fake(fakeGCE, &test.FakeRecorderSource{}) if _, err := test.CreateAndInsertNodes(l4netlb.cloud, nodeNames, vals.ZoneName); err != nil { t.Errorf("Unexpected error when adding nodes %v", err)