Skip to content

Commit 9f2359f

Browse files
khewonckhewonc
authored
Add shared dependencies (#3183)
* add shared dependencies * validate host port conflict prior to apply * fix tests
1 parent eba44d2 commit 9f2359f

18 files changed

Lines changed: 1073 additions & 341 deletions

‎internal/controller/datadogagent/controller_reconcile_v2.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,6 @@ func (r *Reconciler) reconcileInstanceV3(ctx context.Context, logger logr.Logger
9494
}
9595
}
9696

97-
// Manage dependencies
98-
if err := r.manageDDADependenciesWithDDAI(ctx, logger, instance, newDDAStatus); err != nil {
99-
return r.updateStatusIfNeededV2(logger, instance, ddaStatusCopy, result, err, now)
100-
}
101-
10297
// Generate default DDAI object from DDA
10398
ddai, err := r.generateDDAIFromDDA(instance, provider)
10499
if err != nil {
@@ -131,6 +126,12 @@ func (r *Reconciler) reconcileInstanceV3(ctx context.Context, logger logr.Logger
131126
ddais = profileDDAIs
132127
}
133128

129+
// Manage dependencies after DDAIs are computed to include profile changes
130+
err = r.manageDDADependenciesWithDDAI(ctx, logger, instance, newDDAStatus, ddais)
131+
if err != nil {
132+
return r.updateStatusIfNeededV2(logger, instance, ddaStatusCopy, result, err, now)
133+
}
134+
134135
// Create or update the DDAI object in k8s
135136
for _, ddai := range ddais {
136137
if e := r.createOrUpdateDDAI(ddai); e != nil {

‎internal/controller/datadogagent/controller_v2_test.go‎

Lines changed: 168 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"k8s.io/apimachinery/pkg/runtime"
2525
"k8s.io/apimachinery/pkg/types"
2626
"k8s.io/apimachinery/pkg/util/intstr"
27+
"k8s.io/apimachinery/pkg/version"
2728
"k8s.io/client-go/tools/record"
2829
"k8s.io/utils/ptr"
2930
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -37,8 +38,10 @@ import (
3738
"github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
3839
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/common"
3940
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/experimental"
41+
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/store"
4042
agenttestutils "github.com/DataDog/datadog-operator/internal/controller/datadogagent/testutils"
4143
"github.com/DataDog/datadog-operator/internal/controller/datadogagentinternal"
44+
"github.com/DataDog/datadog-operator/pkg/agentprofile"
4245
"github.com/DataDog/datadog-operator/pkg/condition"
4346
"github.com/DataDog/datadog-operator/pkg/constants"
4447
"github.com/DataDog/datadog-operator/pkg/controller/utils/comparison"
@@ -62,6 +65,7 @@ type testCase struct {
6265
profilesEnabled bool // For DDAI tests
6366
introspectionEnabled bool // For introspection tests
6467
clusterProvider string // For control plane monitoring tests: provider returned by the injected detector
68+
platformInfo *kubernetes.PlatformInfo
6569
}
6670

6771
// ddaiReconcilerOptionsFromDDA mirrors setup.go wiring so DDAI tests behave like production
@@ -75,6 +79,13 @@ func ddaiReconcilerOptionsFromDDA(opts ReconcilerOptions) datadogagentinternal.R
7579
}
7680
}
7781

82+
func platformInfoForTest(tt testCase) kubernetes.PlatformInfo {
83+
if tt.platformInfo != nil {
84+
return *tt.platformInfo
85+
}
86+
return kubernetes.PlatformInfo{}
87+
}
88+
7889
// runTestCases runs test cases
7990
func runTestCases(t *testing.T, tests []testCase, testFunc func(t *testing.T, tt testCase, opts ReconcilerOptions)) {
8091
for _, tt := range tests {
@@ -111,12 +122,13 @@ func runDDAReconcilerTest(t *testing.T, tt testCase, opts ReconcilerOptions) {
111122
forwarders := dummyManager{}
112123

113124
c := buildClient(t, tt, s)
125+
platformInfo := platformInfoForTest(tt)
114126

115127
// Create reconciler
116128
r := &Reconciler{
117129
client: c,
118130
scheme: s,
119-
platformInfo: kubernetes.PlatformInfo{},
131+
platformInfo: platformInfo,
120132
recorder: recorder,
121133
log: logf.Log.WithName(tt.name),
122134
forwarders: forwarders,
@@ -127,7 +139,7 @@ func runDDAReconcilerTest(t *testing.T, tt testCase, opts ReconcilerOptions) {
127139
ri := datadogagentinternal.NewReconciler(
128140
ddaiReconcilerOptionsFromDDA(opts),
129141
c,
130-
kubernetes.PlatformInfo{},
142+
platformInfo,
131143
s,
132144
recorder,
133145
forwarders)
@@ -184,12 +196,13 @@ func runFullReconcilerTest(t *testing.T, tt testCase, opts ReconcilerOptions) {
184196
forwarders := dummyManager{}
185197

186198
c := buildClient(t, tt, s)
199+
platformInfo := platformInfoForTest(tt)
187200

188201
// Create reconciler
189202
r := &Reconciler{
190203
client: c,
191204
scheme: s,
192-
platformInfo: kubernetes.PlatformInfo{},
205+
platformInfo: platformInfo,
193206
recorder: recorder,
194207
log: logf.Log.WithName(tt.name),
195208
forwarders: forwarders,
@@ -200,7 +213,7 @@ func runFullReconcilerTest(t *testing.T, tt testCase, opts ReconcilerOptions) {
200213
ri := datadogagentinternal.NewReconciler(
201214
ddaiReconcilerOptionsFromDDA(opts),
202215
c,
203-
kubernetes.PlatformInfo{},
216+
platformInfo,
204217
s,
205218
recorder,
206219
forwarders)
@@ -1834,6 +1847,157 @@ func Test_COSProviderOverrides(t *testing.T) {
18341847
runTestCases(t, tests, runFullReconcilerTest)
18351848
}
18361849

1850+
func Test_ProfileAPMOverrideAddsDDAOwnedLocalAgentServicePort(t *testing.T) {
1851+
const resourcesName = "foo"
1852+
const resourcesNamespace = "bar"
1853+
const profileName = "apm-profile"
1854+
defaultRequeueDuration := 15 * time.Second
1855+
1856+
dda := testutils.NewInitializedDatadogAgentBuilder(resourcesNamespace, resourcesName).
1857+
WithAPMEnabled(false).
1858+
BuildWithDefaults()
1859+
localAgentServiceName := constants.GetLocalAgentServiceName(resourcesName, &dda.Spec)
1860+
platformInfo := kubernetes.NewPlatformInfoFromVersionMaps(&version.Info{GitVersion: "1.32.0"}, nil, nil)
1861+
1862+
newAPMProfile := func(name string, apm *v2alpha1.APMFeatureConfig) *v1alpha1.DatadogAgentProfile {
1863+
return &v1alpha1.DatadogAgentProfile{
1864+
ObjectMeta: metav1.ObjectMeta{
1865+
Name: name,
1866+
Namespace: resourcesNamespace,
1867+
},
1868+
Spec: v1alpha1.DatadogAgentProfileSpec{
1869+
ProfileAffinity: &v1alpha1.ProfileAffinity{
1870+
ProfileNodeAffinity: []corev1.NodeSelectorRequirement{
1871+
{
1872+
Key: "profile",
1873+
Operator: corev1.NodeSelectorOpIn,
1874+
Values: []string{name},
1875+
},
1876+
},
1877+
},
1878+
Config: &v2alpha1.DatadogAgentSpec{
1879+
Features: &v2alpha1.DatadogFeatures{
1880+
APM: apm,
1881+
},
1882+
},
1883+
},
1884+
}
1885+
}
1886+
profile := newAPMProfile(profileName, &v2alpha1.APMFeatureConfig{
1887+
Enabled: ptr.To(true),
1888+
})
1889+
conflictingProfileA := newAPMProfile("apm-profile-a", &v2alpha1.APMFeatureConfig{
1890+
Enabled: ptr.To(true),
1891+
HostPortConfig: &v2alpha1.HostPortConfig{
1892+
Enabled: ptr.To(true),
1893+
Port: ptr.To[int32](8126),
1894+
},
1895+
})
1896+
conflictingProfileB := newAPMProfile("apm-profile-b", &v2alpha1.APMFeatureConfig{
1897+
Enabled: ptr.To(true),
1898+
HostPortConfig: &v2alpha1.HostPortConfig{
1899+
Enabled: ptr.To(true),
1900+
Port: ptr.To[int32](9126),
1901+
},
1902+
})
1903+
1904+
tests := []testCase{
1905+
{
1906+
name: "profile APM override adds trace port to DDA-owned local Agent Service",
1907+
clientBuilder: fake.NewClientBuilder().
1908+
WithStatusSubresource(&v2alpha1.DatadogAgent{}, &v1alpha1.DatadogAgentProfile{}, &v1alpha1.DatadogAgentInternal{}).
1909+
WithObjects(profile),
1910+
loadFunc: func(c client.Client) *v2alpha1.DatadogAgent {
1911+
ddaCopy := dda.DeepCopy()
1912+
_ = c.Create(context.TODO(), ddaCopy)
1913+
return ddaCopy
1914+
},
1915+
profilesEnabled: true,
1916+
platformInfo: &platformInfo,
1917+
want: reconcile.Result{RequeueAfter: defaultRequeueDuration},
1918+
wantErr: false,
1919+
wantFunc: func(t *testing.T, c client.Client) {
1920+
service := &corev1.Service{}
1921+
err := c.Get(context.TODO(), types.NamespacedName{Namespace: resourcesNamespace, Name: localAgentServiceName}, service)
1922+
assert.NoError(t, err)
1923+
assert.Equal(t, "true", service.Labels[store.ManagedByDDAControllerLabelKey])
1924+
1925+
apmPort := findServicePortByName(service.Spec.Ports, constants.DefaultApmPortName)
1926+
assert.NotNil(t, apmPort)
1927+
assert.Equal(t, corev1.ProtocolTCP, apmPort.Protocol)
1928+
assert.Equal(t, int32(constants.DefaultApmPort), apmPort.Port)
1929+
assert.Equal(t, intstr.FromInt(int(constants.DefaultApmPort)), apmPort.TargetPort)
1930+
1931+
defaultDDAI := &v1alpha1.DatadogAgentInternal{}
1932+
err = c.Get(context.TODO(), types.NamespacedName{Namespace: resourcesNamespace, Name: resourcesName}, defaultDDAI)
1933+
assert.NoError(t, err)
1934+
assert.False(t, ptr.Deref(defaultDDAI.Spec.Features.APM.Enabled, true))
1935+
1936+
profileDDAI := &v1alpha1.DatadogAgentInternal{}
1937+
err = c.Get(context.TODO(), types.NamespacedName{Namespace: resourcesNamespace, Name: profileName}, profileDDAI)
1938+
assert.NoError(t, err)
1939+
assert.True(t, ptr.Deref(profileDDAI.Spec.Features.APM.Enabled, false))
1940+
},
1941+
},
1942+
{
1943+
name: "conflicting profile APM override rejects conflicting profile and reconciles accepted profile",
1944+
clientBuilder: fake.NewClientBuilder().
1945+
WithStatusSubresource(&v2alpha1.DatadogAgent{}, &v1alpha1.DatadogAgentProfile{}, &v1alpha1.DatadogAgentInternal{}).
1946+
WithObjects(conflictingProfileA, conflictingProfileB),
1947+
loadFunc: func(c client.Client) *v2alpha1.DatadogAgent {
1948+
ddaCopy := dda.DeepCopy()
1949+
_ = c.Create(context.TODO(), ddaCopy)
1950+
return ddaCopy
1951+
},
1952+
profilesEnabled: true,
1953+
platformInfo: &platformInfo,
1954+
want: reconcile.Result{RequeueAfter: defaultRequeueDuration},
1955+
wantErr: false,
1956+
wantFunc: func(t *testing.T, c client.Client) {
1957+
service := &corev1.Service{}
1958+
err := c.Get(context.TODO(), types.NamespacedName{Namespace: resourcesNamespace, Name: localAgentServiceName}, service)
1959+
assert.NoError(t, err)
1960+
assert.Equal(t, "true", service.Labels[store.ManagedByDDAControllerLabelKey])
1961+
1962+
apmPort := findServicePortByName(service.Spec.Ports, constants.DefaultApmPortName)
1963+
assert.NotNil(t, apmPort)
1964+
assert.Equal(t, int32(8126), apmPort.Port)
1965+
assert.Equal(t, intstr.FromInt(int(constants.DefaultApmPort)), apmPort.TargetPort)
1966+
1967+
appliedProfile := &v1alpha1.DatadogAgentProfile{}
1968+
err = c.Get(context.TODO(), types.NamespacedName{Namespace: resourcesNamespace, Name: "apm-profile-a"}, appliedProfile)
1969+
assert.NoError(t, err)
1970+
assert.Equal(t, metav1.ConditionTrue, appliedProfile.Status.Applied)
1971+
1972+
conflictingProfile := &v1alpha1.DatadogAgentProfile{}
1973+
err = c.Get(context.TODO(), types.NamespacedName{Namespace: resourcesNamespace, Name: "apm-profile-b"}, conflictingProfile)
1974+
assert.NoError(t, err)
1975+
assert.Equal(t, metav1.ConditionFalse, conflictingProfile.Status.Applied)
1976+
assert.Contains(t, profileConditionMessage(conflictingProfile.Status.Conditions, agentprofile.AppliedConditionType), "port \"traceport\" conflicts")
1977+
1978+
appliedDDAI := &v1alpha1.DatadogAgentInternal{}
1979+
err = c.Get(context.TODO(), types.NamespacedName{Namespace: resourcesNamespace, Name: "apm-profile-a"}, appliedDDAI)
1980+
assert.NoError(t, err)
1981+
1982+
conflictingDDAI := &v1alpha1.DatadogAgentInternal{}
1983+
err = c.Get(context.TODO(), types.NamespacedName{Namespace: resourcesNamespace, Name: "apm-profile-b"}, conflictingDDAI)
1984+
assert.True(t, apierrors.IsNotFound(err), "conflicting profile DDAI should not be rendered")
1985+
},
1986+
},
1987+
}
1988+
1989+
runTestCases(t, tests, runFullReconcilerTest)
1990+
}
1991+
1992+
func profileConditionMessage(conditions []metav1.Condition, conditionType string) string {
1993+
for _, condition := range conditions {
1994+
if condition.Type == conditionType {
1995+
return condition.Message
1996+
}
1997+
}
1998+
return ""
1999+
}
2000+
18372001
func verifyDaemonsetContainers(t *testing.T, c client.Client, resourcesNamespace, dsName string, expectedContainers []string) {
18382002
ds := &appsv1.DaemonSet{}
18392003
err := c.Get(context.TODO(), types.NamespacedName{Namespace: resourcesNamespace, Name: dsName}, ds)

‎internal/controller/datadogagent/dependencies.go‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ package datadogagent
77

88
import (
99
"context"
10+
"fmt"
1011

1112
"github.com/go-logr/logr"
1213
corev1 "k8s.io/api/core/v1"
13-
"k8s.io/apimachinery/pkg/util/errors"
14+
utilerrors "k8s.io/apimachinery/pkg/util/errors"
1415

16+
v1alpha1 "github.com/DataDog/datadog-operator/api/datadoghq/v1alpha1"
1517
"github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
1618
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/common"
1719
"github.com/DataDog/datadog-operator/internal/controller/datadogagent/component/clusteragent"
@@ -38,7 +40,7 @@ func (r *Reconciler) setupDDADependenciesStore(instance *v2alpha1.DatadogAgent,
3840
return depsStore, resourceManagers
3941
}
4042

41-
func (r *Reconciler) manageDDADependenciesWithDDAI(ctx context.Context, logger logr.Logger, instance *v2alpha1.DatadogAgent, newDDAStatus *v2alpha1.DatadogAgentStatus) error {
43+
func (r *Reconciler) manageDDADependenciesWithDDAI(ctx context.Context, logger logr.Logger, instance *v2alpha1.DatadogAgent, newDDAStatus *v2alpha1.DatadogAgentStatus, ddais []*v1alpha1.DatadogAgentInternal) error {
4244
// Use a store marked as DDA controller store so resources are labeled
4345
// with ManagedByDDAControllerLabelKey and won't be cleaned up by DDAI controller.
4446
depsStore, resourceManagers := r.setupDDADependenciesStore(instance, logger)
@@ -71,16 +73,22 @@ func (r *Reconciler) manageDDADependenciesWithDDAI(ctx context.Context, logger l
7173
return err
7274
}
7375

76+
// Dependencies that can get configs from DDA and DDAI
77+
// Example: agent local service for APM, DSD, and OTLP
78+
if err := r.addDDASharedDependencies(instance, ddais, resourceManagers); err != nil {
79+
return err
80+
}
81+
7482
// Apply dependencies
7583
if err := depsStore.Apply(ctx, r.client); err != nil {
76-
return errors.NewAggregate(err)
84+
return utilerrors.NewAggregate(err)
7785
}
7886

7987
// Cleanup unused DDA controller dependencies.
8088
// Pass false since we want to clean up DDA-managed resources (this is the DDA controller).
8189
// Note that we don't really need to clean these dependencies as they're all ownerRef'ed by the DDA, so they will be cleaned if the DDA is deleted.
8290
if err := depsStore.Cleanup(ctx, r.client, false); err != nil {
83-
return errors.NewAggregate(err)
91+
return utilerrors.NewAggregate(err)
8492
}
8593

8694
// DatadogCSIDriver: create or delete based on spec.global.csi configuration.
@@ -93,6 +101,21 @@ func (r *Reconciler) manageDDADependenciesWithDDAI(ctx context.Context, logger l
93101
return nil
94102
}
95103

104+
func (r *Reconciler) addDDASharedDependencies(dda *v2alpha1.DatadogAgent, ddais []*v1alpha1.DatadogAgentInternal, managers feature.ResourceManagers) error {
105+
var errs []error
106+
107+
for _, ddai := range ddais {
108+
if err := feature.ApplyDDASharedDependencies(dda, &dda.Spec, ddai, &ddai.Spec, managers); err != nil {
109+
errs = append(errs, fmt.Errorf("%s/%s DDA shared dependencies failed: %w", ddai.Namespace, ddai.Name, err))
110+
}
111+
}
112+
113+
if len(errs) > 0 {
114+
return utilerrors.NewAggregate(errs)
115+
}
116+
return nil
117+
}
118+
96119
func ensureAutoGeneratedTokenInStatus(instance *v2alpha1.DatadogAgent, newStatus *v2alpha1.DatadogAgentStatus, resourceManagers feature.ResourceManagers, logger logr.Logger) {
97120
if instance.Status.ClusterAgent != nil && instance.Status.ClusterAgent.GeneratedToken != "" {
98121
// Already there; nothing to do.

0 commit comments

Comments
 (0)