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 
* Add failing test

* Fix daemonset status duplication in DDA when DS can't be created
  • Loading branch information
@levan-m @swang392
levan-m authored and swang392 committed Jan 17, 2025
1 parent 827d9c2 commit 7a18357
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 102 deletions.
138 changes: 56 additions & 82 deletions api/datadoghq/v2alpha1/condition.go
View file
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 v2alpha1
package condition

import (
"fmt"
Expand All @@ -13,6 +13,8 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

edsdatadoghqv1alpha1 "github.com/DataDog/extendeddaemonset/api/v1alpha1"

"github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
)

// DatadogAgentState type representing the deployment state of the different Agent components.
Expand All @@ -31,28 +33,19 @@ const (
DatadogAgentStateFailed DatadogAgentState = "Failed"
)

// UpdateDatadogAgentStatusConditionsFailure used to update the failure StatusConditions
func UpdateDatadogAgentStatusConditionsFailure(status *DatadogAgentStatus, now metav1.Time, conditionType, reason, message string, err error) {
if err != nil {
UpdateDatadogAgentStatusConditions(status, now, conditionType, metav1.ConditionTrue, reason, fmt.Sprintf("msg:%s, err:%v", message, err), true)
} else {
UpdateDatadogAgentStatusConditions(status, now, conditionType, metav1.ConditionFalse, reason, message, true)
}
}

