Skip to content

Commit 0807861

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2917 from 08volt/health-checks-bs-3
Compare health checks via resource path to support multi-version APIs
2 parents daa7c4f + af5fb48 commit 0807861

File tree

4 files changed

+177
-2
lines changed

4 files changed

+177
-2
lines changed

‎pkg/backends/backends.go‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"k8s.io/cloud-provider-gcp/providers/gce"
2727
"k8s.io/ingress-gce/pkg/backends/features"
2828
"k8s.io/ingress-gce/pkg/composite"
29+
"k8s.io/ingress-gce/pkg/flags"
2930
"k8s.io/ingress-gce/pkg/network"
3031
"k8s.io/ingress-gce/pkg/utils"
3132
"k8s.io/ingress-gce/pkg/utils/namer"
@@ -494,13 +495,20 @@ func (p *Pool) EnsureL4BackendService(params L4BackendServiceParams, beLogger kl
494495
// since that is handled by the neg-linker.
495496
// The list of backends is not checked, since that is handled by the neg-linker.
496497
func backendSvcEqual(newBS, oldBS *composite.BackendService, compareConnectionTracking bool) bool {
498+
497499
svcsEqual := newBS.Protocol == oldBS.Protocol &&
498500
newBS.Description == oldBS.Description &&
499501
newBS.SessionAffinity == oldBS.SessionAffinity &&
500502
newBS.LoadBalancingScheme == oldBS.LoadBalancingScheme &&
501-
utils.EqualStringSets(newBS.HealthChecks, oldBS.HealthChecks) &&
502503
newBS.Network == oldBS.Network
503504

505+
if flags.F.EnableL4ILBZonalAffinity {
506+
// Compare healthChecks sets ignoring api version
507+
svcsEqual = svcsEqual && healthChecksEqual(newBS.HealthChecks, oldBS.HealthChecks)
508+
} else {
509+
svcsEqual = svcsEqual && utils.EqualStringSets(newBS.HealthChecks, oldBS.HealthChecks)
510+
}
511+
504512
// Compare only for backendSvc that uses Strong Session Affinity feature
505513
if compareConnectionTracking {
506514
svcsEqual = svcsEqual && connectionTrackingPolicyEqual(newBS.ConnectionTrackingPolicy, oldBS.ConnectionTrackingPolicy)
@@ -517,6 +525,17 @@ func backendSvcEqual(newBS, oldBS *composite.BackendService, compareConnectionTr
517525
return svcsEqual
518526
}
519527

528+
// removeAPIVersionFromHealthChecks converts a slice of full health check URLs
529+
// into a slice of their URL without the API version
530+
func removeAPIVersionFromHealthChecks(hcLinks []string) []string {
531+
hcResourcePaths := make([]string, 0, len(hcLinks))
532+
for _, hcLink := range hcLinks {
533+
resourcePath := utils.FilterAPIVersionFromResourcePath(hcLink)
534+
hcResourcePaths = append(hcResourcePaths, resourcePath)
535+
}
536+
return hcResourcePaths
537+
}
538+
520539
func convertNetworkLbTrafficPolicyToZonalAffinity(trafficPolicy *composite.BackendServiceNetworkPassThroughLbTrafficPolicy) composite.BackendServiceNetworkPassThroughLbTrafficPolicyZonalAffinity {
521540
if trafficPolicy == nil || trafficPolicy.ZonalAffinity == nil {
522541
return *zonalAffinityDisabledTrafficPolicy().ZonalAffinity
@@ -525,6 +544,14 @@ func convertNetworkLbTrafficPolicyToZonalAffinity(trafficPolicy *composite.Backe
525544
return *trafficPolicy.ZonalAffinity
526545
}
527546

547+
// healthCheckEqual compare healthcheck URL ignoring the API version used
548+
func healthChecksEqual(hcLinksA, hcLinksB []string) bool {
549+
healthChecksA := removeAPIVersionFromHealthChecks(hcLinksA)
550+
healthChecksB := removeAPIVersionFromHealthChecks(hcLinksB)
551+
552+
return utils.EqualStringSets(healthChecksA, healthChecksB)
553+
}
554+
528555
func zonalAffinityEqual(a, b *composite.BackendService) bool {
529556
aZonalAffinity := convertNetworkLbTrafficPolicyToZonalAffinity(a.NetworkPassThroughLbTrafficPolicy)
530557
bZonalAffinity := convertNetworkLbTrafficPolicyToZonalAffinity(b.NetworkPassThroughLbTrafficPolicy)

‎pkg/backends/backends_test.go‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"k8s.io/apimachinery/pkg/types"
1313
"k8s.io/cloud-provider-gcp/providers/gce"
1414
"k8s.io/ingress-gce/pkg/composite"
15+
"k8s.io/ingress-gce/pkg/flags"
1516
"k8s.io/ingress-gce/pkg/network"
1617
"k8s.io/ingress-gce/pkg/utils"
1718
"k8s.io/ingress-gce/pkg/utils/namer"
@@ -347,6 +348,7 @@ func TestBackendSvcEqual(t *testing.T) {
347348
oldBackendService *composite.BackendService
348349
newBackendService *composite.BackendService
349350
compareConnectionTracking bool
351+
withZonalAffinityEnabled bool
350352
wantEqual bool
351353
}{
352354
{
@@ -478,6 +480,72 @@ func TestBackendSvcEqual(t *testing.T) {
478480
},
479481
wantEqual: false,
480482
},
483+
{
484+
desc: "Test with changed health-checks with Zonal Affinity Enabled",
485+
oldBackendService: &composite.BackendService{
486+
HealthChecks: []string{"abc"},
487+
},
488+
newBackendService: &composite.BackendService{
489+
HealthChecks: []string{"abc", "xyz"},
490+
},
491+
withZonalAffinityEnabled: true,
492+
wantEqual: false,
493+
},
494+
{
495+
desc: "Test with same health-checks version v1-beta",
496+
oldBackendService: &composite.BackendService{
497+
HealthChecks: []string{"https://www.googleapis.com/compute/v1/abc"},
498+
},
499+
newBackendService: &composite.BackendService{
500+
HealthChecks: []string{"https://www.googleapis.com/compute/beta/abc"},
501+
},
502+
withZonalAffinityEnabled: true,
503+
wantEqual: true,
504+
},
505+
{
506+
desc: "Test with same health-checks version beta-v1",
507+
oldBackendService: &composite.BackendService{
508+
HealthChecks: []string{"https://www.googleapis.com/compute/beta/abc"},
509+
},
510+
newBackendService: &composite.BackendService{
511+
HealthChecks: []string{"https://www.googleapis.com/compute/v1/abc"},
512+
},
513+
withZonalAffinityEnabled: true,
514+
wantEqual: true,
515+
},
516+
{
517+
desc: "Test with changed health-checks version beta-beta",
518+
oldBackendService: &composite.BackendService{
519+
HealthChecks: []string{"https://www.googleapis.com/compute/beta/abc"},
520+
},
521+
newBackendService: &composite.BackendService{
522+
HealthChecks: []string{"https://www.googleapis.com/compute/beta/abcd"},
523+
},
524+
withZonalAffinityEnabled: true,
525+
wantEqual: false,
526+
},
527+
{
528+
desc: "Test with changed first part of health-checks version v1-v1",
529+
oldBackendService: &composite.BackendService{
530+
HealthChecks: []string{"https://www.googleapis.com/compute/v1/abc"},
531+
},
532+
newBackendService: &composite.BackendService{
533+
HealthChecks: []string{"https://www.google.com/compute/v1/abc"},
534+
},
535+
withZonalAffinityEnabled: true,
536+
wantEqual: false,
537+
},
538+
{
539+
desc: "Test with changed health-checks version beta-v1",
540+
oldBackendService: &composite.BackendService{
541+
HealthChecks: []string{"https://www.googleapis.com/compute/v1/abc"},
542+
},
543+
newBackendService: &composite.BackendService{
544+
HealthChecks: []string{"https://www.googleapis.com/compute/beta/abcd"},
545+
},
546+
withZonalAffinityEnabled: true,
547+
wantEqual: false,
548+
},
481549
{
482550
desc: "Test with deleted network",
483551
oldBackendService: &composite.BackendService{
@@ -728,7 +796,11 @@ func TestBackendSvcEqual(t *testing.T) {
728796
} {
729797
tc := tc
730798
t.Run(tc.desc, func(t *testing.T) {
731-
t.Parallel()
799+
oldFlag := flags.F.EnableL4ILBZonalAffinity
800+
flags.F.EnableL4ILBZonalAffinity = tc.withZonalAffinityEnabled
801+
defer func() {
802+
flags.F.EnableL4ILBZonalAffinity = oldFlag
803+
}()
732804
result := backendSvcEqual(tc.newBackendService, tc.oldBackendService, tc.compareConnectionTracking)
733805
if result != tc.wantEqual {
734806
t.Errorf("backendSvcEqual() returned %v, expected %v. Diff(oldScv, newSvc): %s",

‎pkg/utils/utils.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -884,3 +884,33 @@ func GetDomainFromGABasePath(basePath string) string {
884884
domain = strings.TrimSuffix(domain, "/compute/v1")
885885
return domain
886886
}
887+
888+
// FilterAPIVersionFromResourcePath removes the /v1 /beta /alpha from the resource path
889+
func FilterAPIVersionFromResourcePath(url string) string {
890+
computeIndex := strings.Index(url, "/compute/")
891+
if computeIndex == -1 {
892+
return url
893+
}
894+
895+
pathStartIndex := computeIndex + len("/compute/")
896+
897+
// if the URL ends with "/compute/" there is no version to remove
898+
if pathStartIndex >= len(url) {
899+
return url
900+
}
901+
902+
baseUrlPart := url[:pathStartIndex]
903+
pathAfterCompute := url[pathStartIndex:]
904+
905+
firstSlashIndex := strings.Index(pathAfterCompute, "/")
906+
if firstSlashIndex == -1 {
907+
// This case would mean the url is something like ".../compute/v1", without a resource path.
908+
// in this case with return the first part of the url ".../compute/"
909+
return baseUrlPart
910+
}
911+
912+
// reconstruct the URL removing the version segment
913+
resourcePathPart := pathAfterCompute[firstSlashIndex+1:]
914+
915+
return baseUrlPart + resourcePathPart
916+
}

‎pkg/utils/utils_test.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1755,3 +1755,49 @@ func TestGetDomainFromGABasePath(t *testing.T) {
17551755
})
17561756
}
17571757
}
1758+
1759+
func TestFilterAPIVersionFromResourcePath(t *testing.T) {
1760+
t.Parallel()
1761+
testCases := []struct {
1762+
desc string
1763+
basePath string
1764+
want string
1765+
}{
1766+
{
1767+
desc: "empty string",
1768+
},
1769+
{
1770+
desc: "v1 URL",
1771+
basePath: "https://www.googleapis.com/compute/v1/projects/my-project/global/backendServices/my-bs",
1772+
want: "https://www.googleapis.com/compute/projects/my-project/global/backendServices/my-bs",
1773+
},
1774+
{
1775+
desc: "beta URL",
1776+
basePath: "https://www.googleapis.com/compute/beta/projects/my-project/zones/us-central1-a/instanceGroups/my-ig",
1777+
want: "https://www.googleapis.com/compute/projects/my-project/zones/us-central1-a/instanceGroups/my-ig",
1778+
},
1779+
{
1780+
desc: "arbitrary path",
1781+
basePath: "mycompute.mydomain.com/mypath/compute/v1/abc/def",
1782+
want: "mycompute.mydomain.com/mypath/compute/abc/def",
1783+
},
1784+
{
1785+
desc: "path without compute",
1786+
basePath: "https://www.googleapis.com/storage/v1/b/my-bucket",
1787+
want: "https://www.googleapis.com/storage/v1/b/my-bucket",
1788+
},
1789+
{
1790+
desc: "path ends after version",
1791+
basePath: "https://www.googleapis.com/compute/v1/",
1792+
want: "https://www.googleapis.com/compute/",
1793+
},
1794+
}
1795+
1796+
for _, tc := range testCases {
1797+
t.Run(tc.desc, func(t *testing.T) {
1798+
if got := utils.FilterAPIVersionFromResourcePath(tc.basePath); got != tc.want {
1799+
t.Errorf("FilterAPIVersionFromResourcePath(%q) = %q, want %q", tc.basePath, got, tc.want)
1800+
}
1801+
})
1802+
}
1803+
}

0 commit comments

Comments
 (0)