Skip to content

Commit

Permalink
Fall back to eds options if needed (#1414) (#1419)
Browse files Browse the repository at this point in the history
  • Loading branch information
khewonc authored Sep 13, 2024
1 parent f046e4a commit f7ef0be
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 15 deletions.
7 changes: 7 additions & 0 deletions api/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"encoding/json"
"math/rand"

"k8s.io/apimachinery/pkg/util/intstr"
"sigs.k8s.io/yaml"
)

Expand Down Expand Up @@ -127,3 +128,9 @@ func YAMLToJSONString(yamlConfigs string) string {
}
return string(jsonValue)
}

// NewIntOrStringPointer converts a string value to an IntOrString pointer
func NewIntOrStringPointer(str string) *intstr.IntOrString {
val := intstr.Parse(str)
return &val
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2016-present Datadog, Inc.

package agent
package utils

import (
"testing"
Expand Down Expand Up @@ -35,7 +35,7 @@ func Test_newIntOrStringPointer(t *testing.T) {
}
for _, tt := range tests {
t.Run("", func(t *testing.T) {
output := newIntOrStringPointer(tt.input)
output := NewIntOrStringPointer(tt.input)
if tt.want != *output {
t.Errorf("newIntOrStringPointer() result is %v but want %v", *output, tt.want)
}
Expand Down
15 changes: 5 additions & 10 deletions internal/controller/datadogagent/component/agent/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ package agent
import (
"time"

apiutils "github.com/DataDog/datadog-operator/api/utils"
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/common"

edsv1alpha1 "github.com/DataDog/extendeddaemonset/api/v1alpha1"
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
)

// NewDaemonset use to generate the skeleton of a new daemonset based on few information
Expand All @@ -34,7 +34,7 @@ func NewDaemonset(owner metav1.Object, edsOptions *ExtendedDaemonsetOptions, com
if edsOptions != nil && edsOptions.MaxPodUnavailable != "" {
daemonset.Spec.UpdateStrategy = appsv1.DaemonSetUpdateStrategy{
RollingUpdate: &appsv1.RollingUpdateDaemonSet{
MaxUnavailable: newIntOrStringPointer(edsOptions.MaxPodUnavailable),
MaxUnavailable: apiutils.NewIntOrStringPointer(edsOptions.MaxPodUnavailable),
},
}
}
Expand Down Expand Up @@ -87,19 +87,19 @@ func defaultEDSSpec(options *ExtendedDaemonsetOptions) edsv1alpha1.ExtendedDaemo
edsv1alpha1.DefaultExtendedDaemonSetSpec(&spec, edsv1alpha1.ExtendedDaemonSetSpecStrategyCanaryValidationModeAuto)

if options.MaxPodUnavailable != "" {
spec.Strategy.RollingUpdate.MaxUnavailable = newIntOrStringPointer(options.MaxPodUnavailable)
spec.Strategy.RollingUpdate.MaxUnavailable = apiutils.NewIntOrStringPointer(options.MaxPodUnavailable)
}

if options.MaxPodSchedulerFailure != "" {
spec.Strategy.RollingUpdate.MaxPodSchedulerFailure = newIntOrStringPointer(options.MaxPodSchedulerFailure)
spec.Strategy.RollingUpdate.MaxPodSchedulerFailure = apiutils.NewIntOrStringPointer(options.MaxPodSchedulerFailure)
}

if options.CanaryDuration != 0 {
spec.Strategy.Canary.Duration = &metav1.Duration{Duration: options.CanaryDuration}
}

if options.CanaryReplicas != "" {
spec.Strategy.Canary.Replicas = newIntOrStringPointer(options.CanaryReplicas)
spec.Strategy.Canary.Replicas = apiutils.NewIntOrStringPointer(options.CanaryReplicas)
}

spec.Strategy.Canary.AutoFail.Enabled = edsv1alpha1.NewBool(options.CanaryAutoFailEnabled)
Expand All @@ -117,8 +117,3 @@ func defaultEDSSpec(options *ExtendedDaemonsetOptions) edsv1alpha1.ExtendedDaemo
}
return spec
}

func newIntOrStringPointer(str string) *intstr.IntOrString {
val := intstr.Parse(str)
return &val
}
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func (r *Reconciler) profilesToApply(ctx context.Context, logger logr.Logger, no

sortedProfiles := agentprofile.SortProfiles(profilesList.Items)
for _, profile := range sortedProfiles {
maxUnavailable := agentprofile.GetMaxUnavailable(logger, dda, &profile, len(nodeList))
maxUnavailable := agentprofile.GetMaxUnavailable(logger, dda, &profile, len(nodeList), &r.options.ExtendedDaemonsetOptions)
profileAppliedByNode, err = agentprofile.ApplyProfile(logger, &profile, nodeList, profileAppliedByNode, now, maxUnavailable)
r.updateDAPStatus(logger, &profile)
if err != nil {
Expand Down
14 changes: 13 additions & 1 deletion pkg/agentprofile/agent_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"github.com/DataDog/datadog-operator/api/datadoghq/common/v1"
"github.com/DataDog/datadog-operator/api/datadoghq/v1alpha1"
"github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
apiutils "github.com/DataDog/datadog-operator/api/utils"
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/component/agent"
"github.com/DataDog/datadog-operator/internal/controller/metrics"
"github.com/DataDog/datadog-operator/pkg/controller/utils/comparison"

Expand Down Expand Up @@ -469,7 +471,7 @@ func SlowStartEnabled() bool {

// GetMaxUnavailable gets the maxUnavailable value as in int.
// Priority is DAP > DDA > Kubernetes default value
func GetMaxUnavailable(logger logr.Logger, dda *v2alpha1.DatadogAgent, profile *v1alpha1.DatadogAgentProfile, numNodes int) int {
func GetMaxUnavailable(logger logr.Logger, dda *v2alpha1.DatadogAgent, profile *v1alpha1.DatadogAgentProfile, numNodes int, edsOptions *agent.ExtendedDaemonsetOptions) int {
// Kubernetes default for DaemonSet MaxUnavailable is 1
// https://github.com/kubernetes/kubernetes/blob/4aca09bc0c45acc69cfdb425d1eea8818eee04d9/pkg/apis/apps/v1/defaults.go#L87
defaultMaxUnavailable := 1
Expand Down Expand Up @@ -500,6 +502,16 @@ func GetMaxUnavailable(logger logr.Logger, dda *v2alpha1.DatadogAgent, profile *
}
}

// maxUnavailable from EDS options
if edsOptions != nil && edsOptions.MaxPodUnavailable != "" {
numToScale, err := intstr.GetScaledValueFromIntOrPercent(apiutils.NewIntOrStringPointer(edsOptions.MaxPodUnavailable), numNodes, true)
if err != nil {
logger.Error(err, "unable to get max unavailable pods from EDS options, defaulting to 1")
return defaultMaxUnavailable
}
return numToScale
}

// k8s default
return defaultMaxUnavailable
}
66 changes: 65 additions & 1 deletion pkg/agentprofile/agent_profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/DataDog/datadog-operator/api/datadoghq/v1alpha1"
"github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
apiutils "github.com/DataDog/datadog-operator/api/utils"
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/component/agent"

"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -784,6 +785,7 @@ func TestGetMaxUnavailable(t *testing.T) {
name string
dda *v2alpha1.DatadogAgent
profile *v1alpha1.DatadogAgentProfile
edsOptions *agent.ExtendedDaemonsetOptions
expectedMaxUnavailable int
}{
{
Expand Down Expand Up @@ -891,14 +893,76 @@ func TestGetMaxUnavailable(t *testing.T) {
},
},
},
edsOptions: &agent.ExtendedDaemonsetOptions{
MaxPodSchedulerFailure: "5",
},
expectedMaxUnavailable: 15,
},
{
name: "empty dda, empty profile, non-empty edsOptions, string value",
dda: &v2alpha1.DatadogAgent{
Spec: v2alpha1.DatadogAgentSpec{
Override: map[v2alpha1.ComponentName]*v2alpha1.DatadogAgentComponentOverride{
v2alpha1.NodeAgentComponentName: {
Name: apiutils.NewStringPointer("test"),
},
},
},
},
profile: &v1alpha1.DatadogAgentProfile{
Spec: v1alpha1.DatadogAgentProfileSpec{
Config: &v1alpha1.Config{
Override: map[v1alpha1.ComponentName]*v1alpha1.Override{
v1alpha1.NodeAgentComponentName: {
PriorityClassName: apiutils.NewStringPointer("test"),
},
},
},
},
},
edsOptions: &agent.ExtendedDaemonsetOptions{
MaxPodUnavailable: "50%",
},
expectedMaxUnavailable: 50,
},
{
name: "non-empty dda, non-empty profile with empty updatestrategy, string value",
dda: &v2alpha1.DatadogAgent{
Spec: v2alpha1.DatadogAgentSpec{
Override: map[v2alpha1.ComponentName]*v2alpha1.DatadogAgentComponentOverride{
v2alpha1.NodeAgentComponentName: {
UpdateStrategy: &common.UpdateStrategy{
Type: "RollingUpdate",
RollingUpdate: &common.RollingUpdate{
MaxUnavailable: &intstr.IntOrString{
Type: intstr.String,
StrVal: "20%",
},
},
},
},
},
},
},
profile: &v1alpha1.DatadogAgentProfile{
Spec: v1alpha1.DatadogAgentProfileSpec{
Config: &v1alpha1.Config{
Override: map[v1alpha1.ComponentName]*v1alpha1.Override{
v1alpha1.NodeAgentComponentName: {
PriorityClassName: apiutils.NewStringPointer("test"),
},
},
},
},
},
expectedMaxUnavailable: 20,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
testLogger := zap.New(zap.UseDevMode(true))
actualMaxUnavailable := GetMaxUnavailable(testLogger, tt.dda, tt.profile, 100)
actualMaxUnavailable := GetMaxUnavailable(testLogger, tt.dda, tt.profile, 100, tt.edsOptions)
assert.Equal(t, tt.expectedMaxUnavailable, actualMaxUnavailable)
})
}
Expand Down

0 comments on commit f7ef0be

Please sign in to comment.