Skip to content

Commit

Permalink
Fix daemonset status duplication in DDA when DS can't be created (#1629
Browse files Browse the repository at this point in the history
…) (#1632)

* Fix daemonset status duplication in DDA when DS can't be created (#1629)

* Add failing test

* Fix daemonset status duplication in DDA when DS can't be created

* add test file
  • Loading branch information
levan-m authored Jan 17, 2025
1 parent c54912e commit a715d8c
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 41 deletions.
67 changes: 34 additions & 33 deletions api/datadoghq/v2alpha1/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,47 +165,48 @@ func UpdateDeploymentStatus(dep *appsv1.Deployment, depStatus *DeploymentStatus,
}

// UpdateDaemonSetStatus updates a daemonset's DaemonSetStatus
func UpdateDaemonSetStatus(ds *appsv1.DaemonSet, dsStatus []*DaemonSetStatus, updateTime *metav1.Time) []*DaemonSetStatus {
func UpdateDaemonSetStatus(dsName string, ds *appsv1.DaemonSet, dsStatus []*DaemonSetStatus, updateTime *metav1.Time) []*DaemonSetStatus {
if dsStatus == nil {
dsStatus = []*DaemonSetStatus{}
}
if ds == nil {
dsStatus = append(dsStatus, &DaemonSetStatus{
State: string(DatadogAgentStateFailed),
Status: string(DatadogAgentStateFailed),
})
return dsStatus
}

newStatus := DaemonSetStatus{
Desired: ds.Status.DesiredNumberScheduled,
Current: ds.Status.CurrentNumberScheduled,
Ready: ds.Status.NumberReady,
Available: ds.Status.NumberAvailable,
UpToDate: ds.Status.UpdatedNumberScheduled,
DaemonsetName: ds.ObjectMeta.Name,
}
var newStatus DaemonSetStatus
if ds == nil {
newStatus = DaemonSetStatus{
State: string(DatadogAgentStateFailed),
Status: string(DatadogAgentStateFailed),
DaemonsetName: dsName,
}
} else {
newStatus = DaemonSetStatus{
Desired: ds.Status.DesiredNumberScheduled,
Current: ds.Status.CurrentNumberScheduled,
Ready: ds.Status.NumberReady,
Available: ds.Status.NumberAvailable,
UpToDate: ds.Status.UpdatedNumberScheduled,
DaemonsetName: ds.ObjectMeta.Name,
}
if updateTime != nil {
newStatus.LastUpdate = updateTime
}
if hash, ok := ds.Annotations[MD5AgentDeploymentAnnotationKey]; ok {
newStatus.CurrentHash = hash
}

if updateTime != nil {
newStatus.LastUpdate = updateTime
}
if hash, ok := ds.Annotations[MD5AgentDeploymentAnnotationKey]; ok {
newStatus.CurrentHash = hash
}
var deploymentState DatadogAgentState
switch {
case newStatus.UpToDate != newStatus.Desired:
deploymentState = DatadogAgentStateUpdating
case newStatus.Ready == 0 && newStatus.Desired != 0:
deploymentState = DatadogAgentStateProgressing
default:
deploymentState = DatadogAgentStateRunning
}

var deploymentState DatadogAgentState
switch {
case newStatus.UpToDate != newStatus.Desired:
deploymentState = DatadogAgentStateUpdating
case newStatus.Ready == 0 && newStatus.Desired != 0:
deploymentState = DatadogAgentStateProgressing
default:
deploymentState = DatadogAgentStateRunning
newStatus.State = fmt.Sprintf("%v", deploymentState)
newStatus.Status = fmt.Sprintf("%v (%d/%d/%d)", deploymentState, newStatus.Desired, newStatus.Ready, newStatus.UpToDate)
}

newStatus.State = fmt.Sprintf("%v", deploymentState)
newStatus.Status = fmt.Sprintf("%v (%d/%d/%d)", deploymentState, newStatus.Desired, newStatus.Ready, newStatus.UpToDate)

// match ds name to ds status
found := false
for id := range dsStatus {
Expand Down
11 changes: 11 additions & 0 deletions api/datadoghq/v2alpha1/condition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ package v2alpha1

import (
"testing"
"time"

apiutils "github.com/DataDog/datadog-operator/api/utils"
"github.com/google/go-cmp/cmp"
assert "github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -106,3 +109,11 @@ func TestDeleteDatadogAgentStatusCondition(t *testing.T) {
})
}
}

func TestUpdateWhenDSNil(t *testing.T) {
var ds *appsv1.DaemonSet
dsStatus := UpdateDaemonSetStatus("ds", ds, []*DaemonSetStatus{}, &metav1.Time{Time: time.Now()})
dsStatus = UpdateDaemonSetStatus("ds", ds, dsStatus, &metav1.Time{Time: time.Now()})
dsStatus = UpdateDaemonSetStatus("ds", ds, dsStatus, &metav1.Time{Time: time.Now()})
assert.Equal(t, 1, len(dsStatus))
}
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ func (r *Reconciler) reconcileV2Agent(logger logr.Logger, requiredComponents fea
return r.createOrUpdateDaemonset(daemonsetLogger, dda, daemonset, newStatus, updateDSStatusV2WithAgent, profile)
}

func updateDSStatusV2WithAgent(ds *appsv1.DaemonSet, newStatus *datadoghqv2alpha1.DatadogAgentStatus, updateTime metav1.Time, status metav1.ConditionStatus, reason, message string) {
newStatus.AgentList = datadoghqv2alpha1.UpdateDaemonSetStatus(ds, newStatus.AgentList, &updateTime)
func updateDSStatusV2WithAgent(dsName string, ds *appsv1.DaemonSet, newStatus *datadoghqv2alpha1.DatadogAgentStatus, updateTime metav1.Time, status metav1.ConditionStatus, reason, message string) {
newStatus.AgentList = datadoghqv2alpha1.UpdateDaemonSetStatus(dsName, ds, newStatus.AgentList, &updateTime)
datadoghqv2alpha1.UpdateDatadogAgentStatusConditions(newStatus, updateTime, datadoghqv2alpha1.AgentReconcileConditionType, status, reason, message, true)
newStatus.Agent = datadoghqv2alpha1.UpdateCombinedDaemonSetStatus(newStatus.AgentList)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const (
)

type updateDepStatusComponentFunc func(deployment *appsv1.Deployment, newStatus *datadoghqv2alpha1.DatadogAgentStatus, updateTime metav1.Time, status metav1.ConditionStatus, reason, message string)
type updateDSStatusComponentFunc func(daemonset *appsv1.DaemonSet, newStatus *datadoghqv2alpha1.DatadogAgentStatus, updateTime metav1.Time, status metav1.ConditionStatus, reason, message string)
type updateDSStatusComponentFunc func(daemonsetName string, daemonset *appsv1.DaemonSet, newStatus *datadoghqv2alpha1.DatadogAgentStatus, updateTime metav1.Time, status metav1.ConditionStatus, reason, message string)
type updateEDSStatusComponentFunc func(eds *edsv1alpha1.ExtendedDaemonSet, newStatus *datadoghqv2alpha1.DatadogAgentStatus, updateTime metav1.Time, status metav1.ConditionStatus, reason, message string)

func (r *Reconciler) createOrUpdateDeployment(parentLogger logr.Logger, dda *datadoghqv2alpha1.DatadogAgent, deployment *appsv1.Deployment, newStatus *datadoghqv2alpha1.DatadogAgentStatus, updateStatusFunc updateDepStatusComponentFunc) (reconcile.Result, error) {
Expand Down Expand Up @@ -218,7 +218,7 @@ func (r *Reconciler) createOrUpdateDaemonset(parentLogger logr.Logger, dda *data
// Even if the DaemonSet is still the same, its status might have
// changed (for example, the number of pods ready). This call is
// needed to keep the agent status updated.
newStatus.AgentList = datadoghqv2alpha1.UpdateDaemonSetStatus(currentDaemonset, newStatus.AgentList, &now)
newStatus.AgentList = datadoghqv2alpha1.UpdateDaemonSetStatus(currentDaemonset.Name, currentDaemonset, newStatus.AgentList, &now)
newStatus.Agent = datadoghqv2alpha1.UpdateCombinedDaemonSetStatus(newStatus.AgentList)

// Stop reconcile loop since DaemonSet hasn't changed
Expand All @@ -242,12 +242,12 @@ func (r *Reconciler) createOrUpdateDaemonset(parentLogger logr.Logger, dda *data

err = kubernetes.UpdateFromObject(context.TODO(), r.client, updateDaemonset, currentDaemonset.ObjectMeta)
if err != nil {
updateStatusFunc(updateDaemonset, newStatus, now, metav1.ConditionFalse, updateSucceeded, "Unable to update Daemonset")
updateStatusFunc(updateDaemonset.Name, updateDaemonset, newStatus, now, metav1.ConditionFalse, updateSucceeded, "Unable to update Daemonset")
return reconcile.Result{}, err
}
event := buildEventInfo(updateDaemonset.Name, updateDaemonset.Namespace, kubernetes.DaemonSetKind, datadog.UpdateEvent)
r.recordEvent(dda, event)
updateStatusFunc(updateDaemonset, newStatus, now, metav1.ConditionTrue, updateSucceeded, "Daemonset updated")
updateStatusFunc(updateDaemonset.Name, updateDaemonset, newStatus, now, metav1.ConditionTrue, updateSucceeded, "Daemonset updated")
} else {
// From here the PodTemplateSpec should be ready, we can generate the hash that will be added to this daemonset.
_, err = comparison.SetMD5DatadogAgentGenerationAnnotation(&daemonset.ObjectMeta, daemonset.Spec)
Expand All @@ -259,12 +259,12 @@ func (r *Reconciler) createOrUpdateDaemonset(parentLogger logr.Logger, dda *data

err = r.client.Create(context.TODO(), daemonset)
if err != nil {
updateStatusFunc(nil, newStatus, now, metav1.ConditionFalse, createSucceeded, "Unable to create Daemonset")
updateStatusFunc(daemonset.Name, nil, newStatus, now, metav1.ConditionFalse, createSucceeded, "Unable to create Daemonset")
return reconcile.Result{}, err
}
event := buildEventInfo(daemonset.Name, daemonset.Namespace, kubernetes.DaemonSetKind, datadog.CreationEvent)
r.recordEvent(dda, event)
updateStatusFunc(daemonset, newStatus, now, metav1.ConditionTrue, createSucceeded, "Daemonset created")
updateStatusFunc(daemonset.Name, daemonset, newStatus, now, metav1.ConditionTrue, createSucceeded, "Daemonset created")
}

logger.Info("Creating Daemonset")
Expand Down

0 comments on commit a715d8c

Please sign in to comment.