From 8fffbe3bc2a1458221309622cce695dbe4e396e6 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Fri, 22 Nov 2024 14:34:50 -0800 Subject: [PATCH 1/6] platform agnostic lb ep hash --- .../platform-agnostic-lbephash.yaml | 6 ++++++ projects/gateway2/krtcollections/endpoints.go | 4 ++-- .../gateway2/krtcollections/endpoints_test.go | 20 +++++++++++++++---- projects/gateway2/proxy_syncer/cla.go | 12 ++++++++--- 4 files changed, 33 insertions(+), 9 deletions(-) create mode 100644 changelog/v1.18.0-rc2/platform-agnostic-lbephash.yaml diff --git a/changelog/v1.18.0-rc2/platform-agnostic-lbephash.yaml b/changelog/v1.18.0-rc2/platform-agnostic-lbephash.yaml new file mode 100644 index 00000000000..8642963f8aa --- /dev/null +++ b/changelog/v1.18.0-rc2/platform-agnostic-lbephash.yaml @@ -0,0 +1,6 @@ +changelog: + - type: NON_USER_FACING + description: >- + Don't use xor or native endian-ness when generating hashes. + This creates difficulty reproducing tests on different machines + where the hashes end up in goldenfiles. diff --git a/projects/gateway2/krtcollections/endpoints.go b/projects/gateway2/krtcollections/endpoints.go index 4a69ba85c09..611b23df16d 100644 --- a/projects/gateway2/krtcollections/endpoints.go +++ b/projects/gateway2/krtcollections/endpoints.go @@ -170,8 +170,8 @@ func hashEndpoints(l PodLocality, emd EndpointWithMd) uint64 { func hash(a, b uint64) uint64 { hasher := fnv.New64a() var buf [16]byte - binary.NativeEndian.PutUint64(buf[:8], a) - binary.NativeEndian.PutUint64(buf[8:], b) + binary.LittleEndian.PutUint64(buf[:8], a) + binary.LittleEndian.PutUint64(buf[8:], b) hasher.Write(buf[:]) return hasher.Sum64() } diff --git a/projects/gateway2/krtcollections/endpoints_test.go b/projects/gateway2/krtcollections/endpoints_test.go index 1f4d84d7191..6ab89f25ef8 100644 --- a/projects/gateway2/krtcollections/endpoints_test.go +++ b/projects/gateway2/krtcollections/endpoints_test.go @@ -2,6 +2,8 @@ package krtcollections import ( "context" + "crypto/sha256" + "encoding/binary" "testing" envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -131,7 +133,6 @@ func TestEndpointsForUpstreamOrderDoesntMatter(t *testing.T) { Zone: "zone2", }, emd2) g.Expect(result1.Equals(*result4)).To(BeFalse(), "not expected %v, got %v", result1, result2) - } func TestEndpointsForUpstreamWithDiscoveredUpstream(t *testing.T) { @@ -235,11 +236,23 @@ func TestEndpointsForUpstreamWithDiscoveredUpstream(t *testing.T) { Zone: "zone", }, emd2) - h1 := result1.LbEpsEqualityHash ^ result2.LbEpsEqualityHash - h2 := result3.LbEpsEqualityHash ^ result4.LbEpsEqualityHash + h1 := combineHashes(result1.LbEpsEqualityHash, result2.LbEpsEqualityHash) + h2 := combineHashes(result3.LbEpsEqualityHash, result4.LbEpsEqualityHash) g.Expect(h1).NotTo(Equal(h2), "not expected %v, got %v", h1, h2) +} + +func combineHashes(hash1, hash2 uint64) uint64 { + // Create a buffer for the combined hashes + buf := make([]byte, 16) + binary.LittleEndian.PutUint64(buf[:8], hash1) + binary.LittleEndian.PutUint64(buf[8:], hash2) + // Hash the combined buffer + sum := sha256.Sum256(buf) + + // Return the first 8 bytes of the hash as a uint64 + return binary.LittleEndian.Uint64(sum[:8]) } func TestEndpoints(t *testing.T) { @@ -960,5 +973,4 @@ func TestEndpoints(t *testing.T) { g.Expect(eps.Equals(*res)).To(BeTrue(), "expected %v, got %v", res, eps) }) } - } diff --git a/projects/gateway2/proxy_syncer/cla.go b/projects/gateway2/proxy_syncer/cla.go index 86961495fcc..f57dc3994da 100644 --- a/projects/gateway2/proxy_syncer/cla.go +++ b/projects/gateway2/proxy_syncer/cla.go @@ -1,6 +1,7 @@ package proxy_syncer import ( + "encoding/binary" "fmt" "hash/fnv" @@ -129,18 +130,23 @@ func NewPerClientEnvoyEndpoints(logger *zap.Logger, dbg *krt.DebugHandler, uccs } func PrioritizeEndpoints(logger *zap.Logger, destrule *DestinationRuleWrapper, ep krtcollections.EndpointsForUpstream, ucc krtcollections.UniqlyConnectedClient) UccWithEndpoints { - var additionalHash uint64 var priorityInfo *PriorityInfo + updatedHash := ep.LbEpsEqualityHash if destrule != nil { trafficPolicy := getTraficPolicy(destrule, ep.Port) localityLb := getLocalityLbSetting(trafficPolicy) if localityLb != nil { priorityInfo = getPriorityInfoFromDestrule(localityLb) hasher := fnv.New64() + + oldHash := make([]byte, 8) + binary.LittleEndian.PutUint64(oldHash, updatedHash) + hasher.Write(oldHash) + hasher.Write([]byte(destrule.UID)) hasher.Write([]byte(fmt.Sprintf("%v", destrule.Generation))) - additionalHash = hasher.Sum64() + updatedHash = hasher.Sum64() } } lbInfo := LoadBalancingInfo{ @@ -153,7 +159,7 @@ func PrioritizeEndpoints(logger *zap.Logger, destrule *DestinationRuleWrapper, e return UccWithEndpoints{ Client: ucc, Endpoints: resource.NewEnvoyResource(cla), - EndpointsHash: ep.LbEpsEqualityHash ^ additionalHash, + EndpointsHash: updatedHash, endpointsName: ep.ResourceName(), } } From 7174e7c00c22cc57fea6a358e04c9eadf402b452 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Mon, 25 Nov 2024 10:02:41 -0800 Subject: [PATCH 2/6] address comments --- .../gateway2/krtcollections/endpoints_test.go | 17 ++--------------- projects/gateway2/proxy_syncer/cla.go | 11 +++-------- projects/gateway2/utils/hash.go | 2 +- 3 files changed, 6 insertions(+), 24 deletions(-) diff --git a/projects/gateway2/krtcollections/endpoints_test.go b/projects/gateway2/krtcollections/endpoints_test.go index 6ab89f25ef8..f3e8b4c9690 100644 --- a/projects/gateway2/krtcollections/endpoints_test.go +++ b/projects/gateway2/krtcollections/endpoints_test.go @@ -236,25 +236,12 @@ func TestEndpointsForUpstreamWithDiscoveredUpstream(t *testing.T) { Zone: "zone", }, emd2) - h1 := combineHashes(result1.LbEpsEqualityHash, result2.LbEpsEqualityHash) - h2 := combineHashes(result3.LbEpsEqualityHash, result4.LbEpsEqualityHash) + h1 := result1.LbEpsEqualityHash ^ result2.LbEpsEqualityHash + h2 := result3.LbEpsEqualityHash ^ result4.LbEpsEqualityHash g.Expect(h1).NotTo(Equal(h2), "not expected %v, got %v", h1, h2) } -func combineHashes(hash1, hash2 uint64) uint64 { - // Create a buffer for the combined hashes - buf := make([]byte, 16) - binary.LittleEndian.PutUint64(buf[:8], hash1) - binary.LittleEndian.PutUint64(buf[8:], hash2) - - // Hash the combined buffer - sum := sha256.Sum256(buf) - - // Return the first 8 bytes of the hash as a uint64 - return binary.LittleEndian.Uint64(sum[:8]) -} - func TestEndpoints(t *testing.T) { testCases := []struct { name string diff --git a/projects/gateway2/proxy_syncer/cla.go b/projects/gateway2/proxy_syncer/cla.go index f57dc3994da..74a8f3225b5 100644 --- a/projects/gateway2/proxy_syncer/cla.go +++ b/projects/gateway2/proxy_syncer/cla.go @@ -130,23 +130,18 @@ func NewPerClientEnvoyEndpoints(logger *zap.Logger, dbg *krt.DebugHandler, uccs } func PrioritizeEndpoints(logger *zap.Logger, destrule *DestinationRuleWrapper, ep krtcollections.EndpointsForUpstream, ucc krtcollections.UniqlyConnectedClient) UccWithEndpoints { + var additionalHash uint64 var priorityInfo *PriorityInfo - updatedHash := ep.LbEpsEqualityHash if destrule != nil { trafficPolicy := getTraficPolicy(destrule, ep.Port) localityLb := getLocalityLbSetting(trafficPolicy) if localityLb != nil { priorityInfo = getPriorityInfoFromDestrule(localityLb) hasher := fnv.New64() - - oldHash := make([]byte, 8) - binary.LittleEndian.PutUint64(oldHash, updatedHash) - hasher.Write(oldHash) - hasher.Write([]byte(destrule.UID)) hasher.Write([]byte(fmt.Sprintf("%v", destrule.Generation))) - updatedHash = hasher.Sum64() + additionalHash = hasher.Sum64() } } lbInfo := LoadBalancingInfo{ @@ -159,7 +154,7 @@ func PrioritizeEndpoints(logger *zap.Logger, destrule *DestinationRuleWrapper, e return UccWithEndpoints{ Client: ucc, Endpoints: resource.NewEnvoyResource(cla), - EndpointsHash: updatedHash, + EndpointsHash: additionalHash, endpointsName: ep.ResourceName(), } } diff --git a/projects/gateway2/utils/hash.go b/projects/gateway2/utils/hash.go index e787776475c..8ec7b988b76 100644 --- a/projects/gateway2/utils/hash.go +++ b/projects/gateway2/utils/hash.go @@ -119,7 +119,7 @@ func hashValue(newhash func() hash.Hash64, h hash.Hash, value *structpb.Value) e func HashUint64(hasher io.Writer, value uint64) { var bytes [8]byte - binary.NativeEndian.PutUint64(bytes[:], value) + binary.LittleEndian.PutUint64(bytes[:], value) hasher.Write(bytes[:]) } From 30b2f470b4487f82c34fd755ffe8e83e8834e4ea Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Mon, 25 Nov 2024 11:50:57 -0800 Subject: [PATCH 3/6] changelog --- .../{v1.18.0-rc2 => v1.18.0-rc3}/platform-agnostic-lbephash.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{v1.18.0-rc2 => v1.18.0-rc3}/platform-agnostic-lbephash.yaml (100%) diff --git a/changelog/v1.18.0-rc2/platform-agnostic-lbephash.yaml b/changelog/v1.18.0-rc3/platform-agnostic-lbephash.yaml similarity index 100% rename from changelog/v1.18.0-rc2/platform-agnostic-lbephash.yaml rename to changelog/v1.18.0-rc3/platform-agnostic-lbephash.yaml From 709febcd841742651b3e575529561a39c1b99337 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Mon, 25 Nov 2024 12:14:14 -0800 Subject: [PATCH 4/6] cleanup imports --- projects/gateway2/krtcollections/endpoints_test.go | 2 -- projects/gateway2/proxy_syncer/cla.go | 1 - 2 files changed, 3 deletions(-) diff --git a/projects/gateway2/krtcollections/endpoints_test.go b/projects/gateway2/krtcollections/endpoints_test.go index f3e8b4c9690..40c8348d9f6 100644 --- a/projects/gateway2/krtcollections/endpoints_test.go +++ b/projects/gateway2/krtcollections/endpoints_test.go @@ -2,8 +2,6 @@ package krtcollections import ( "context" - "crypto/sha256" - "encoding/binary" "testing" envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" diff --git a/projects/gateway2/proxy_syncer/cla.go b/projects/gateway2/proxy_syncer/cla.go index 74a8f3225b5..60baf2dcf2f 100644 --- a/projects/gateway2/proxy_syncer/cla.go +++ b/projects/gateway2/proxy_syncer/cla.go @@ -1,7 +1,6 @@ package proxy_syncer import ( - "encoding/binary" "fmt" "hash/fnv" From 562ab0e57c42a4b91ffbf79301600d501ebd4962 Mon Sep 17 00:00:00 2001 From: Steven Landow Date: Mon, 25 Nov 2024 14:34:02 -0800 Subject: [PATCH 5/6] revert --- projects/gateway2/proxy_syncer/cla.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/gateway2/proxy_syncer/cla.go b/projects/gateway2/proxy_syncer/cla.go index 60baf2dcf2f..86961495fcc 100644 --- a/projects/gateway2/proxy_syncer/cla.go +++ b/projects/gateway2/proxy_syncer/cla.go @@ -153,7 +153,7 @@ func PrioritizeEndpoints(logger *zap.Logger, destrule *DestinationRuleWrapper, e return UccWithEndpoints{ Client: ucc, Endpoints: resource.NewEnvoyResource(cla), - EndpointsHash: additionalHash, + EndpointsHash: ep.LbEpsEqualityHash ^ additionalHash, endpointsName: ep.ResourceName(), } } From 03cfffe92c5f3d5e3c49f3f2cd004e47db613e61 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Tue, 26 Nov 2024 09:09:01 -0500 Subject: [PATCH 6/6] Update changelog/v1.18.0-rc3/platform-agnostic-lbephash.yaml --- changelog/v1.18.0-rc3/platform-agnostic-lbephash.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/v1.18.0-rc3/platform-agnostic-lbephash.yaml b/changelog/v1.18.0-rc3/platform-agnostic-lbephash.yaml index 8642963f8aa..049b7fdc8c5 100644 --- a/changelog/v1.18.0-rc3/platform-agnostic-lbephash.yaml +++ b/changelog/v1.18.0-rc3/platform-agnostic-lbephash.yaml @@ -1,6 +1,6 @@ changelog: - type: NON_USER_FACING description: >- - Don't use xor or native endian-ness when generating hashes. + Don't use native endian-ness when generating hashes. This creates difficulty reproducing tests on different machines where the hashes end up in goldenfiles.