From 6c9bcc6dcfcb8d1a01cce5d095182c8b1a91ec01 Mon Sep 17 00:00:00 2001 From: Anton Troshin Date: Wed, 26 Feb 2025 12:08:12 -0600 Subject: [PATCH] Fix dapr upgrade command incorrectly detecting HA mode for new version 1.15 (#1494) * Fix dapr upgrade command detecting HA mode for new version 1.15 The issue is that the scheduler by default uses 3 replicas, which incorrectly identified non-HA install as HA. Signed-off-by: Anton Troshin * Fix e2e Signed-off-by: Anton Troshin --------- Signed-off-by: Anton Troshin --- pkg/kubernetes/upgrade.go | 6 +++ pkg/kubernetes/upgrade_test.go | 72 ++++++++++++++++++++++++++++++++++ tests/e2e/common/common.go | 27 ++++++++++--- 3 files changed, 100 insertions(+), 5 deletions(-) diff --git a/pkg/kubernetes/upgrade.go b/pkg/kubernetes/upgrade.go index eab2b341..a1505c67 100644 --- a/pkg/kubernetes/upgrade.go +++ b/pkg/kubernetes/upgrade.go @@ -17,6 +17,7 @@ import ( "fmt" "net/http" "os" + "strings" "time" helm "helm.sh/helm/v3/pkg/action" @@ -255,6 +256,11 @@ func highAvailabilityEnabled(status []StatusOutput) bool { if s.Name == "dapr-dashboard" { continue } + // Skip the scheduler server because it's in HA mode by default since version 1.15.0 + // This will fall back to other dapr services to determine if HA mode is enabled. + if strings.HasPrefix(s.Name, "dapr-scheduler-server") { + continue + } if s.Replicas > 1 { return true } diff --git a/pkg/kubernetes/upgrade_test.go b/pkg/kubernetes/upgrade_test.go index b56a5056..70e1a165 100644 --- a/pkg/kubernetes/upgrade_test.go +++ b/pkg/kubernetes/upgrade_test.go @@ -31,6 +31,38 @@ func TestHAMode(t *testing.T) { assert.True(t, r) }) + t.Run("ha mode with scheduler and other services", func(t *testing.T) { + s := []StatusOutput{ + { + Name: "dapr-scheduler-server", + Replicas: 3, + }, + { + Name: "dapr-placement-server", + Replicas: 3, + }, + } + + r := highAvailabilityEnabled(s) + assert.True(t, r) + }) + + t.Run("non-ha mode with only scheduler image variant", func(t *testing.T) { + s := []StatusOutput{ + { + Name: "dapr-scheduler-server-mariner", + Replicas: 3, + }, + { + Name: "dapr-placement-server-mariner", + Replicas: 3, + }, + } + + r := highAvailabilityEnabled(s) + assert.True(t, r) + }) + t.Run("non-ha mode", func(t *testing.T) { s := []StatusOutput{ { @@ -41,6 +73,46 @@ func TestHAMode(t *testing.T) { r := highAvailabilityEnabled(s) assert.False(t, r) }) + + t.Run("non-ha mode with scheduler and other services", func(t *testing.T) { + s := []StatusOutput{ + { + Name: "dapr-scheduler-server", + Replicas: 3, + }, + { + Name: "dapr-placement-server", + Replicas: 1, + }, + } + + r := highAvailabilityEnabled(s) + assert.False(t, r) + }) + + t.Run("non-ha mode with only scheduler", func(t *testing.T) { + s := []StatusOutput{ + { + Name: "dapr-scheduler-server", + Replicas: 3, + }, + } + + r := highAvailabilityEnabled(s) + assert.False(t, r) + }) + + t.Run("non-ha mode with only scheduler image variant", func(t *testing.T) { + s := []StatusOutput{ + { + Name: "dapr-scheduler-server-mariner", + Replicas: 3, + }, + } + + r := highAvailabilityEnabled(s) + assert.False(t, r) + }) } func TestMTLSChartValues(t *testing.T) { diff --git a/tests/e2e/common/common.go b/tests/e2e/common/common.go index e609b0a6..1dccc090 100644 --- a/tests/e2e/common/common.go +++ b/tests/e2e/common/common.go @@ -368,6 +368,12 @@ func StatusTestOnInstallUpgrade(details VersionDetails, opts TestOptions) func(t daprPath := GetDaprPath() output, err := spawn.Command(daprPath, "status", "-k") require.NoError(t, err, "status check failed") + + version, err := semver.NewVersion(details.RuntimeVersion) + if err != nil { + t.Error("failed to parse runtime version", err) + } + var notFound map[string][]string if !opts.HAEnabled { notFound = map[string][]string{ @@ -377,6 +383,11 @@ func StatusTestOnInstallUpgrade(details VersionDetails, opts TestOptions) func(t "dapr-placement-server": {details.RuntimeVersion, "1"}, "dapr-operator": {details.RuntimeVersion, "1"}, } + if version.GreaterThanEqual(VersionWithHAScheduler) { + notFound["dapr-scheduler-server"] = []string{details.RuntimeVersion, "3"} + } else if version.GreaterThanEqual(VersionWithScheduler) { + notFound["dapr-scheduler-server"] = []string{details.RuntimeVersion, "1"} + } } else { notFound = map[string][]string{ "dapr-sentry": {details.RuntimeVersion, "3"}, @@ -385,6 +396,9 @@ func StatusTestOnInstallUpgrade(details VersionDetails, opts TestOptions) func(t "dapr-placement-server": {details.RuntimeVersion, "3"}, "dapr-operator": {details.RuntimeVersion, "3"}, } + if version.GreaterThanEqual(VersionWithScheduler) { + notFound["dapr-scheduler-server"] = []string{details.RuntimeVersion, "3"} + } } if details.ImageVariant != "" { @@ -392,6 +406,9 @@ func StatusTestOnInstallUpgrade(details VersionDetails, opts TestOptions) func(t notFound["dapr-sidecar-injector"][0] = notFound["dapr-sidecar-injector"][0] + "-" + details.ImageVariant notFound["dapr-placement-server"][0] = notFound["dapr-placement-server"][0] + "-" + details.ImageVariant notFound["dapr-operator"][0] = notFound["dapr-operator"][0] + "-" + details.ImageVariant + if notFound["dapr-scheduler-server"] != nil { + notFound["dapr-scheduler-server"][0] = notFound["dapr-scheduler-server"][0] + "-" + details.ImageVariant + } } lines := strings.Split(output, "\n")[1:] // remove header of status. @@ -400,13 +417,13 @@ func StatusTestOnInstallUpgrade(details VersionDetails, opts TestOptions) func(t cols := strings.Fields(strings.TrimSpace(line)) if len(cols) > 6 { // atleast 6 fields are verified from status (Age and created time are not). if toVerify, ok := notFound[cols[0]]; ok { // get by name. - require.Equal(t, DaprTestNamespace, cols[1], "namespace must match") - require.Equal(t, "True", cols[2], "healthly field must be true") - require.Equal(t, "Running", cols[3], "pods must be Running") - require.Equal(t, toVerify[1], cols[4], "replicas must be equal") + require.Equal(t, DaprTestNamespace, cols[1], "%s namespace must match", cols[0]) + require.Equal(t, "True", cols[2], "%s healthy field must be true", cols[0]) + require.Equal(t, "Running", cols[3], "%s pods must be Running", cols[0]) + require.Equal(t, toVerify[1], cols[4], "%s replicas must be equal", cols[0]) // TODO: Skip the dashboard version check for now until the helm chart is updated. if cols[0] != "dapr-dashboard" { - require.Equal(t, toVerify[0], cols[5], "versions must match") + require.Equal(t, toVerify[0], cols[5], "%s versions must match", cols[0]) } delete(notFound, cols[0]) }