Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add EDS Support #591

Merged
merged 2 commits into from
Aug 11, 2022
Merged

Add EDS Support #591

merged 2 commits into from
Aug 11, 2022

Conversation

khewonc
Copy link
Contributor

@khewonc khewonc commented Aug 5, 2022

What does this PR do?

Adds EDS support to v2 reconciler

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Describe your test plan

> kubectl get pods
NAME                                             READY   STATUS    RESTARTS   AGE
datadog-agent-xxxxx-xxxxx                        2/2     Running   0          5m26s
datadog-cluster-agent-xxxxxxxxxx-xxxxx           1/1     Running   0          5m33s
datadog-cluster-checks-runner-xxxxxxxxxx-xxxxx   1/1     Running   0          5m34s
datadog-operator-manager-xxxxxxxxxx-xxxxx        1/1     Running   0          7m
eds-extendeddaemonset-xxxxxxxxx-xxxxx            1/1     Running   0          2d
> kubectl get ers
NAME                  STATUS   DESIRED   CURRENT   READY   AVAILABLE   IGNORED UNRESPONSIVE NODES   NODE SELECTOR                                                                                    AGE
datadog-agent-xxxxx   active   1         1         1       1           0                            {"matchLabels":{"agent.datadoghq.com/component":"agent","agent.datadoghq.com/name":"datadog"}}   5m40s
> kubectl get eds
NAME            DESIRED   CURRENT   READY   UP-TO-DATE   AVAILABLE   IGNORED UNRESPONSIVE NODES   STATUS    REASON   ACTIVE RS             CANARY RS   AGE
datadog-agent   1         1         1       1            1                                        Running            datadog-agent-xxxxx               5m44s

@khewonc khewonc added enhancement New feature or request component/controller labels Aug 5, 2022
@khewonc khewonc added this to the v1.0.0 milestone Aug 5, 2022
@khewonc khewonc requested review from a team as code owners August 5, 2022 20:15
celenechang
celenechang reviewed Aug 10, 2022
View reviewed changes
controllers/datadogagent/controller_reconcile_v2_common.go Outdated
var result reconcile.Result
var err error

// Set DatadogAgent instance instance as the owner and controller
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Set DatadogAgent instance instance as the owner and controller
// Set DatadogAgent instance as the owner and controller

celenechang
celenechang reviewed Aug 10, 2022
View reviewed changes
controllers/datadogagent/controller_reconcile_v2_common.go Outdated
return reconcile.Result{}, err
}

// From here the PodTemplateSpec should be ready, we can generate the hash that will be use to compare this extendeddaemonset with the current (if exist).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// From here the PodTemplateSpec should be ready, we can generate the hash that will be use to compare this extendeddaemonset with the current (if exist).
// From here the PodTemplateSpec should be ready, we can generate the hash that will be used to compare this extendeddaemonset with the current one (if it exists).

celenechang
celenechang reviewed Aug 10, 2022
View reviewed changes
controllers/datadogagent/controller_reconcile_v2_common.go Outdated
}

currentEDS := &edsv1alpha1.ExtendedDaemonSet{}
alreadyExist := true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: alreadyExists ?

celenechang
celenechang reviewed Aug 10, 2022
View reviewed changes
controllers/datadogagent/controller_reconcile_v2_common.go Outdated
// check if same hash
needUpdate := !comparison.IsSameSpecMD5Hash(hash, currentEDS.GetAnnotations())
if !needUpdate {
// Even if the EDS is still the same, it's status might have
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Even if the EDS is still the same, it's status might have
// Even if the EDS is still the same, its status might have

celenechang
celenechang reviewed Aug 10, 2022
View reviewed changes
controllers/datadogagent/controller_reconcile_v2_common.go Outdated
now := metav1.NewTime(time.Now())
newStatus.Agent = datadoghqv2alpha1.UpdateExtendedDaemonSetStatus(currentEDS, newStatus.Agent, &now)

// no need to update the EDS to stop here the process
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// no need to update the EDS to stop here the process
// Stop reconcile loop since EDS hasn't changed

celenechang
celenechang reviewed Aug 10, 2022
View reviewed changes
controllers/datadogagent/controller_reconcile_v2_common.go Outdated

logger.Info("Updating ExtendedDaemonSet")

// TODO: these parameter can be added to the override.PodTemplateSpec. (it exist in v1alpha)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// TODO: these parameter can be added to the override.PodTemplateSpec. (it exist in v1alpha)
// TODO: these parameters can be added to the override.PodTemplateSpec. (It exists in v1alpha1)

celenechang
celenechang reviewed Aug 10, 2022
View reviewed changes
controllers/datadogagent/controller_reconcile_v2_common.go Outdated
}
event := buildEventInfo(updateEDS.Name, updateEDS.Namespace, extendedDaemonSetKind, datadog.UpdateEvent)
r.recordEvent(dda, event)
updateStatusFunc(updateEDS, newStatus, now, metav1.ConditionTrue, "Extended_Daemonset_updated", "ExtendedDaemonSet updated")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
updateStatusFunc(updateEDS, newStatus, now, metav1.ConditionTrue, "Extended_Daemonset_updated", "ExtendedDaemonSet updated")
updateStatusFunc(updateEDS, newStatus, now, metav1.ConditionTrue, "ExtendedDaemonSet_updated", "ExtendedDaemonSet updated")

maybe?

celenechang
celenechang approved these changes Aug 10, 2022
View reviewed changes
Copy link
Contributor

@celenechang celenechang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just some grammar suggestions, lgtm

@khewonc khewonc merged commit b3d4ea4 into main Aug 11, 2022
@khewonc khewonc deleted the khewonc/eds-v2 branch August 11, 2022 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@celenechang celenechang celenechang approved these changes

@github-actions GitHub Actions github-actions approved these changes

Assignees
No one assigned
Labels
component/controller enhancement New feature or request
Projects
None yet
Milestone
v1.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@khewonc @celenechang