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.

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

[cleanup] move some common constants to v2alpha1 #1432

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

celenechang
Copy link
Contributor

What does this PR do?

Move a subset of constants out of api/datadoghq/common/const.go and into api/datadoghq/v2alpha1/const.go

Motivation

Cleanup

Additional Notes

Anything else we should know when reviewing?

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

  • Agent: vX.Y.Z
  • Cluster Agent: vX.Y.Z

Describe your test plan

The change is noop. Make sure the operator builds and runs as expected.

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label

@celenechang celenechang added this to the v1.10.0 milestone Sep 24, 2024
@celenechang celenechang requested review from a team as code owners September 24, 2024 12:02
KubeletAgentCAPath = "/var/run/host-kubelet-ca.crt"
KubeletCAVolumeName = "kubelet-ca"
APMHostPortName = "traceport"
// APMHostPortHostPort = 8126 // Not used
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i didn't remove these because i don't want to forget to check for consistency when moving these constants

@codecov-commenter
Copy link

codecov-commenter commented Sep 24, 2024

Codecov Report

Attention: Patch coverage is 58.94737% with 39 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@e740b32). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...ller/datadogagent/feature/enabledefault/feature.go 25.00% 6 Missing ⚠️
...oller/datadogagent/component/clusteragent/utils.go 0.00% 5 Missing ⚠️
...adogagent/component/clusterchecksrunner/default.go 16.66% 5 Missing ⚠️
api/datadoghq/v2alpha1/secret.go 0.00% 4 Missing ⚠️
...ntroller/datadogagent/component/objects/network.go 50.00% 3 Missing ⚠️
...ntroller/datadogagent/feature/dogstatsd/feature.go 50.00% 3 Missing ⚠️
...nternal/controller/datadogagent/override/global.go 0.00% 2 Missing and 1 partial ⚠️
...er/datadogagent/feature/externalmetrics/feature.go 71.42% 2 Missing ⚠️
api/datadoghq/v2alpha1/datadogagent_default.go 80.00% 1 Missing ⚠️
api/datadoghq/v2alpha1/utils.go 75.00% 1 Missing ⚠️
... and 6 more
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1432   +/-   ##
=======================================
  Coverage        ?   48.94%           
=======================================
  Files           ?      221           
  Lines           ?    19250           
  Branches        ?        0           
=======================================
  Hits            ?     9422           
  Misses          ?     9345           
  Partials        ?      483           
Flag Coverage Δ
unittests 48.94% <58.94%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...controller/datadogagent/controller_reconcile_v2.go 54.31% <100.00%> (ø)
...atadogagent/feature/admissioncontroller/feature.go 79.52% <100.00%> (ø)
...al/controller/datadogagent/feature/cspm/feature.go 72.79% <100.00%> (ø)
...nal/controller/datadogagent/feature/cws/feature.go 79.55% <100.00%> (ø)
...ntroller/datadogagent/feature/ebpfcheck/feature.go 83.60% <100.00%> (ø)
...er/datadogagent/feature/eventcollection/feature.go 62.31% <100.00%> (ø)
...ntroller/datadogagent/feature/helmcheck/feature.go 78.65% <100.00%> (ø)
...atadogagent/feature/kubernetesstatecore/feature.go 69.78% <100.00%> (ø)
...nal/controller/datadogagent/feature/npm/feature.go 89.25% <100.00%> (ø)
...controller/datadogagent/feature/oomkill/feature.go 87.17% <100.00%> (ø)
... and 21 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e740b32...e386dd9. Read the comment docs.

Copy link
Member

@tbavelier tbavelier Sep 25, 2024

Choose a reason for hiding this comment

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

Not directly related to this PR, but as part of this cleanup, should we possibly delete

// check it the resource was properly decoded in v2
// if not it means it was a v1
/*if apiequality.Semantic.DeepEqual(instance.Spec, datadoghqv2alpha1.DatadogAgentSpec{}) {
instanceV1 := &datadoghqv1alpha1.DatadogAgent{}
if err = r.client.Get(ctx, request.NamespacedName, instanceV1); err != nil {
if apierrors.IsNotFound(err) {
// Request object not found, could have been deleted after reconcile request.
// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
// Return and don't requeue
return result, nil
}
// Error reading the object - requeue the request.starting metrics server
return result, err
}
if err = datadoghqv1alpha1.ConvertTo(instanceV1, instance); err != nil {
reqLogger.Error(err, "unable to convert to v2alpha1")
return result, err
}
}*/
and
// TODO check if IsValideDatadogAgent function is needed for v2
/*
if err = datadoghqv2alpha1.IsValidDatadogAgent(&instance.Spec); err != nil {
reqLogger.V(1).Info("Invalid spec", "error", err)
return r.updateStatusIfNeeded(reqLogger, instance, &instance.Status, result, err)
}
*/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, thanks

Comment on lines -91 to -98
// ExtendedDaemonset defaulting
DefaultRollingUpdateMaxUnavailable = "10%"
DefaultUpdateStrategy = appsv1.RollingUpdateDaemonSetStrategyType
DefaultRollingUpdateMaxPodSchedulerFailure = "10%"
DefaultRollingUpdateMaxParallelPodCreation int32 = 250
DefaultRollingUpdateSlowStartIntervalDuration = 1 * time.Minute
DefaultRollingUpdateSlowStartAdditiveIncrease = "5"
DefaultReconcileFrequency = 10 * time.Second
Copy link
Member

Choose a reason for hiding this comment

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

👏 Good catch, these were only used in Agent default v1alpha1

@celenechang celenechang merged commit a798a7a into main Sep 25, 2024
19 checks passed
@celenechang celenechang deleted the celene/common_consts branch September 25, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants