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.

Sign up for GitHub

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

NETOBSERV-155: Added sampling field to console frontend configuration #264

Merged
merged 3 commits into from
Feb 22, 2023
Merged

NETOBSERV-155: Added sampling field to console frontend configuration #264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e1aa23
Added sampling field to console frontend configuration
OlivierCazade Feb 17, 2023
2cbf593
Fix tests address management
OlivierCazade Feb 17, 2023
69e9bd9
Refactoring to move API functions to helper section
OlivierCazade Feb 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions api/v1alpha1/flowcollector_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ const (
DeploymentModelKafka = "KAFKA"
)

func (spec *FlowCollectorSpec) UseEBPF() bool { return spec.Agent.Type == AgentEBPF }
func (spec *FlowCollectorSpec) UseIPFIX() bool { return spec.Agent.Type == AgentIPFIX }
func (spec *FlowCollectorSpec) UseKafka() bool { return spec.DeploymentModel == DeploymentModelKafka }

Copy link
Member

Choose a reason for hiding this comment

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

(by the way, I don't think it was necessary to add the new helper to the v1alpha1 version, and maybe not doing so would have spared us from the controversy 😅)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Operator controller now point to v1beta1, this functions were not used anyway

// Please notice that the FlowCollectorSpec's properties MUST redefine one of the default
// values to force the definition of the section when it is not provided by the manifest.
// This will cause that the remaining default fields will be set according to their definition.
Expand Down Expand Up @@ -394,32 +390,12 @@ type FlowCollectorHPA struct {
Metrics []ascv2.MetricSpec `json:"metrics"`
}

func (spec *FlowCollectorHPA) Disabled() bool {
return spec.Status == HPAStatusDisabled
}

func (spec *FlowCollectorHPA) Enabled() bool {
return spec.Status == HPAStatusEnabled
}

const (
LokiAuthDisabled = "DISABLED"
LokiAuthUseHostToken = "HOST"
LokiAuthForwardUserToken = "FORWARD"
)

func (spec *FlowCollectorLoki) NoAuthToken() bool {
return spec.AuthToken == LokiAuthDisabled
}

func (spec *FlowCollectorLoki) UseHostToken() bool {
return spec.AuthToken == LokiAuthUseHostToken
}

func (spec *FlowCollectorLoki) ForwardUserToken() bool {
return spec.AuthToken == LokiAuthForwardUserToken
}

// FlowCollectorLoki defines the desired state for FlowCollector's Loki client.
type FlowCollectorLoki struct {
//+kubebuilder:default:="http://loki:3100/"
Expand Down
24 changes: 0 additions & 24 deletions api/v1beta1/flowcollector_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ const (
DeploymentModelKafka = "KAFKA"
)

func (spec *FlowCollectorSpec) UseEBPF() bool { return spec.Agent.Type == AgentEBPF }
func (spec *FlowCollectorSpec) UseIPFIX() bool { return spec.Agent.Type == AgentIPFIX }
func (spec *FlowCollectorSpec) UseKafka() bool { return spec.DeploymentModel == DeploymentModelKafka }

// Please notice that the FlowCollectorSpec's properties MUST redefine one of the default
// values to force the definition of the section when it is not provided by the manifest.
// This will cause that the remaining default fields will be set according to their definition.
Expand Down Expand Up @@ -394,32 +390,12 @@ type FlowCollectorHPA struct {
Metrics []ascv2.MetricSpec `json:"metrics"`
}

func (spec *FlowCollectorHPA) Disabled() bool {
return spec.Status == HPAStatusDisabled
}

func (spec *FlowCollectorHPA) Enabled() bool {
return spec.Status == HPAStatusEnabled
}

const (
LokiAuthDisabled = "DISABLED"
LokiAuthUseHostToken = "HOST"
LokiAuthForwardUserToken = "FORWARD"
)

func (spec *FlowCollectorLoki) NoAuthToken() bool {
return spec.AuthToken == LokiAuthDisabled
}

func (spec *FlowCollectorLoki) UseHostToken() bool {
return spec.AuthToken == LokiAuthUseHostToken
}

func (spec *FlowCollectorLoki) ForwardUserToken() bool {
return spec.AuthToken == LokiAuthForwardUserToken
}

// FlowCollectorLoki defines the desired state for FlowCollector's Loki client.
type FlowCollectorLoki struct {
//+kubebuilder:default:="http://loki:3100/"
Expand Down
61 changes: 30 additions & 31 deletions controllers/consoleplugin/consoleplugin_objects.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@ const lokiCerts = "loki-certs"
const tokensPath = "/var/run/secrets/tokens/"

type builder struct {
namespace string
labels map[string]string
selector map[string]string
desired *flowslatest.FlowCollectorConsolePlugin
desiredLoki *flowslatest.FlowCollectorLoki
imageName string
cWatcher *watchers.CertificatesWatcher
namespace string
labels map[string]string
selector map[string]string
desired *flowslatest.FlowCollectorSpec
imageName string
cWatcher *watchers.CertificatesWatcher
}

func newBuilder(ns, imageName string, desired *flowslatest.FlowCollectorConsolePlugin, desiredLoki *flowslatest.FlowCollectorLoki, cWatcher *watchers.CertificatesWatcher) builder {
func newBuilder(ns, imageName string, desired *flowslatest.FlowCollectorSpec, cWatcher *watchers.CertificatesWatcher) builder {
version := helper.ExtractVersion(imageName)
return builder{
namespace: ns,
Expand All @@ -52,10 +51,9 @@ func newBuilder(ns, imageName string, desired *flowslatest.FlowCollectorConsoleP
selector: map[string]string{
"app": constants.PluginName,
},
desired: desired,
desiredLoki: desiredLoki,
imageName: imageName,
cWatcher: cWatcher,
desired: desired,
imageName: imageName,
cWatcher: cWatcher,
}
}

Expand All @@ -69,7 +67,7 @@ func (b *builder) consolePlugin() *osv1alpha1.ConsolePlugin {
Service: osv1alpha1.ConsolePluginService{
Name: constants.PluginName,
Namespace: b.namespace,
Port: b.desired.Port,
Port: b.desired.ConsolePlugin.Port,
BasePath: "/",
},
Proxy: []osv1alpha1.ConsolePluginProxy{{
Expand All @@ -79,7 +77,7 @@ func (b *builder) consolePlugin() *osv1alpha1.ConsolePlugin {
Service: osv1alpha1.ConsolePluginProxyServiceConfig{
Name: constants.PluginName,
Namespace: b.namespace,
Port: b.desired.Port,
Port: b.desired.ConsolePlugin.Port,
},
}},
},
Expand Down Expand Up @@ -134,7 +132,7 @@ func (b *builder) deployment(cmDigest string) *appsv1.Deployment {
Labels: b.labels,
},
Spec: appsv1.DeploymentSpec{
Replicas: &b.desired.Replicas,
Replicas: &b.desired.ConsolePlugin.Replicas,
Selector: &metav1.LabelSelector{
MatchLabels: b.selector,
},
Expand All @@ -144,7 +142,7 @@ func (b *builder) deployment(cmDigest string) *appsv1.Deployment {
}

func tokenPath(desiredLoki *flowslatest.FlowCollectorLoki) string {
if desiredLoki.UseHostToken() {
if helper.LokiUseHostToken(desiredLoki) {
return tokensPath + constants.PluginName
}
return ""
Expand All @@ -164,7 +162,7 @@ func buildArgs(desired *flowslatest.FlowCollectorConsolePlugin, desiredLoki *flo
"-frontend-config", filepath.Join(configPath, configFile),
}

if desiredLoki.ForwardUserToken() {
if helper.LokiForwardUserToken(desiredLoki) {
args = append(args, "-loki-forward-user-token")
}

Expand All @@ -179,7 +177,7 @@ func buildArgs(desired *flowslatest.FlowCollectorConsolePlugin, desiredLoki *flo
args = append(args, "--loki-ca-path", helper.GetCACertPath(&desiredLoki.TLS, lokiCerts))
}
}
if desiredLoki.UseHostToken() {
if helper.LokiUseHostToken(desiredLoki) {
args = append(args, "-loki-token-path", tokenPath(desiredLoki))
}
return args
Expand Down Expand Up @@ -216,12 +214,12 @@ func (b *builder) podTemplate(cmDigest string) *corev1.PodTemplateSpec {
},
}

args := buildArgs(b.desired, b.desiredLoki)
if b.desiredLoki != nil && b.desiredLoki.TLS.Enable && !b.desiredLoki.TLS.InsecureSkipVerify {
volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &b.desiredLoki.TLS, lokiCerts, b.cWatcher)
args := buildArgs(&b.desired.ConsolePlugin, &b.desired.Loki)
if b.desired != nil && b.desired.Loki.TLS.Enable && !b.desired.Loki.TLS.InsecureSkipVerify {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you still need to check b.desired.Loki != nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

b.desiredLoki was a pointer while b.desired.Loki is not, we only need to check b.desired here

volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &b.desired.Loki.TLS, lokiCerts, b.cWatcher)
}

if b.desiredLoki.UseHostToken() {
if helper.LokiUseHostToken(&b.desired.Loki) {
volumes, volumeMounts = helper.AppendTokenVolume(volumes, volumeMounts, constants.PluginName, constants.PluginName)
}

Expand All @@ -236,8 +234,8 @@ func (b *builder) podTemplate(cmDigest string) *corev1.PodTemplateSpec {
Containers: []corev1.Container{{
Name: constants.PluginName,
Image: b.imageName,
ImagePullPolicy: corev1.PullPolicy(b.desired.ImagePullPolicy),
Resources: *b.desired.Resources.DeepCopy(),
ImagePullPolicy: corev1.PullPolicy(b.desired.ConsolePlugin.ImagePullPolicy),
Resources: *b.desired.ConsolePlugin.Resources.DeepCopy(),
VolumeMounts: volumeMounts,
Args: args,
}},
Expand All @@ -260,9 +258,9 @@ func (b *builder) autoScaler() *ascv2.HorizontalPodAutoscaler {
Kind: "Deployment",
Name: constants.PluginName,
},
MinReplicas: b.desired.Autoscaler.MinReplicas,
MaxReplicas: b.desired.Autoscaler.MaxReplicas,
Metrics: b.desired.Autoscaler.Metrics,
MinReplicas: b.desired.ConsolePlugin.Autoscaler.MinReplicas,
MaxReplicas: b.desired.ConsolePlugin.Autoscaler.MaxReplicas,
Metrics: b.desired.ConsolePlugin.Autoscaler.Metrics,
},
}
}
Expand All @@ -281,7 +279,7 @@ func (b *builder) service(old *corev1.Service) *corev1.Service {
Spec: corev1.ServiceSpec{
Selector: b.selector,
Ports: []corev1.ServicePort{{
Port: b.desired.Port,
Port: b.desired.ConsolePlugin.Port,
Protocol: "TCP",
Name: "main",
}},
Expand All @@ -291,7 +289,7 @@ func (b *builder) service(old *corev1.Service) *corev1.Service {
// In case we're updating an existing service, we need to build from the old one to keep immutable fields such as clusterIP
newService := old.DeepCopy()
newService.Spec.Ports = []corev1.ServicePort{{
Port: b.desired.Port,
Port: b.desired.ConsolePlugin.Port,
Protocol: corev1.ProtocolUDP,
}}
return newService
Expand All @@ -313,9 +311,10 @@ func buildServiceAccount(ns string) *corev1.ServiceAccount {
// detect any configuration change
func (b *builder) configMap() (*corev1.ConfigMap, string) {
config := map[string]interface{}{
"portNaming": b.desired.PortNaming,
"quickFilters": b.desired.QuickFilters,
"portNaming": b.desired.ConsolePlugin.PortNaming,
"quickFilters": b.desired.ConsolePlugin.QuickFilters,
"alertNamespaces": []string{b.namespace},
"sampling": helper.GetSampling(b.desired),
}

configStr := "{}"
Expand Down
6 changes: 3 additions & 3 deletions controllers/consoleplugin/consoleplugin_reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (r *CPReconciler) Reconcile(ctx context.Context, desired *flowslatest.FlowC
}

// Create object builder
builder := newBuilder(ns, r.image, &desired.Spec.ConsolePlugin, &desired.Spec.Loki, r.CertWatcher)
builder := newBuilder(ns, r.image, &desired.Spec, r.CertWatcher)

if err = r.reconcilePlugin(ctx, builder, &desired.Spec); err != nil {
return err
Expand Down Expand Up @@ -190,7 +190,7 @@ func (r *CPReconciler) reconcileDeployment(ctx context.Context, builder builder,
if err := r.CreateOwned(ctx, newDepl); err != nil {
return err
}
} else if helper.DeploymentChanged(r.owned.deployment, newDepl, constants.PluginName, desired.ConsolePlugin.Autoscaler.Disabled(), desired.ConsolePlugin.Replicas, &report) {
} else if helper.DeploymentChanged(r.owned.deployment, newDepl, constants.PluginName, helper.HPADisabled(&desired.ConsolePlugin.Autoscaler), desired.ConsolePlugin.Replicas, &report) {
if err := r.UpdateOwned(ctx, r.owned.deployment, newDepl); err != nil {
return err
}
Expand Down Expand Up @@ -229,7 +229,7 @@ func (r *CPReconciler) reconcileHPA(ctx context.Context, builder builder, desire
defer report.LogIfNeeded(ctx)

// Delete or Create / Update Autoscaler according to HPA option
if desired.ConsolePlugin.Autoscaler.Disabled() {
if helper.HPADisabled(&desired.ConsolePlugin.Autoscaler) {
r.nobjMngr.TryDelete(ctx, r.owned.hpa)
} else {
newASC := builder.autoScaler()
Expand Down
31 changes: 19 additions & 12 deletions controllers/consoleplugin/consoleplugin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,17 @@ func TestContainerUpdateCheck(t *testing.T) {

//equals specs
plugin := getPluginConfig()
loki := &flowslatest.FlowCollectorLoki{URL: "http://loki:3100/", TenantID: "netobserv"}
builder := newBuilder(testNamespace, testImage, &plugin, loki, &certWatcher)
loki := flowslatest.FlowCollectorLoki{URL: "http://loki:3100/", TenantID: "netobserv"}
spec := flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder := newBuilder(testNamespace, testImage, &spec, &certWatcher)
old := builder.deployment("digest")
new := builder.deployment("digest")
report := helper.NewChangeReport("")
assert.False(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
assert.Contains(report.String(), "no change")

//wrong resources
plugin.Resources.Limits = map[corev1.ResourceName]resource.Quantity{
spec.ConsolePlugin.Resources.Limits = map[corev1.ResourceName]resource.Quantity{
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("500Gi"),
}
Expand All @@ -137,31 +138,32 @@ func TestContainerUpdateCheck(t *testing.T) {
old = new

//new pull policy
plugin.ImagePullPolicy = string(corev1.PullAlways)
spec.ConsolePlugin.ImagePullPolicy = string(corev1.PullAlways)
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.True(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
assert.Contains(report.String(), "Pull policy changed")
old = new

//new log level
plugin.LogLevel = "debug"
spec.ConsolePlugin.LogLevel = "debug"
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.True(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
assert.Contains(report.String(), "Args changed")
old = new

//new loki config
loki = &flowslatest.FlowCollectorLoki{URL: "http://loki:3100/", TenantID: "netobserv", TLS: flowslatest.ClientTLS{
loki = flowslatest.FlowCollectorLoki{URL: "http://loki:3100/", TenantID: "netobserv", TLS: flowslatest.ClientTLS{
Enable: true,
CACert: flowslatest.CertificateReference{
Type: "configmap",
Name: "cm-name",
CertFile: "ca.crt",
},
}}
builder = newBuilder(testNamespace, testImage, &plugin, loki, &certWatcher)
spec = flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder = newBuilder(testNamespace, testImage, &spec, &certWatcher)
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.True(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
Expand All @@ -170,7 +172,8 @@ func TestContainerUpdateCheck(t *testing.T) {

//new loki cert name
loki.TLS.CACert.Name = "cm-name-2"
builder = newBuilder(testNamespace, testImage, &plugin, loki, &certWatcher)
spec = flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder = newBuilder(testNamespace, testImage, &spec, &certWatcher)
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.True(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
Expand All @@ -179,7 +182,8 @@ func TestContainerUpdateCheck(t *testing.T) {

//test again no change
loki.TLS.CACert.Name = "cm-name-2"
builder = newBuilder(testNamespace, testImage, &plugin, loki, &certWatcher)
spec = flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder = newBuilder(testNamespace, testImage, &spec, &certWatcher)
new = builder.deployment("digest")
report = helper.NewChangeReport("")
assert.False(helper.PodChanged(&old.Spec.Template, &new.Spec.Template, constants.PluginName, &report))
Expand Down Expand Up @@ -215,7 +219,9 @@ func TestBuiltService(t *testing.T) {

//newly created service should not need update
plugin := getPluginConfig()
builder := newBuilder(testNamespace, testImage, &plugin, nil, &certWatcher)
loki := flowslatest.FlowCollectorLoki{URL: "http://foo:1234"}
spec := flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder := newBuilder(testNamespace, testImage, &spec, &certWatcher)
newService := builder.service(nil)
report := helper.NewChangeReport("")
assert.Equal(serviceNeedsUpdate(newService, &plugin, &report), false)
Expand All @@ -226,8 +232,9 @@ func TestLabels(t *testing.T) {
assert := assert.New(t)

plugin := getPluginConfig()
loki := &flowslatest.FlowCollectorLoki{URL: "http://foo:1234"}
builder := newBuilder(testNamespace, testImage, &plugin, loki, &certWatcher)
loki := flowslatest.FlowCollectorLoki{URL: "http://foo:1234"}
spec := flowslatest.FlowCollectorSpec{ConsolePlugin: plugin, Loki: loki}
builder := newBuilder(testNamespace, testImage, &spec, &certWatcher)

// Deployment
depl := builder.deployment("digest")
Expand Down
Loading