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.

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

Updating k8s pod and container templates with no host ips and realistic hostnames #111

Merged
merged 10 commits into from
Dec 4, 2023

Conversation

gizas
Copy link
Contributor

@gizas gizas commented Oct 6, 2023

TSDB nightly tests demand a more realistic set of data generated. As part of this enhancement we update kubernetes pod and container templates with following:

How to test:

  1. Checkout this PR
  2. Produce pod and container samples:
./elastic-integration-corpus-generator-tool generate-with-template ./assets/templates/kubernetes.pod/gotext.tpl ./assets/templates/kubernetes.pod/fields.yml -c ./assets/templates/kubernetes.pod/configs.yml -y gotext -t 2

./assets/templates/kubernetes.container/gotext.tpl ./assets/templates/kubernetes.container/fields.yml -c ./assets/templates/kubernetes.container/configs.yml -y gotext -t 2

Check that host.ips and host.mac fields are removed
Check the node names created and have the same value as hostnames.

Screenshot

Screenshot 2023-10-06 at 1 09 03 PM

@gizas gizas added the Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team label Oct 6, 2023
@gizas gizas self-assigned this Oct 6, 2023
"fe80::5d:14ff:fec1:2d3a"
],
"name": "{{ $agentName }}",
"name": "{{- $nodeid := generate "nodeid" -}}",
Copy link
Contributor

Choose a reason for hiding this comment

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

beware, if you invoke generate "nodeid" two times in the same template you should end up with two different values

you should: because looking at the screenshot you posted it does not seems so, but this is something to investigate, since it seems to be bug :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok once per file. Then I refer to nodeid variable

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

I think the host name doesn't need to be to same as node name? As long as each node name uses the same host name?

@@ -103,12 +104,12 @@
},
"node":{
"uid": "{{ $uId }}" ,
"hostname":"{{ $agentName }}.c.elastic-obs-integrations-dev.internal",
"name":"{{ $agentName }}-{{ $rangeofid }}",
"hostname":"node-{{ $nodeid }}",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe host-{{ $nodeid }}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -214,7 +215,7 @@
"dataset":"kubernetes.container"
},
"host":{
"hostname":"{{ $agentName }}",
"hostname":"node-{{ $nodeid }}",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe host-?

@@ -28,12 +29,12 @@
"kubernetes": {
"node":{
"uid": "{{ $uId }}" ,
"hostname":"{{ $agentName }}.c.elastic-obs-integrations-dev.internal",
"name":"{{ $agentName }}-{{ $rangeofid }}",
"hostname":"node-{{ $nodeid }}",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe host-?

@aspacca
Copy link
Contributor

aspacca commented Oct 10, 2023

@gizas could you please move the files of each folder inside the schema-b subfolder? thanks :)

@gizas
Copy link
Contributor Author

gizas commented Oct 10, 2023

@gizas could you please move the files of each folder inside the schema-b subfolder? thanks :)

Done @aspacca

…generator-tool into update_k8s_pod_container_templatesforTSDB
@@ -45,7 +45,7 @@ fields:
- name: container.name
enum: ["web", "default-http-backend", "dnsmasq", "csi-driver", "web", "web", "web", "prometheus", "konnectivity-agent", "sidecar", "kubedns", "metrics-server-nanny", "web", "web", "fluentbit", "autoscaler", "gke-metrics-agent", "elastic-agent", "web", "kube-state-metrics", "metrics-server", "fluentbit", "elastic-agent", "web", "prometheus-to-sd-exporter"]
- name: timedate
# Add more dates below in case you want your data to be spread
# Add more dates below in case you want your data to be spread.Those are fixed because this is required in the nightly tests of Rally
Copy link
Contributor

Choose a reason for hiding this comment

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

can you explain about this?
will it be fixed "forever", or only for the current nightly tests of rally.
what happens if we use timestamp with period: 2880m (two days) like I did here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is fixed for the current nightly tests of rally. We have specific visualisations and we want our data to be in specific range in order our queries to be be always specific

Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to change the visualisation range to be -2d/now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aspacca
I have made some tests today and from generator tool side we can change the timedate to date type with [period: 48h] and all seem ok. So I dont see an issue with this now

Moreover on the rally side we have hardcoded the values of visualisations here

So we need to find a way to import to calculate the new itervals and end_time based on the track provided.
cc @martijnvg for visibility

@ruflin
Copy link
Contributor

ruflin commented Nov 20, 2023

I assume eventually we will move these files over to the k8s package?

@gizas
Copy link
Contributor Author

gizas commented Nov 20, 2023

I assume eventually we will move these files over to the k8s package?

This is the purpose of 8443 and 8444

@gizas
Copy link
Contributor Author

gizas commented Dec 4, 2023

@aspacca and @martijnvg I think this can also be merged now as it has been tested in various places and different PRs.
Please any approvals to merge?

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

👍

…generator-tool into update_k8s_pod_container_templatesforTSDB
@gizas gizas merged commit 605bde0 into main Dec 4, 2023
2 checks passed
@gizas gizas deleted the update_k8s_pod_container_templatesforTSDB branch December 4, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants