scheduler: annotate tasksUpdated with reason and purge DeepEquals

lib/lang/opaque.go

@@ -0,0 +1,48 @@
+package lang
+
+import (
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"golang.org/x/exp/maps"
+)
+
+var (
+	cmpOptIgnoreUnexported = ignoreUnexportedAlways()
+	cmpOptNilIsEmpty       = cmpopts.EquateEmpty()
+	cmpOptIgnore           = cmp.Ignore()
+)
+
+// 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 {
+			sf, ok := p.Index(-1).(cmp.StructField)
+			if !ok {
+				return false
+			}
+			c := sf.Name()[0]
+			return c < 'A' || c > 'Z'
+		},
+		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,
+			cmpOptIgnoreUnexported, // ignore all unexported fields
+			cmpOptNilIsEmpty,       // treat nil/empty slices as equal
+		)
+	})
+}

lib/lang/opaque_test.go

@@ -0,0 +1,88 @@
+package lang
+
+import (
+	"testing"
+
+	"github.com/hashicorp/nomad/ci"
+	"github.com/shoenig/test/must"
+)
+
+func Test_OpaqueMapsEqual(t *testing.T) {
+	ci.Parallel(t)
+
+	type public struct {
+		F int
+	}
+
+	type private struct {
+		g int
+	}
+
+	type mix struct {
+		F int
+		g int
+	}
+
+	cases := []struct {
+		name string
+		a, b map[string]any
+		exp  bool
+	}{{
+		name: "both nil",
+		a:    nil,
+		b:    nil,
+		exp:  true,
+	}, {
+		name: "empty and nil",
+		a:    nil,
+		b:    make(map[string]any),
+		exp:  true,
+	}, {
+		name: "same strings",
+		a:    map[string]any{"a": "A"},
+		b:    map[string]any{"a": "A"},
+		exp:  true,
+	}, {
+		name: "same public struct",
+		a:    map[string]any{"a": &public{F: 42}},
+		b:    map[string]any{"a": &public{F: 42}},
+		exp:  true,
+	}, {
+		name: "different public struct",
+		a:    map[string]any{"a": &public{F: 42}},
+		b:    map[string]any{"a": &public{F: 10}},
+		exp:  false,
+	}, {
+		name: "different private struct",
+		a:    map[string]any{"a": &private{g: 42}},
+		b:    map[string]any{"a": &private{g: 10}},
+		exp:  true, // private fields not compared
+	}, {
+		name: "mix same public different private",
+		a:    map[string]any{"a": &mix{F: 42, g: 1}},
+		b:    map[string]any{"a": &mix{F: 42, g: 2}},
+		exp:  true, // private fields not compared
+	}, {
+		name: "mix different public same private",
+		a:    map[string]any{"a": &mix{F: 42, g: 1}},
+		b:    map[string]any{"a": &mix{F: 10, g: 1}},
+		exp:  false,
+	}, {
+		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,
+	}}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := OpaqueMapsEqual(tc.a, tc.b)
+			must.Eq(t, tc.exp, result)
+		})
+	}
+}

nomad/structs/csi.go

@@ -80,6 +80,25 @@ type TaskCSIPluginConfig struct {
 	HealthTimeout time.Duration `mapstructure:"health_timeout" hcl:"health_timeout,optional"`
 }
 
+func (t *TaskCSIPluginConfig) Equal(o *TaskCSIPluginConfig) bool {
+	if t == nil || o == nil {
+		return t == o
+	}
+	switch {
+	case t.ID != o.ID:
+		return false
+	case t.Type != o.Type:
+		return false
+	case t.MountDir != o.MountDir:
+		return false
+	case t.StagePublishBaseDir != o.StagePublishBaseDir:
+		return false
+	case t.HealthTimeout != o.HealthTimeout:
+		return false
+	}
+	return true
+}
+
 func (t *TaskCSIPluginConfig) Copy() *TaskCSIPluginConfig {
 	if t == nil {
 		return nil

nomad/structs/csi_test.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/nomad/ci"
+	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/require"
 )
 
@@ -1033,3 +1034,33 @@ func TestDeleteNodeForType_Monolith_NilNode(t *testing.T) {
 	_, ok = plug.Controllers["foo"]
 	require.False(t, ok)
 }
+
+func TestTaskCSIPluginConfig_Equal(t *testing.T) {
+	ci.Parallel(t)
+
+	must.Equal[*TaskCSIPluginConfig](t, nil, nil)
+	must.NotEqual[*TaskCSIPluginConfig](t, nil, new(TaskCSIPluginConfig))
+
+	must.StructEqual(t, &TaskCSIPluginConfig{
+		ID:                  "abc123",
+		Type:                CSIPluginTypeMonolith,
+		MountDir:            "/opt/csi/mount",
+		StagePublishBaseDir: "/base",
+		HealthTimeout:       42 * time.Second,
+	}, []must.Tweak[*TaskCSIPluginConfig]{{
+		Field: "ID",
+		Apply: func(c *TaskCSIPluginConfig) { c.ID = "def345" },
+	}, {
+		Field: "Type",
+		Apply: func(c *TaskCSIPluginConfig) { c.Type = CSIPluginTypeNode },
+	}, {
+		Field: "MountDir",
+		Apply: func(c *TaskCSIPluginConfig) { c.MountDir = "/csi" },
+	}, {
+		Field: "StagePublishBaseDir",
+		Apply: func(c *TaskCSIPluginConfig) { c.StagePublishBaseDir = "/opt/base" },
+	}, {
+		Field: "HealthTimeout",
+		Apply: func(c *TaskCSIPluginConfig) { c.HealthTimeout = 1 * time.Second },
+	}})
+}

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
 	}