Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf1da54920
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -0,0 +1,991 @@ | |||
| --- | |||
There was a problem hiding this comment.
This commit adds a new CRD base file, but config/crd/kustomization.yaml (resources list at lines 6-15) is not updated to include bases/v1/datadoghq.com_datadogpodautoscalerprofiles.yaml; as a result, kustomize build config/crd and downstream install/bundle workflows will omit this CRD, and clusters applying those manifests will reject DatadogPodAutoscalerProfile resources as unknown.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2676 +/- ##
=======================================
Coverage 38.81% 38.81%
=======================================
Files 307 307
Lines 26610 26610
=======================================
Hits 10329 10329
Misses 15501 15501
Partials 780 780
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| // ApplyPolicy defines how recommendations should be applied. | ||
| // +optional | ||
| // +kubebuilder:default={} | ||
| ApplyPolicy *DatadogPodAutoscalerApplyPolicy `json:"applyPolicy,omitempty"` |
There was a problem hiding this comment.
I am not entirely sure it should be part of a template
There was a problem hiding this comment.
Why? I mean, it looks useful to be able to setup apply policy since it affect how autoscaler will behave 🤔
There was a problem hiding this comment.
Well, the status of an Autoscaler is disjoint from its actual Spec, it would imply that you cannot pause autoscaling on a single Autoscaler managed by a template, you would need to pause all autoscalers.
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ObjectMeta `json:"metadata,omitempty"` | ||
|
|
||
| Spec DatadogPodAutoscalerSpec `json:"spec,omitempty"` |
There was a problem hiding this comment.
DatadogPodAutoscalerSpec should inline DatadogPodAutoscalerTemplate
There was a problem hiding this comment.
I initially tried this, but faced the problem that initializing DatadogPodAutoscalerSpec struct will be backward incompatible (because now you need to explicitly initialize Template struct).
So I decided to go in a more safe way and just recreate this layer.
There was a problem hiding this comment.
The YAML is backward compatible, that's what is important, the fact that the Go struct is not is actually not a problem.
|
Close in a favour of #2718 |
3 participants
What does this PR do?
Introduction of DatadogPodAutoscalerProfile. Please check this and this document to learn more about this project.
Motivation
Initial draft of new CRD for Profile entity.
Additional Notes
I want to explain my decision for moving some parts into
commonpackage.From one side, it is beneficial to have exactly
v1alpha2.DatadogPodAutoscalerSpectype insideDatadogPodAutoscalerProfilestruct. In that case all changes make to the originalv1alpha2.DatadogPodAutoscalerSpecwill automatically be propagated to profiles and usage in the code will be a bit easier.However there are two main problems with that
v1alpha1but we very likely want it to generate the latest version of DPA -v1alpha2Owner,RemoteVersionandTargetTherefore the plan is
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
Describe your test plan
Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel