Skip to content

Commit

Permalink
Fix dapr upgrade command incorrectly detecting HA mode for new versio…
Browse files Browse the repository at this point in the history
…n 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 <anton@diagrid.io>

* Fix e2e

Signed-off-by: Anton Troshin <anton@diagrid.io>

---------

Signed-off-by: Anton Troshin <anton@diagrid.io>
  • Loading branch information
antontroshin authored Feb 26, 2025
1 parent bd09c94 commit 6c9bcc6
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 5 deletions.
6 changes: 6 additions & 0 deletions pkg/kubernetes/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"fmt"
"net/http"
"os"
"strings"
"time"

helm "helm.sh/helm/v3/pkg/action"
Expand Down Expand Up @@ -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
}
Expand Down
72 changes: 72 additions & 0 deletions pkg/kubernetes/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand All @@ -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) {
Expand Down
27 changes: 22 additions & 5 deletions tests/e2e/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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"},
Expand All @@ -385,13 +396,19 @@ 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 != "" {
notFound["dapr-sentry"][0] = notFound["dapr-sentry"][0] + "-" + details.ImageVariant
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.
Expand All @@ -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])
}
Expand Down

0 comments on commit 6c9bcc6

Please sign in to comment.