Skip to content

Commit

Permalink
Consider logmonitoring in the node-selector check (#4124) (#4130)
Browse files Browse the repository at this point in the history
  • Loading branch information
@0sewa0
0sewa0 authored Dec 2, 2024
1 parent f85df8f commit e24d46c
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 13 deletions.
23 changes: 17 additions & 6 deletions pkg/api/validation/dynakube/logmonitoring_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,33 +11,44 @@ import (

func TestIgnoredLogMonitoringTemplate(t *testing.T) {
t.Run("no warning if logMonitoring template section is empty", func(t *testing.T) {
dk := createStandaloneLogMonitoringDynakube(testName, "")
dk := createStandaloneLogMonitoringDynakube(testName, testApiUrl, "")
dk.Spec.OneAgent.CloudNativeFullStack = &dynakube.CloudNativeFullStackSpec{}
dk.Spec.Templates.LogMonitoring = nil
assertAllowedWithoutWarnings(t, dk)
})
t.Run("warning if logMonitoring template section is not empty", func(t *testing.T) {
dk := createStandaloneLogMonitoringDynakube(testName, "something")
dk := createStandaloneLogMonitoringDynakube(testName, testApiUrl, "something")
dk.Spec.OneAgent.CloudNativeFullStack = &dynakube.CloudNativeFullStackSpec{}
assertAllowedWithWarnings(t, 1, dk)
})
}

func createStandaloneLogMonitoringDynakube(name, nodeSelector string) *dynakube.DynaKube {
func createStandaloneLogMonitoringDynakube(name, apiUrl, nodeSelector string) *dynakube.DynaKube {
dk := &dynakube.DynaKube{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: testNamespace,
},
Spec: dynakube.DynaKubeSpec{
APIURL: testApiUrl,
APIURL: apiUrl,
LogMonitoring: &logmonitoring.Spec{},
Templates: dynakube.TemplatesSpec{
LogMonitoring: &logmonitoring.TemplateSpec{
ImageRef: image.Ref{
Repository: "repo/image",
Tag: "version",
},
},
},
},
}

if nodeSelector != "" {
dk.Spec.Templates.LogMonitoring = &logmonitoring.TemplateSpec{
NodeSelector: map[string]string{"node": nodeSelector},
if dk.Spec.Templates.LogMonitoring == nil {
dk.Spec.Templates.LogMonitoring = &logmonitoring.TemplateSpec{}
}

dk.Spec.Templates.LogMonitoring.NodeSelector = map[string]string{"node": nodeSelector}
}

return dk
Expand Down
16 changes: 14 additions & 2 deletions pkg/api/validation/dynakube/oneagent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func conflictingOneAgentConfiguration(_ context.Context, _ *Validator, dk *dynak
}

func conflictingOneAgentNodeSelector(ctx context.Context, dv *Validator, dk *dynakube.DynaKube) string {
if !dk.NeedsOneAgent() {
if !dk.NeedsOneAgent() && !dk.LogMonitoring().IsStandalone() {
return ""
}

Expand All @@ -79,7 +79,7 @@ func conflictingOneAgentNodeSelector(ctx context.Context, dv *Validator, dk *dyn
continue
}

if item.NeedsOneAgent() {
if hasLogMonitoringSelectorConflict(dk, &item) || hasOneAgentSelectorConflict(dk, &item) {
if hasConflictingMatchLabels(oneAgentNodeSelector, item.OneAgentNodeSelector()) {
log.Info("requested dynakube has conflicting OneAgent nodeSelector", "name", dk.Name, "namespace", dk.Namespace)

Expand All @@ -95,6 +95,18 @@ func conflictingOneAgentNodeSelector(ctx context.Context, dv *Validator, dk *dyn
return ""
}

func hasLogMonitoringSelectorConflict(dk1, dk2 *dynakube.DynaKube) bool {
return dk1.LogMonitoring().IsStandalone() && dk1.ApiUrl() == dk2.ApiUrl() &&
(dk2.NeedsOneAgent() || dk2.LogMonitoring().IsStandalone()) &&
hasConflictingMatchLabels(dk1.OneAgentNodeSelector(), dk2.OneAgentNodeSelector())
}

func hasOneAgentSelectorConflict(dk1, dk2 *dynakube.DynaKube) bool {
return dk1.NeedsOneAgent() &&
(dk2.NeedsOneAgent() || dk2.LogMonitoring().IsStandalone() && dk1.ApiUrl() == dk2.ApiUrl()) &&
hasConflictingMatchLabels(dk1.OneAgentNodeSelector(), dk2.OneAgentNodeSelector())
}

func mapKeysToString(m map[string]bool, sep string) string {
keys := make([]string, 0, len(m))
for k := range m {
Expand Down
89 changes: 84 additions & 5 deletions pkg/api/validation/dynakube/oneagent_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,28 @@ func TestConflictingOneAgentConfiguration(t *testing.T) {
}

func TestConflictingNodeSelector(t *testing.T) {
t.Run("valid dynakube specs", func(t *testing.T) {
newCloudNativeDynakube := func(name, apiUrl, nodeSelectorValue string) *dynakube.DynaKube {
return &dynakube.DynaKube{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: testNamespace,
},
Spec: dynakube.DynaKubeSpec{
APIURL: apiUrl,
OneAgent: dynakube.OneAgentSpec{
CloudNativeFullStack: &dynakube.CloudNativeFullStackSpec{
HostInjectSpec: dynakube.HostInjectSpec{
NodeSelector: map[string]string{
"node": nodeSelectorValue,
},
},
},
},
},
}
}

t.Run("valid dynakube specs - 2 host-monitoring DK, different nodes", func(t *testing.T) {
assertAllowedWithoutWarnings(t,
&dynakube.DynaKube{
ObjectMeta: defaultDynakubeObjectMeta,
Expand Down Expand Up @@ -106,7 +127,8 @@ func TestConflictingNodeSelector(t *testing.T) {
},
},
})

})
t.Run("valid dynakube specs - 1 cloud-native + 1 host-monitoring DK, different nodes", func(t *testing.T) {
assertAllowedWithoutWarnings(t,
&dynakube.DynaKube{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -140,13 +162,37 @@ func TestConflictingNodeSelector(t *testing.T) {
},
})
})
t.Run(`invalid dynakube specs`, func(t *testing.T) {

t.Run("valid dynakube specs - 1 cloud-native + 1 log-monitoring DK, same tenant, different nodes", func(t *testing.T) {
api1 := "https://f1.q.d.n/api"

assertAllowedWithoutWarnings(t, newCloudNativeDynakube("dk1", api1, "1"),
createStandaloneLogMonitoringDynakube("dk-lm", api1, "12"))
})

t.Run("valid dynakube specs - 1 cloud-native + 1 log-monitoring DK, different tenant, same nodes", func(t *testing.T) {
api1 := "https://f1.q.d.n/api"
api2 := "https://f2.q.d.n/api"
assertAllowedWithoutWarnings(t, newCloudNativeDynakube("dk1", api1, "1"),
createStandaloneLogMonitoringDynakube("dk-lm", api2, "1"))
})

t.Run("valid dynakube specs - 2 log-monitoring DK, different tenant, same nodes", func(t *testing.T) {
api1 := "https://f1.q.d.n/api"
api2 := "https://f2.q.d.n/api"
assertAllowedWithoutWarnings(t, createStandaloneLogMonitoringDynakube("dk1", api1, "1"),
createStandaloneLogMonitoringDynakube("dk-lm", api2, "1"))
})

t.Run("invalid dynakube specs - 1 cloud-native + 1 host-monitoring DK, SAME nodes, different tenant", func(t *testing.T) {
api1 := "https://f1.q.d.n/api"
api2 := "https://f2.q.d.n/api"
assertDenied(t,
[]string{fmt.Sprintf(errorNodeSelectorConflict, "conflicting-dk")},
&dynakube.DynaKube{
ObjectMeta: defaultDynakubeObjectMeta,
Spec: dynakube.DynaKubeSpec{
APIURL: testApiUrl,
APIURL: api1,
OneAgent: dynakube.OneAgentSpec{
CloudNativeFullStack: &dynakube.CloudNativeFullStackSpec{
HostInjectSpec: dynakube.HostInjectSpec{
Expand All @@ -164,7 +210,7 @@ func TestConflictingNodeSelector(t *testing.T) {
Namespace: testNamespace,
},
Spec: dynakube.DynaKubeSpec{
APIURL: testApiUrl,
APIURL: api2,
OneAgent: dynakube.OneAgentSpec{
HostMonitoring: &dynakube.HostInjectSpec{
NodeSelector: map[string]string{
Expand All @@ -174,6 +220,39 @@ func TestConflictingNodeSelector(t *testing.T) {
},
},
})
t.Run("invalid dynakube specs - 1 cloud-native + 1 log-monitoring DK, same tenant, same nodes", func(t *testing.T) {
api1 := "https://f1.q.d.n/api"

assertDenied(t, []string{fmt.Sprintf(errorNodeSelectorConflict, "dk-lm")},
newCloudNativeDynakube("dk-cm", api1, "1"),
createStandaloneLogMonitoringDynakube("dk-lm", api1, "1"))
})
t.Run("multiple invalid dynakube specs - 2 cloud-native + 1 log-monitoring DK, same tenant, same nodes", func(t *testing.T) {
api1 := "https://f1.q.d.n/api"

assertDenied(t, []string{fmt.Sprintf(errorNodeSelectorConflict, ""), "dk-lm", "dk-cm2"},
newCloudNativeDynakube("dk-cm1", api1, "1"),
createStandaloneLogMonitoringDynakube("dk-lm", api1, ""),
newCloudNativeDynakube("dk-cm2", api1, "1"))
})

t.Run("invalid dynakube specs - 1 log-monitoring DK + 1 cloud-native, same tenant, same nodes", func(t *testing.T) {
api1 := "https://f1.q.d.n/api"

assertDenied(t, []string{fmt.Sprintf(errorNodeSelectorConflict, "dk-cn")},
createStandaloneLogMonitoringDynakube("dk-lm", api1, "1"),
newCloudNativeDynakube("dk-cn", api1, "1"))
})

t.Run("some invalid dynakube specs - 2 log-monitoring DK + 1 cloud-native, 2 tenants, same nodes", func(t *testing.T) {
api1 := "https://f1.q.d.n/api"
api2 := "https://f2.q.d.n/api"

assertDenied(t, []string{fmt.Sprintf(errorNodeSelectorConflict, "dk-lm2")},
createStandaloneLogMonitoringDynakube("dk-lm1", api1, "1"),
newCloudNativeDynakube("dk-cm1", api2, "1"),
createStandaloneLogMonitoringDynakube("dk-lm2", api1, "1"))
})
})
}

Expand Down

0 comments on commit e24d46c

Please sign in to comment.