Hawk apiserver monitoring #133
Conversation
ayoub-belarbi
force-pushed
the
hawk-apiserver-monitoring
branch
2 times, most recently
from
b8127aa
to
52987c4
Compare
ayoub-belarbi
changed the title
ayoub-belarbi
force-pushed
the
hawk-apiserver-monitoring
branch
from
5444850
to
96e7b07
Compare
| Here an example: | ||
|
|
||
| ``` | ||
| cluster: |
There was a problem hiding this comment.
Perhaps we could create an example file
ayoub-belarbi
dismissed
diegoakechi’s stale review
Diego is on vacation and issue has been resolved
There was a problem hiding this comment.
I don't know if I have missed some discussions about this implementation, but I find many details that I don't really understand.
I think with the changes of this PR the deployments makes too many assumptions.
- The exporters are always deployed
- The ports are fixed
- We add a development mode (which i don't know if it's neccessary)
I'm not saying that we have to change them. I would only want to understand them
| {% endfor %} | ||
| - targets: | ||
| {% for ip in grains['monitored_services'] %} | ||
| - "{{ ip }}:8001" # 8801: hanadb exporter port |
There was a problem hiding this comment.
I don't like having fixed ports. Actually, the hana instance deployed in the second VM is not using this port.
More than that, the name wouldn't make any sense anymore. We are refering to machines here, not services...
I think that we need to consider many things here
There was a problem hiding this comment.
I think for moment we can keep the old part of service with Port number.
There was a problem hiding this comment.
I think for now, it's best if we enable the monitoring in both nodes by default and avoid specifying the IP of the hosts.
There was a problem hiding this comment.
if we add this, we should change monitored_services with monitored_ips
I think in future this solution is really less flexible, because imagine we will have 4 ips,
for all for IPS we will monitor the same serivces.
Before it was more fine grained. I think we have more duplicata but more control which imho better..
| - "{{ ip }}:8001" # 8801: hanadb exporter port | ||
| {% endfor %} | ||
|
|
||
| - job_name: node |
| {% endfor %} | ||
| - targets: | ||
| {% for ip in grains['monitored_services'] %} | ||
| - "{{ ip }}:8001" # 8801: hanadb exporter port |
There was a problem hiding this comment.
btw there is a typo on the comment .. 😁 8801 != 8001 port
{ ip }}:8001" # 8801: hanadb exporter port
There was a problem hiding this comment.
Thanks @ayoub-belarbi
More comments, questions, suggestions
| @@ -118,7 +118,7 @@ variable "pool" { | |||
| } | |||
|
|
|||
|
|
|||
| variable "monitored_services" { | |||
| variable "monitored_hosts" { | |||
| description = "HOST:PORT of service you want to monitor, it can contain same host with different ports number (diff services)" | |||
There was a problem hiding this comment.
We would need to update this doc
| @@ -36,8 +41,8 @@ ha_sap_deployment_repo = "" | |||
| #background = true | |||
|
|
|||
| # Monitoring: | |||
| # add here HOST:PORT. In this example we monitor on same hosts different services | |||
| monitored_services = ["192.168.110.X:8001", "192.168.110.X+1:8001", "192.168.110.X:9100", "192.168.110.X+1:9100"] | |||
| # Add here IP addresses of the hosts to be monitored: | |||
There was a problem hiding this comment.
The last colom maybe it's not needed here
There was a problem hiding this comment.
what do you mean exactly?
There was a problem hiding this comment.
Just that we might remove the last :. Never mind... XD
| @@ -26,6 +26,10 @@ cluster: | |||
| {% endif %} | |||
| resource_agents: | |||
| - SAPHanaSR | |||
| {% if grains.get('devel_mode') %} | |||
There was a problem hiding this comment.
As a temporary solution it might work, but eventually we will need to update this
| @@ -97,6 +97,11 @@ variable "ha_sap_deployment_repo" { | |||
| type = "string" | |||
| } | |||
|
|
|||
| variable "devel_mode" { | |||
There was a problem hiding this comment.
And now we have the devel_mode and qa_mode variables.
Could we just have mode or something similar.
Are they auto exclusive?
There was a problem hiding this comment.
I think that devel and qa mode are incompatible.. :(
There was a problem hiding this comment.
yes now we have both, but I'm not sure about the qa mode scope to be honest.
There was a problem hiding this comment.
Ok, this is something we will need to manage... Maybe unify them to a single variable.
We can work on it in a next PR
| @@ -1,3 +1,12 @@ | |||
| {% if grains.get('devel_mode') %} | |||
There was a problem hiding this comment.
Couldn't we just add this operation in the pre_installation states?
I mean, they do the same...
Just add the if condition there and depending the path set one priority or other
There was a problem hiding this comment.
that caused some problems during the sap pattern installation if I recall correctly since it happens before the HA one.
There was a problem hiding this comment.
mmm, Ok, it can be.
Anyway, the devel_mode is kind of strange in that case as it only affects to a single package.
| @@ -4,6 +4,7 @@ ha-factory-repo: | |||
| - baseurl: {{ grains['ha_sap_deployment_repo'] }} | |||
| - gpgautoimport: True | |||
| - priority: 110 | |||
There was a problem hiding this comment.
As mentioned in other comment, wouldn't be better to manage the priority (and the new devel_mode variable) here?
There was a problem hiding this comment.
that caused some problems during the sap pattern installation if I recall correctly since it happens before the HA one.
|
please open issue in github as follow-up (if any) and reference to this PR so we can remember and keep track of them. |
No description provided.