scheduler: eliminate more uses of reflect

lib/lang/opaque.go

@@ -10,12 +10,16 @@ import (
 )
 
 var (
-	cmpOptIgnorePrivate = ignoreUnexportedAlways()
-	cmpOptNilIsEmpty    = cmpopts.EquateEmpty()
+	cmpOptIgnoreUnexported = ignoreUnexportedAlways()
+	cmpOptNilIsEmpty       = cmpopts.EquateEmpty()
+	cmpOptIgnore           = cmp.Ignore()
 )
 
-// ignoreUnexportedAlways is a derivative of go-cmp.IgnoreUnexported, but this one
-// will always ignore unexported types, recursively.
+// ignoreUnexportedAlways creates a cmp.Option filter that will ignore unexported
+// fields of on any/all types. It is a derivative of go-cmp.IgnoreUnexported,
+// here we do not require specifying individual types.
+//
+// reference: https://github.com/google/go-cmp/blob/master/cmp/cmpopts/ignore.go#L110
 func ignoreUnexportedAlways() cmp.Option {
 	return cmp.FilterPath(
 		func(p cmp.Path) bool {
@@ -26,18 +30,22 @@ func ignoreUnexportedAlways() cmp.Option {
 			r, _ := utf8.DecodeRuneInString(sf.Name())
 			return !unicode.IsUpper(r)
 		},
-		cmp.Ignore(),
+		cmpOptIgnore,
 	)
 }
 
 // OpaqueMapsEqual compare maps[<comparable>]<any> for equality, but safely by
 // using the cmp package and ignoring un-exported types, and by treating nil/empty
 // slices and maps as equal.
+//
+// This is intended as a substitute for reflect.DeepEqual in the case of "opaque maps",
+// e.g. `map[comparable]any` - such as the case for Task Driver config or Envoy proxy
+// pass-through configuration.
 func OpaqueMapsEqual[M ~map[K]V, K comparable, V any](m1, m2 M) bool {
 	return maps.EqualFunc(m1, m2, func(a, b V) bool {
 		return cmp.Equal(a, b,
-			cmpOptIgnorePrivate, // ignore all private fields
-			cmpOptNilIsEmpty,    // nil/empty slices treated as equal
+			cmpOptIgnoreUnexported, // ignore all unexported fields
+			cmpOptNilIsEmpty,       // treat nil/empty slices as equal
 		)
 	})
 }

lib/lang/opaque_test.go

@@ -11,16 +11,16 @@ func Test_OpaqueMapsEqual(t *testing.T) {
 	ci.Parallel(t)
 
 	type public struct {
-		A int
+		X int
 	}
 
 	type private struct {
-		a int
+		y int
 	}
 
 	type mix struct {
-		A int
-		b int
+		X int
+		y int
 	}
 
 	cases := []struct {
@@ -44,33 +44,38 @@ func Test_OpaqueMapsEqual(t *testing.T) {
 		exp:  true,
 	}, {
 		name: "same public struct",
-		a:    map[string]any{"a": &public{A: 42}},
-		b:    map[string]any{"a": &public{A: 42}},
+		a:    map[string]any{"a": &public{X: 42}},
+		b:    map[string]any{"a": &public{X: 42}},
 		exp:  true,
 	}, {
 		name: "different public struct",
-		a:    map[string]any{"a": &public{A: 42}},
-		b:    map[string]any{"a": &public{A: 10}},
+		a:    map[string]any{"a": &public{X: 42}},
+		b:    map[string]any{"a": &public{X: 10}},
 		exp:  false,
 	}, {
 		name: "different private struct",
-		a:    map[string]any{"a": &private{a: 42}},
-		b:    map[string]any{"a": &private{a: 10}},
+		a:    map[string]any{"a": &private{y: 42}},
+		b:    map[string]any{"a": &private{y: 10}},
 		exp:  true, // private fields not compared
 	}, {
 		name: "mix same public different private",
-		a:    map[string]any{"a": &mix{A: 42, b: 1}},
-		b:    map[string]any{"a": &mix{A: 42, b: 2}},
+		a:    map[string]any{"a": &mix{X: 42, y: 1}},
+		b:    map[string]any{"a": &mix{X: 42, y: 2}},
 		exp:  true, // private fields not compared
 	}, {
 		name: "mix different public same private",
-		a:    map[string]any{"a": &mix{A: 42, b: 1}},
-		b:    map[string]any{"a": &mix{A: 10, b: 1}},
+		a:    map[string]any{"a": &mix{X: 42, y: 1}},
+		b:    map[string]any{"a": &mix{X: 10, y: 1}},
 		exp:  false,
 	}, {
-		name: "nil empty slice values",
-		a:    map[string]any{"a": []string(nil)},
-		b:    map[string]any{"a": make([]string, 0)},
+		name: "nil vs empty slice values",
+		a:    map[string]any{"key": []string(nil)},
+		b:    map[string]any{"key": make([]string, 0)},
+		exp:  true,
+	}, {
+		name: "nil vs empty map values",
+		a:    map[string]any{"key": map[int]int(nil)},
+		b:    map[string]any{"key": make(map[int]int, 0)},
 		exp:  true,
 	}}
 

nomad/structs/services.go

@@ -21,6 +21,7 @@ import (
 	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/helper/args"
 	"github.com/hashicorp/nomad/helper/pointer"
+	"github.com/hashicorp/nomad/lib/lang"
 	"github.com/mitchellh/copystructure"
 	"golang.org/x/exp/maps"
 	"golang.org/x/exp/slices"
@@ -1202,7 +1203,7 @@ func (t *SidecarTask) Equal(o *SidecarTask) bool {
 	}
 
 	// config compare
-	if !opaqueMapsEqual(t.Config, o.Config) {
+	if !lang.OpaqueMapsEqual(t.Config, o.Config) {
 		return false
 	}
 
@@ -1378,15 +1379,6 @@ func (p *ConsulProxy) Copy() *ConsulProxy {
 	}
 }
 
-// opaqueMapsEqual compares map[string]interface{} commonly used for opaque
-// config blocks. Interprets nil and {} as the same.
-func opaqueMapsEqual(a, b map[string]interface{}) bool {
-	if len(a) == 0 && len(b) == 0 {
-		return true
-	}
-	return reflect.DeepEqual(a, b)
-}
-
 // Equal returns true if the structs are recursively equal.
 func (p *ConsulProxy) Equal(o *ConsulProxy) bool {
 	if p == nil || o == nil {
@@ -1409,7 +1401,7 @@ func (p *ConsulProxy) Equal(o *ConsulProxy) bool {
 		return false
 	}
 
-	if !opaqueMapsEqual(p.Config, o.Config) {
+	if !lang.OpaqueMapsEqual(p.Config, o.Config) {
 		return false
 	}
 
@@ -1504,7 +1496,7 @@ func (u *ConsulUpstream) Equal(o *ConsulUpstream) bool {
 		return false
 	case !u.MeshGateway.Equal(o.MeshGateway):
 		return false
-	case !opaqueMapsEqual(u.Config, o.Config):
+	case !lang.OpaqueMapsEqual(u.Config, o.Config):
 		return false
 	}
 	return true
@@ -1790,7 +1782,7 @@ func (p *ConsulGatewayProxy) Equal(o *ConsulGatewayProxy) bool {
 		return false
 	}
 
-	if !opaqueMapsEqual(p.Config, o.Config) {
+	if !lang.OpaqueMapsEqual(p.Config, o.Config) {
 		return false
 	}
 

scheduler/util.go

@@ -422,9 +422,9 @@ func connectSidecarServiceUpdated(ssA, ssB *structs.ConsulSidecarService) compar
 		return difference("connect port", ssA.Port, ssB.Port)
 	}
 
-	// sidecar_service.tags handled in-place (registration)
+	// sidecar_service.tags (handled in-place via registration)
 
-	// sidecar_service.proxy handled in-place (registration + xDS)
+	// sidecar_service.proxy (handled in-place via registration + xDS)
 
 	return same
 }