// UpdateDatadogAgentStatusConditions used to update a specific string in conditions
func UpdateDatadogAgentStatusConditions(status *DatadogAgentStatus, now metav1.Time, t string, conditionStatus metav1.ConditionStatus, reason, message string, writeFalseIfNotExist bool) {
func UpdateDatadogAgentStatusConditions(status *v2alpha1.DatadogAgentStatus, now metav1.Time, t string, conditionStatus metav1.ConditionStatus, reason, message string, writeFalseIfNotExist bool) {
idConditionComplete := getIndexForConditionType(status, t)
if idConditionComplete >= 0 {
UpdateDatadogAgentStatusCondition(&status.Conditions[idConditionComplete], now, t, conditionStatus, reason, message)
updateDatadogAgentStatusCondition(&status.Conditions[idConditionComplete], now, conditionStatus, reason, message)
} else if conditionStatus == metav1.ConditionTrue || writeFalseIfNotExist {
// Only add if the condition is True
status.Conditions = append(status.Conditions, NewDatadogAgentStatusCondition(t, conditionStatus, now, reason, message))
status.Conditions = append(status.Conditions, newDatadogAgentStatusCondition(t, conditionStatus, now, reason, message))
}
}

// UpdateDatadogAgentStatusCondition used to update a specific string
func UpdateDatadogAgentStatusCondition(condition *metav1.Condition, now metav1.Time, t string, conditionStatus metav1.ConditionStatus, reason, message string) *metav1.Condition {
// updateDatadogAgentStatusCondition used to update a specific string
func updateDatadogAgentStatusCondition(condition *metav1.Condition, now metav1.Time, conditionStatus metav1.ConditionStatus, reason, message string) *metav1.Condition {
if condition.Status != conditionStatus {
condition.LastTransitionTime = now
condition.Status = conditionStatus
Expand All @@ -63,26 +56,16 @@ func UpdateDatadogAgentStatusCondition(condition *metav1.Condition, now metav1.T
return condition
}

// SetDatadogAgentStatusCondition use to set a condition
func SetDatadogAgentStatusCondition(status *DatadogAgentStatus, condition *metav1.Condition) {
idConditionComplete := getIndexForConditionType(status, condition.Type)
if idConditionComplete >= 0 {
status.Conditions[idConditionComplete] = *condition
} else {
status.Conditions = append(status.Conditions, *condition)
}
}

// DeleteDatadogAgentStatusCondition is used to delete a condition
func DeleteDatadogAgentStatusCondition(status *DatadogAgentStatus, conditionType string) {
func DeleteDatadogAgentStatusCondition(status *v2alpha1.DatadogAgentStatus, conditionType string) {
idConditionComplete := getIndexForConditionType(status, conditionType)
if idConditionComplete >= 0 {
status.Conditions = append(status.Conditions[:idConditionComplete], status.Conditions[idConditionComplete+1:]...)
}
}

// NewDatadogAgentStatusCondition returns new metav1.Condition instance
func NewDatadogAgentStatusCondition(conditionType string, conditionStatus metav1.ConditionStatus, now metav1.Time, reason, message string) metav1.Condition {
// newDatadogAgentStatusCondition returns new metav1.Condition instance
func newDatadogAgentStatusCondition(conditionType string, conditionStatus metav1.ConditionStatus, now metav1.Time, reason, message string) metav1.Condition {
return metav1.Condition{
Type: conditionType,
Status: conditionStatus,
Expand All @@ -100,7 +83,7 @@ func GetMetav1ConditionStatus(status bool) metav1.ConditionStatus {
return metav1.ConditionFalse
}

func getIndexForConditionType(status *DatadogAgentStatus, t string) int {
func getIndexForConditionType(status *v2alpha1.DatadogAgentStatus, t string) int {
idCondition := -1
if status == nil {
return idCondition
Expand All @@ -117,17 +100,17 @@ func getIndexForConditionType(status *DatadogAgentStatus, t string) int {
}

// UpdateDeploymentStatus updates a deployment's DeploymentStatus
func UpdateDeploymentStatus(dep *appsv1.Deployment, depStatus *DeploymentStatus, updateTime *metav1.Time) *DeploymentStatus {
func UpdateDeploymentStatus(dep *appsv1.Deployment, depStatus *v2alpha1.DeploymentStatus, updateTime *metav1.Time) *v2alpha1.DeploymentStatus {
if depStatus == nil {
depStatus = &DeploymentStatus{}
depStatus = &v2alpha1.DeploymentStatus{}
}
if dep == nil {
depStatus.State = string(DatadogAgentStateFailed)
depStatus.Status = string(DatadogAgentStateFailed)
return depStatus
}

if hash, ok := dep.Annotations[MD5AgentDeploymentAnnotationKey]; ok {
if hash, ok := dep.Annotations[v2alpha1.MD5AgentDeploymentAnnotationKey]; ok {
depStatus.CurrentHash = hash
}
if updateTime != nil {
Expand Down Expand Up @@ -165,47 +148,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 []*v2alpha1.DaemonSetStatus, updateTime *metav1.Time) []*v2alpha1.DaemonSetStatus {
if dsStatus == nil {
dsStatus = []*DaemonSetStatus{}
}
if ds == nil {
dsStatus = append(dsStatus, &DaemonSetStatus{
State: string(DatadogAgentStateFailed),
Status: string(DatadogAgentStateFailed),
})
return dsStatus
dsStatus = []*v2alpha1.DaemonSetStatus{}
}

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 v2alpha1.DaemonSetStatus
if ds == nil {
newStatus = v2alpha1.DaemonSetStatus{
State: string(DatadogAgentStateFailed),
Status: string(DatadogAgentStateFailed),
DaemonsetName: dsName,
}
} else {
newStatus = v2alpha1.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[v2alpha1.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 All @@ -222,12 +206,12 @@ func UpdateDaemonSetStatus(ds *appsv1.DaemonSet, dsStatus []*DaemonSetStatus, up
}

// UpdateExtendedDaemonSetStatus updates an ExtendedDaemonSet's DaemonSetStatus
func UpdateExtendedDaemonSetStatus(eds *edsdatadoghqv1alpha1.ExtendedDaemonSet, dsStatus []*DaemonSetStatus, updateTime *metav1.Time) []*DaemonSetStatus {
func UpdateExtendedDaemonSetStatus(eds *edsdatadoghqv1alpha1.ExtendedDaemonSet, dsStatus []*v2alpha1.DaemonSetStatus, updateTime *metav1.Time) []*v2alpha1.DaemonSetStatus {
if dsStatus == nil {
dsStatus = []*DaemonSetStatus{}
dsStatus = []*v2alpha1.DaemonSetStatus{}
}

newStatus := DaemonSetStatus{
newStatus := v2alpha1.DaemonSetStatus{
Desired: eds.Status.Desired,
Current: eds.Status.Current,
Ready: eds.Status.Ready,
Expand All @@ -239,7 +223,7 @@ func UpdateExtendedDaemonSetStatus(eds *edsdatadoghqv1alpha1.ExtendedDaemonSet,
if updateTime != nil {
newStatus.LastUpdate = updateTime
}
if hash, ok := eds.Annotations[MD5AgentDeploymentAnnotationKey]; ok {
if hash, ok := eds.Annotations[v2alpha1.MD5AgentDeploymentAnnotationKey]; ok {
newStatus.CurrentHash = hash
}

Expand Down Expand Up @@ -274,8 +258,8 @@ func UpdateExtendedDaemonSetStatus(eds *edsdatadoghqv1alpha1.ExtendedDaemonSet,
}

// UpdateCombinedDaemonSetStatus combines the status of multiple DaemonSetStatus
func UpdateCombinedDaemonSetStatus(dsStatus []*DaemonSetStatus) *DaemonSetStatus {
combinedStatus := DaemonSetStatus{}
func UpdateCombinedDaemonSetStatus(dsStatus []*v2alpha1.DaemonSetStatus) *v2alpha1.DaemonSetStatus {
combinedStatus := v2alpha1.DaemonSetStatus{}
if len(dsStatus) == 0 {
return &combinedStatus
}
Expand Down Expand Up @@ -328,13 +312,3 @@ func assignNumeralState(state string) int {
return 0
}
}

// DatadogForwarderConditionType type use to represent a Datadog Metrics Forwarder condition.
type DatadogForwarderConditionType string

const (
// DatadogMetricsActive forwarding metrics and events to Datadog is active.
DatadogMetricsActive DatadogForwarderConditionType = "ActiveDatadogMetrics"
// DatadogMetricsError cannot forward deployment metrics and events to Datadog.
DatadogMetricsError DatadogForwarderConditionType = "DatadogMetricsError"
)
11 changes: 11 additions & 0 deletions api/datadoghq/v2alpha1/condition_test.go
View file
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, []*v2alpha1.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))
}
24 changes: 13 additions & 11 deletions internal/controller/datadogagent/controller_reconcile_agent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"context"
"time"

edsv1alpha1 "github.com/DataDog/extendeddaemonset/api/v1alpha1"

apicommon "github.com/DataDog/datadog-operator/api/datadoghq/common"
"github.com/DataDog/datadog-operator/api/datadoghq/v1alpha1"
datadoghqv2alpha1 "github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
Expand All @@ -17,10 +19,10 @@ import (
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/feature"
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/override"
"github.com/DataDog/datadog-operator/pkg/agentprofile"
"github.com/DataDog/datadog-operator/pkg/condition"
"github.com/DataDog/datadog-operator/pkg/constants"
"github.com/DataDog/datadog-operator/pkg/controller/utils/datadog"
"github.com/DataDog/datadog-operator/pkg/kubernetes"
edsv1alpha1 "github.com/DataDog/extendeddaemonset/api/v1alpha1"

"github.com/go-logr/logr"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -107,7 +109,7 @@ func (r *Reconciler) reconcileV2Agent(logger logr.Logger, requiredComponents fea
if disabledByOverride {
if agentEnabled {
// The override supersedes what's set in requiredComponents; update status to reflect the conflict
datadoghqv2alpha1.UpdateDatadogAgentStatusConditions(
condition.UpdateDatadogAgentStatusConditions(
newStatus,
metav1.NewTime(time.Now()),
datadoghqv2alpha1.OverrideReconcileConflictConditionType,
Expand Down Expand Up @@ -182,7 +184,7 @@ func (r *Reconciler) reconcileV2Agent(logger logr.Logger, requiredComponents fea
if disabledByOverride {
if agentEnabled {
// The override supersedes what's set in requiredComponents; update status to reflect the conflict
datadoghqv2alpha1.UpdateDatadogAgentStatusConditions(
condition.UpdateDatadogAgentStatusConditions(
newStatus,
metav1.NewTime(time.Now()),
datadoghqv2alpha1.OverrideReconcileConflictConditionType,
Expand All @@ -202,16 +204,16 @@ 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)
datadoghqv2alpha1.UpdateDatadogAgentStatusConditions(newStatus, updateTime, datadoghqv2alpha1.AgentReconcileConditionType, status, reason, message, true)
newStatus.Agent = datadoghqv2alpha1.UpdateCombinedDaemonSetStatus(newStatus.AgentList)
func updateDSStatusV2WithAgent(dsName string, ds *appsv1.DaemonSet, newStatus *datadoghqv2alpha1.DatadogAgentStatus, updateTime metav1.Time, status metav1.ConditionStatus, reason, message string) {
newStatus.AgentList = condition.UpdateDaemonSetStatus(dsName, ds, newStatus.AgentList, &updateTime)
condition.UpdateDatadogAgentStatusConditions(newStatus, updateTime, datadoghqv2alpha1.AgentReconcileConditionType, status, reason, message, true)
newStatus.Agent = condition.UpdateCombinedDaemonSetStatus(newStatus.AgentList)
}

func updateEDSStatusV2WithAgent(eds *edsv1alpha1.ExtendedDaemonSet, newStatus *datadoghqv2alpha1.DatadogAgentStatus, updateTime metav1.Time, status metav1.ConditionStatus, reason, message string) {
newStatus.AgentList = datadoghqv2alpha1.UpdateExtendedDaemonSetStatus(eds, newStatus.AgentList, &updateTime)
datadoghqv2alpha1.UpdateDatadogAgentStatusConditions(newStatus, updateTime, datadoghqv2alpha1.AgentReconcileConditionType, status, reason, message, true)
newStatus.Agent = datadoghqv2alpha1.UpdateCombinedDaemonSetStatus(newStatus.AgentList)
newStatus.AgentList = condition.UpdateExtendedDaemonSetStatus(eds, newStatus.AgentList, &updateTime)
condition.UpdateDatadogAgentStatusConditions(newStatus, updateTime, datadoghqv2alpha1.AgentReconcileConditionType, status, reason, message, true)
newStatus.Agent = condition.UpdateCombinedDaemonSetStatus(newStatus.AgentList)
}

func (r *Reconciler) deleteV2DaemonSet(logger logr.Logger, dda *datadoghqv2alpha1.DatadogAgent, ds *appsv1.DaemonSet, newStatus *datadoghqv2alpha1.DatadogAgentStatus) error {
Expand Down Expand Up @@ -242,7 +244,7 @@ func (r *Reconciler) deleteV2ExtendedDaemonSet(logger logr.Logger, dda *datadogh

func deleteStatusWithAgent(newStatus *datadoghqv2alpha1.DatadogAgentStatus) {
newStatus.Agent = nil
datadoghqv2alpha1.DeleteDatadogAgentStatusCondition(newStatus, datadoghqv2alpha1.AgentReconcileConditionType)
condition.DeleteDatadogAgentStatusCondition(newStatus, datadoghqv2alpha1.AgentReconcileConditionType)
}

// removeStaleStatus removes a DaemonSet's status from a DatadogAgent's
Expand Down
Loading

0 comments on commit 7a18357

Please sign in to comment.