Add initial watcher api conf generation #27
Conversation
|
Skipping CI for Draft Pull Request. |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/69a6a581343c4d59884c53128154ddc6 ✔️ noop SUCCESS in 0s |
cescgina
force-pushed
the
watcherapi_generate_config
branch
from
78598be to
d27e084
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/843017e005094e32973188fa06cc2a8a ✔️ noop SUCCESS in 0s |
cescgina
force-pushed
the
watcherapi_generate_config
branch
3 times, most recently
from
a14f1e4 to
cfd6038
Compare
cescgina
force-pushed
the
watcherapi_generate_config
branch
from
cfd6038 to
8e740bf
Compare
cescgina
force-pushed
the
watcherapi_generate_config
branch
2 times, most recently
from
472a393 to
48a6210
Compare
cescgina
changed the title
cescgina
force-pushed
the
watcherapi_generate_config
branch
2 times, most recently
from
6584afa to
6562865
Compare
cescgina
force-pushed
the
watcherapi_generate_config
branch
from
6562865 to
d03f0cc
Compare
cescgina
force-pushed
the
watcherapi_generate_config
branch
2 times, most recently
from
98252f6 to
ba040a3
Compare
cescgina
force-pushed
the
watcherapi_generate_config
branch
from
ba040a3 to
40953e8
Compare
|
marking it ready for review again, as I added the missing config files (at leas I think I did). I expect the files will need tweak when I create the deployment and they're actually used |
templates/watcher/config/main
Outdated
There was a problem hiding this comment.
Comment for future usage of this config, the openstack-watcher-api package provides already a wsgi file in /usr/bin/watcher-api-wsgi that can be used for the httpd configuration. It has exactly the same content.
There may be a reason to maintain it in the operators (I've seen others are doing the same), but I think it may be worthy to test if we could switch to the wsgi executable provided upstream (which is the one in the packages) instead of keeping it here.
In any case, we can discuss details about configuration later, when creating the deployment.
There was a problem hiding this comment.
that's interesting, I didn't know that. I'll do what you propose, leave this file in, and once I have a working deployment see if we can reuse the existing file
|
/lgtm |
cescgina
force-pushed
the
watcherapi_generate_config
branch
from
40953e8 to
c1ad7d8
Compare
Implement an initial version of watcher config generation. This change adds a watcher config template and generates a secret with the templated config. Some fields that require changes in the watcher controller like the transporturl and memcached servers. This change also modifies the WatcherAPI functional tests so the WatcherAPI instances use a different name that the Watcher one, so it's easier to debug.
cescgina
force-pushed
the
watcherapi_generate_config
branch
from
c1ad7d8 to
c957afb
Compare
|
/lgtm |
| lock_path = /var/lib/watcher/tmp | ||
|
|
||
| [watcher_datasources] | ||
| datasources = ceilometer |
There was a problem hiding this comment.
actually we should not merge with this as Takashi is actually removing with https://review.opendev.org/c/openstack/watcher/+/937625
really we want the default to be prometheus? it is up to the deployer what the default is here i think.
There was a problem hiding this comment.
that's a good point. But considering that prometheus support has not yet landed upstream, if we merge with that, it'll probably cause an error once I create the deployment and the service starts running. Probably the best option for now is leave ceilometer and add a comment explaining the situation and then move the prometheus as soon as we support it
There was a problem hiding this comment.
best thing is to leave empty, so strategies will check for all available datasources.
There was a problem hiding this comment.
+1 to go with ceilometer now.
There was a problem hiding this comment.
WRT:
really we want the default to be prometheus? it is up to the deployer what the default is here i think.
I think we should leave a meaningful default value. In this case, the deployer using openstack-k8s-operators have no option to deploy monasca, gnocchi and will be limited to prometheus, so I think we should default to prometheus.
best thing is to leave empty, so strategies will check for all available datasources.
What I've seen in logs is that, when all the datasources are tried, it tries multiple times (10 by default) with a certain timeout on each one. I think this is a waste of resources for a case where we know (or we will know) that prometheus is really the only available data source (as deployed by operators). So, I'd be prescriptive in the default configuration.
In any case, we are still not running any service and we will have follow-up PRs where we will do the actual deployment. We can retake this discussion by then.
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amoralej The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 3466392
into
openstack-k8s-operators:main
Implement an initial version of watcher config generation. This change
adds a watcher config template and generates a secret with the templated
config. Some fields that require changes in the watcher controller like
the transporturl and memcached servers.
This change also modifies the WatcherAPI functional tests so the
WatcherAPI instances use a different name that the Watcher one, so it's
easier to debug.
Related: OSPRH-11483