Add settings.RECEPTOR_LOG_LEVEL, update work signing key path #14098
Conversation
fosterseth
force-pushed
the
receptor_log_level
branch
2 times, most recently
from
7f531e1 to
31d94e9
Compare
fosterseth
force-pushed
the
receptor_log_level
branch
from
9c36f5d to
4b3cdf1
Compare
awx/main/tasks/receptor.py
Outdated
| @task() | ||
| def write_receptor_config(): | ||
| current_config = read_receptor_config() # this grabs lock |
There was a problem hiding this comment.
Isn't this dangerous? By using the current config as a starter, aren't you kind of cargo-culting the config in an automated way? That could create some weird problems of config drift down the line.
Also, I'd like to move in the direction of generating receptor_config in a separate method so that we can test it. Before this, it looked like all we needed to test it was the Instance and InstanceLink objects in the database. We can still test this after your restructuring here, but that just means moving the "starter config" to the tests.
There was a problem hiding this comment.
Looking down at the line receptor_config.append(peer), I can see this not working at all since you're not checking in advance whether the link already exists in the config, so it could end up adding it every time it rewrites the config.
There was a problem hiding this comment.
cargo culting (uncountable) (chiefly computing) An approach that copies an existing successful approach without properly analysing and understanding it.
There was a problem hiding this comment.
The logic here strips out any "peer" entry, and we simply readd them later. So in a way we are starting "fresh" without having the hard coded template.
I think I will just revert this part back for now and we can figure out a better solution later.
|
@fosterseth i think the safer approach is to just go ahead and use the old method of templating out the receptor config file and add the ability to config log level instead of trying to rewrite the way that we generate the receptor config file I understand that there's currently a PITA that when u change the file names you need to change in both awx and awx-operator we can address this separately if desired. |
fosterseth
force-pushed
the
receptor_log_level
branch
from
4b3cdf1 to
3875656
Compare
| {'node': {'firewallrules': [{'action': 'reject', 'tonode': settings.CLUSTER_HOST_ID, 'toservice': 'control'}]}}, | ||
| {'control-service': {'service': 'control', 'filename': '/var/run/receptor/receptor.sock', 'permissions': '0660'}}, | ||
| {'work-command': {'worktype': 'local', 'command': 'ansible-runner', 'params': 'worker', 'allowruntimeparams': True}}, | ||
| {'work-signing': {'privatekey': '/etc/receptor/signing/work-private-key.pem', 'tokenexpiration': '1m'}}, | ||
| {'work-signing': {'privatekey': '/etc/receptor/work_private_key.pem', 'tokenexpiration': '1m'}}, |
There was a problem hiding this comment.
Is this change to be paired with ansible/awx-operator#1442?
The new location looks like it's the correct one, I agree with the change, but I'd like a little bit better linking and description of the changes here, since it's not all all that this should happen from the title of this PR, and the associated changes & issue are not immediately discoverable.
There was a problem hiding this comment.
Never mind my prior comments, this was rendered defunct by #14151, but did not show due to a git issue. This line is not actually changed with this merge.
AlanCoding
changed the title
|
I'm going to merge this myself because of the stated compatibility concerns, and because Seth isn't around right now. |
SUMMARY
Offers ability to set log level of receptor (k8s only)
Intended to be paired with ansible/awx-operator#1444
Should be backwards compatible with older operators too.
Instead of hard-coding part of the receptor config file, we can just read the current file (and pop the -peers and -listeners off)
ISSUE TYPE
COMPONENT NAME
AWX VERSION