Skip to content

Commit 87732a4

Browse files
jcantrilljcantrill
committed
LOG-7618: Modify lokistack datamodel to default to 'Otel' in lieu of 'Viaq'
1 parent d87d486 commit 87732a4

File tree

9 files changed

+176
-17
lines changed

9 files changed

+176
-17
lines changed

‎api/observability/v1/output_types.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,7 @@ type Kafka struct {
744744
Brokers []BrokerURL `json:"brokers,omitempty"`
745745
}
746746

747-
// +kubebuilder:validation:XValidation:rule="self == '' || (isURL(self) && (self.startsWith('tcp://') || self.startsWith('tls://')))",message="each broker must be a valid URL with a tcp or tls scheme"
747+
// +kubebuilder:validation:XValidation:rule="isURL(self) && (self.startsWith('tcp://') || self.startsWith('tls://'))",message="each broker must be a valid URL with a tcp or tls scheme"
748748
type BrokerURL string
749749

750750
type LokiTuningSpec struct {
@@ -834,10 +834,10 @@ type LokiStack struct {
834834
//
835835
// There are two different models to choose from:
836836
//
837-
// - Viaq
837+
// - Viaq (DEPRECATED: To not be ignored in a future release)
838838
// - Otel
839839
//
840-
// When the data model is not set, it currently defaults to the "Viaq" data model.
840+
// When the data model is not set, it currently defaults to the "Otel" data model.
841841
//
842842
// +kubebuilder:validation:Optional
843843
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Data Model"

‎config/crd/bases/observability.openshift.io_clusterlogforwarders.yaml‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2535,8 +2535,7 @@ spec:
25352535
x-kubernetes-validations:
25362536
- message: each broker must be a valid URL with a tcp
25372537
or tls scheme
2538-
rule: self == '' || (isURL(self) && (self.startsWith('tcp://')
2539-
|| self.startsWith('tls://')))
2538+
rule: isURL(self) && (self.startsWith('tcp://') || self.startsWith('tls://'))
25402539
type: array
25412540
topic:
25422541
description: |-
@@ -2839,10 +2838,10 @@ spec:
28392838
28402839
There are two different models to choose from:
28412840
2842-
- Viaq
2841+
- Viaq (DEPRECATED: To not be ignored in a future release)
28432842
- Otel
28442843
2845-
When the data model is not set, it currently defaults to the "Viaq" data model.
2844+
When the data model is not set, it currently defaults to the "Otel" data model.
28462845
enum:
28472846
- Viaq
28482847
- Otel

‎internal/api/initialize/init_outputs.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package initialize
2+
3+
import (
4+
obs "github.com/openshift/cluster-logging-operator/api/observability/v1"
5+
"github.com/openshift/cluster-logging-operator/internal/constants"
6+
"github.com/openshift/cluster-logging-operator/internal/utils"
7+
"github.com/openshift/cluster-logging-operator/internal/validations/observability/common"
8+
)
9+
10+
// MigrateOutputs initializes the outputs for a ClusterLogForwarder
11+
func MigrateOutputs(forwarder obs.ClusterLogForwarder, options utils.Options) obs.ClusterLogForwarder {
12+
enabled := common.IsEnabledAnnotation(forwarder, constants.AnnotationOtlpOutputTechPreview)
13+
for _, o := range forwarder.Spec.Outputs {
14+
if enabled {
15+
if o.Type == obs.OutputTypeLokiStack && o.LokiStack != nil && o.LokiStack.DataModel == "" {
16+
o.LokiStack.DataModel = obs.LokiStackDataModelOpenTelemetry
17+
}
18+
} else if o.Type == obs.OutputTypeLokiStack && o.LokiStack != nil {
19+
o.LokiStack.DataModel = obs.LokiStackDataModelViaq
20+
}
21+
}
22+
return forwarder
23+
}

‎internal/api/initialize/init_outputs_test.go‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
package initialize
2+
3+
import (
4+
. "github.com/onsi/ginkgo/v2"
5+
. "github.com/onsi/gomega"
6+
obs "github.com/openshift/cluster-logging-operator/api/observability/v1"
7+
"github.com/openshift/cluster-logging-operator/internal/constants"
8+
"github.com/openshift/cluster-logging-operator/internal/utils"
9+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10+
)
11+
12+
var _ = Describe("migrateOutputs", func() {
13+
var (
14+
initContext utils.Options
15+
)
16+
BeforeEach(func() {
17+
initContext = utils.Options{}
18+
})
19+
Context("for LokiStack outputs", func() {
20+
21+
Context("when the OTLP Tech-preview feature is not enabled", func() {
22+
It("should set the datamodel to 'Viaq' regardless of of the value spec'd for the datamodel", func() {
23+
forwarder := obs.ClusterLogForwarder{
24+
Spec: obs.ClusterLogForwarderSpec{
25+
Outputs: []obs.OutputSpec{
26+
{
27+
Name: "anOutput",
28+
Type: obs.OutputTypeLokiStack,
29+
LokiStack: &obs.LokiStack{
30+
DataModel: obs.LokiStackDataModelOpenTelemetry,
31+
}},
32+
},
33+
},
34+
}
35+
result := MigrateOutputs(forwarder, initContext)
36+
Expect(result.Spec.Outputs).To(HaveLen(1))
37+
Expect(result.Spec.Outputs[0]).To(Equal(obs.OutputSpec{
38+
Name: "anOutput",
39+
Type: obs.OutputTypeLokiStack,
40+
LokiStack: &obs.LokiStack{
41+
DataModel: obs.LokiStackDataModelViaq,
42+
},
43+
}))
44+
})
45+
46+
})
47+
48+
Context("when the OTLP Tech-preview feature is enabled", func() {
49+
50+
It("should set the datamodel to 'Otel' when the datamodel is not spec'd", func() {
51+
forwarder := obs.ClusterLogForwarder{
52+
ObjectMeta: v1.ObjectMeta{
53+
Annotations: map[string]string{
54+
constants.AnnotationOtlpOutputTechPreview: "enabled",
55+
},
56+
},
57+
Spec: obs.ClusterLogForwarderSpec{
58+
Outputs: []obs.OutputSpec{
59+
{
60+
Name: "anOutput",
61+
Type: obs.OutputTypeLokiStack,
62+
LokiStack: &obs.LokiStack{},
63+
},
64+
},
65+
},
66+
}
67+
result := MigrateOutputs(forwarder, initContext)
68+
Expect(result.Spec.Outputs).To(HaveLen(1))
69+
Expect(result.Spec.Outputs[0]).To(Equal(obs.OutputSpec{
70+
Name: "anOutput",
71+
Type: obs.OutputTypeLokiStack,
72+
LokiStack: &obs.LokiStack{
73+
DataModel: obs.LokiStackDataModelOpenTelemetry,
74+
},
75+
}))
76+
})
77+
It("should honor the datamodel 'ViaQ' when the datamodel is spec'd to ViaQ", func() {
78+
forwarder := obs.ClusterLogForwarder{
79+
ObjectMeta: v1.ObjectMeta{
80+
Annotations: map[string]string{
81+
constants.AnnotationOtlpOutputTechPreview: "true",
82+
},
83+
},
84+
Spec: obs.ClusterLogForwarderSpec{
85+
Outputs: []obs.OutputSpec{
86+
{
87+
Name: "anOutput",
88+
Type: obs.OutputTypeLokiStack,
89+
LokiStack: &obs.LokiStack{
90+
DataModel: obs.LokiStackDataModelViaq,
91+
},
92+
},
93+
},
94+
},
95+
}
96+
result := MigrateOutputs(forwarder, initContext)
97+
Expect(result.Spec.Outputs).To(HaveLen(1))
98+
Expect(result.Spec.Outputs[0]).To(Equal(obs.OutputSpec{
99+
Name: "anOutput",
100+
Type: obs.OutputTypeLokiStack,
101+
LokiStack: &obs.LokiStack{
102+
DataModel: obs.LokiStackDataModelViaq,
103+
},
104+
}))
105+
})
106+
It("should honor the datamodel 'Otel' when the datamodel is spec'd to Otel", func() {
107+
forwarder := obs.ClusterLogForwarder{
108+
ObjectMeta: v1.ObjectMeta{
109+
Annotations: map[string]string{
110+
constants.AnnotationOtlpOutputTechPreview: "true",
111+
},
112+
},
113+
Spec: obs.ClusterLogForwarderSpec{
114+
Outputs: []obs.OutputSpec{
115+
{
116+
Name: "anOutput",
117+
Type: obs.OutputTypeLokiStack,
118+
LokiStack: &obs.LokiStack{
119+
DataModel: obs.LokiStackDataModelOpenTelemetry,
120+
},
121+
},
122+
},
123+
},
124+
}
125+
result := MigrateOutputs(forwarder, initContext)
126+
Expect(result.Spec.Outputs).To(HaveLen(1))
127+
Expect(result.Spec.Outputs[0]).To(Equal(obs.OutputSpec{
128+
Name: "anOutput",
129+
Type: obs.OutputTypeLokiStack,
130+
LokiStack: &obs.LokiStack{
131+
DataModel: obs.LokiStackDataModelOpenTelemetry,
132+
},
133+
}))
134+
})
135+
})
136+
137+
})
138+
})

‎internal/api/initialize/initializations.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ const (
1616
var clfInitializers = []func(spec obs.ClusterLogForwarder, migrateContext utils.Options) obs.ClusterLogForwarder{
1717
Resources,
1818
MigrateInputs,
19+
MigrateOutputs,
1920
}
2021

2122
// ClusterLogForwarder initializes the forwarder for fields that must be set and are inferred from settings already defined.

‎internal/controller/observability/clusterlogforwarder_controller.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,6 @@ func initialize(cxt internalcontext.ForwarderContext) (internalcontext.Forwarder
175175
cxt.AdditionalContext = utils.Options{}
176176
migrated := internalinit.ClusterLogForwarder(*cxt.Forwarder, cxt.AdditionalContext)
177177
cxt.Forwarder = &migrated
178-
179178
if cxt.Secrets, err = MapSecrets(cxt.Client, cxt.Forwarder.Namespace, cxt.Forwarder.Spec.Inputs, cxt.Forwarder.Spec.Outputs); err != nil {
180179
return cxt, err
181180
}

‎internal/validations/observability/common/validate.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ package common
22

33
import (
44
"fmt"
5+
"strings"
6+
57
obsv1 "github.com/openshift/cluster-logging-operator/api/observability/v1"
6-
internalcontext "github.com/openshift/cluster-logging-operator/internal/api/context"
78
"github.com/openshift/cluster-logging-operator/internal/utils/sets"
89
corev1 "k8s.io/api/core/v1"
9-
"strings"
1010
)
1111

1212
// ValidateValueReference checks for valid names and keys referenced in secrets and configMaps
@@ -49,9 +49,9 @@ func validateConfigMap(configMapName, key string, configMaps map[string]*corev1.
4949
}
5050

5151
// IsEnabledAnnotation checks if an annotation is set to either "true" or "enabled"
52-
func IsEnabledAnnotation(context internalcontext.ForwarderContext, annotation string) bool {
52+
func IsEnabledAnnotation(forwarder obsv1.ClusterLogForwarder, annotation string) bool {
5353
enabledValues := sets.NewString("true", "enabled")
54-
if value, ok := context.Forwarder.Annotations[annotation]; ok {
54+
if value, ok := forwarder.Annotations[annotation]; ok {
5555
if enabledValues.Has(value) {
5656
return true
5757
}

‎internal/validations/observability/outputs/validate_tech_preview.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const MissingAnnotationMessage = "requires a valid tech-preview annotation"
1414

1515
// ValidateTechPreviewAnnotation verifies the tech-preview annotation for outputs sending OTEL data
1616
func ValidateTechPreviewAnnotation(out obsv1.OutputSpec, context internalcontext.ForwarderContext) (messages []string) {
17-
enabled := common.IsEnabledAnnotation(context, constants.AnnotationOtlpOutputTechPreview)
17+
enabled := common.IsEnabledAnnotation(*context.Forwarder, constants.AnnotationOtlpOutputTechPreview)
1818
if out.Type == obsv1.OutputTypeOTLP && !enabled {
1919
log.V(3).Info("ValidateTechPreviewAnnotation failed", "reason", MissingAnnotationMessage)
2020
messages = append(messages, fmt.Sprintf("output %q %v", out.Name, MissingAnnotationMessage))

‎test/e2e/logforwarding/lokistack/forward_to_lokistack_test.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,7 @@ var _ = Describe("[ClusterLogForwarder] Forward to Lokistack", func() {
103103
Fail(fmt.Sprintf("unable to deploy log generator %v.", err))
104104
}
105105
})
106-
107-
It("should send logs to lokistack's OTLP endpoint when dataModel == Otel", func() {
108-
lokiStackOut.LokiStack.DataModel = obs.LokiStackDataModelOpenTelemetry
106+
It("should send logs to lokistack's OTLP endpoint when dataModel is not spec'd", func() {
109107
forwarder.Spec.Outputs = append(forwarder.Spec.Outputs, *lokiStackOut)
110108

111109
if err := e2e.CreateObservabilityClusterLogForwarder(forwarder); err != nil {
@@ -134,7 +132,8 @@ var _ = Describe("[ClusterLogForwarder] Forward to Lokistack", func() {
134132
Expect(found).To(BeTrue())
135133
})
136134

137-
It("should send logs to lokistack when dataModel is not spec'd", func() {
135+
It("should send logs to lokistack when dataModel == Viaq", func() {
136+
lokiStackOut.LokiStack.DataModel = obs.LokiStackDataModelViaq
138137
forwarder.Spec.Outputs = append(forwarder.Spec.Outputs, *lokiStackOut)
139138

140139
if err := e2e.CreateObservabilityClusterLogForwarder(forwarder); err != nil {

0 commit comments

Comments
 (0)