Stop injecting python code in the configuration #52
Conversation
cescgina
force-pushed
the
remove_api_python_script
branch
2 times, most recently
from
1e9466d to
fc5ca5d
Compare
There was a problem hiding this comment.
so this is missing several other refernces to the removed file
3466392#diff-bd98cc39be1777271bfb5fb5bacf8322d12b3cab28168c9b1e78720fce771083R24-R30
the mime.conf should also not be required. so this is incomplete
Instead of passing the api main script through a secret, use the one installed in the container. Also reduce the number of processes of the api running from 8 to 2 (one process for the hearbeat and the other to listen to requests) and remove the mime.conf file.
cescgina
force-pushed
the
remove_api_python_script
branch
from
fc5ca5d to
3cfc1ba
Compare
| @@ -32,6 +33,7 @@ AccessFileName .htaccess | |||
| LogLevel debug | |||
| EnableSendfile On | |||
|
|
|||
| TypesConfig /etc/mime.types | |||
There was a problem hiding this comment.
so previously we were getting mime.conf becuase of the Include on the next line 37...
but you now removed that, so we need to configure this TypesConfig here explicitly ?
(for my own understanding)
There was a problem hiding this comment.
exactly, that's what other operators do, e.g nova https://github.com/openstack-k8s-operators/nova-operator/blob/0933c978bad435a91ec3fff1121f762a93e9bba9/templates/novaapi/config/httpd.conf#L13 or keystone https://github.com/openstack-k8s-operators/keystone-operator/blob/ced167413cf979dfdbaec16468115894af26384c/templates/keystoneapi/config/httpd.conf#L13
| @@ -31,9 +31,9 @@ | |||
|
|
|||
There was a problem hiding this comment.
Although it's unrelated with removing the main file from here. There are another couple of issues related to httpd config:
- I've realized we are not getting the apache access and error logs from the pods. I think we should set errorlog, customlog, etc... to stdout so that openshift shows it in oc logs -c watcher-api . See:
and
- We should provide a valid ssl.conf file as in https://github.com/openstack-k8s-operators/nova-operator/blob/main/templates/novaapi/config/ssl.conf . I think that would allow us to revert Add httpd fix to watcher api containerfile tcib#236
We can fix it in follow-up if you prefer.
There was a problem hiding this comment.
thanks that's a good point about the logs, I have another PR prepared after this one with more fixes for the config files, I can add it to that one. Simlarly, I have the ssl file added in a different patch where I' working to add more TLS support.
There was a problem hiding this comment.
good callout on the logging ya we should do that but i agree that can be in a separate pr.
|
/lgtm |
| @@ -31,9 +31,9 @@ | |||
|
|
|||
There was a problem hiding this comment.
good callout on the logging ya we should do that but i agree that can be in a separate pr.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: SeanMooney 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 7559f12
into
openstack-k8s-operators:main
Instead of passing the api main script through a secret, use the one
installed in the container.