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

feat: implement extra_settings_files #1836

Merged
merged 6 commits into from
May 23, 2024

Conversation

kurokobo
Copy link
Contributor

@kurokobo kurokobo commented Apr 22, 2024

SUMMARY

Closes #1834

This PR add a new paramerter extra_settings_files for AWX CR that allows users to add extra settings through python (*.py) files from ConfigMaps or Secrets.

This is helpful when users are in following situations that are difficult to solve by existing extra_settings:

  • The extra_settings is very long and hard to manage
  • Want to pass dictionaries or lists which written in readable multiline codes
  • Doesn't want to consider quoted-quotes in extra_settings
  • Want to manage extra_settings as sepalated multiple files
  • Want to pass values from Secrets

Design:

  • Users can add extra settings by mounting *.py files from ConfigMaps or Secrets
  • This feature mounts specified *.py files under /etc/tower/conf.d in following pods/jobs/cronjobs which already have settngs.py
    • The task and web pods
    • The migration job
    • The metrics-utility-gather cronjobs and metrics-utility-report cronjobs
  • Users can specify multiple ConfigMaps and Secrets
  • Users can include multiple files in a single ConfigMap or Secret

Restrictions:

ISSUE TYPE
  • New or Enhanced Feature
ADDITIONAL INFORMATION

Tested by deploying custom Operator:

IMG=registry.example.com/ansible/awx:extra_settings_files \
DEFAULT_AWX_VERSION=24.2.0 \
NAMESPACE=awx \
make docker-build docker-push deploy

Example demo ConfigMaps and Secrets:

Since this is just for demonstration purposes, included settings and its value have no meaning.

---
apiVersion: v1
kind: ConfigMap
metadata:
  namespace: awx
  name: demo-configmap-01
data:
  democonfigmap01_01.py: |
    UI_NEXT = False
    AUTOMATION_ANALYTICS_GATHER_INTERVAL = 1800
  democonfigmap01_02.py: |
    REMOTE_HOST_HEADERS = [
      "HTTP_X_FORWARDED_FOR",
      "REMOTE_ADDR",
      "REMOTE_HOST",
    ]
---
apiVersion: v1
kind: ConfigMap
metadata:
  namespace: awx
  name: demo-configmap-02
data:
  democonfigmap02_01.py: |
    AUTOMATION_ANALYTICS_URL = "https://awesome.demo.example.com"
    GALAXY_TASK_ENV = {
      "ANSIBLE_FORCE_COLOR": "false",
      "GIT_SSH_COMMAND": "ssh -o StrictHostKeyChecking=no",
      "EXTRA_SETTINGS_FROM_CONFIGMAP": "hello awx community",
    }
---
apiVersion: v1
kind: Secret
metadata:
  namespace: awx
  name: demo-secret-01
stringData:
  demosecret01_01.py: |
    TOWER_URL_BASE = "https://extra.settings.from.secret.example.com"
    ACTIVITY_STREAM_ENABLED = False
  demosecret01_02.py: |
    AWX_ISOLATION_SHOW_PATHS = [
      "/etc/pki/ca-trust:/etc/pki/ca-trust:O",
      "/usr/share/pki:/usr/share/pki:O",
      "/tmp/awx/demo:/tmp/awx/demo:0",
    ]
---
apiVersion: v1
kind: Secret
metadata:
  namespace: awx
  name: demo-secret-02
stringData:
  demosecret02_01.py: |
    SUBSCRIPTIONS_PASSWORD = "my-subscription-password123!"
    REDHAT_PASSWORD = "my-redhat-password123!"

Example CR:

---
apiVersion: awx.ansible.com/v1beta1
kind: AWX
metadata:
  namespace: awx
  name: awx
spec:
  service_type: nodeport
  nodeport_port: 30080

  extra_settings_files:
    configmaps:
      - name: demo-configmap-01
        key: democonfigmap01_01.py
      - name: demo-configmap-01
        key: democonfigmap01_02.py
      - name: demo-configmap-02
        key: democonfigmap02_01.py
    secrets:
      - name: demo-secret-01
        key: demosecret01_01.py
      - name: demo-secret-01
        key: demosecret01_02.py
      - name: demo-secret-02
        key: demosecret02_01.py

Results

$ kubectl -n awx get deployment/awx-task -o json | jq -r  '.spec.template.spec.volumes[].name' | grep demo
awx-democonfigmap01-01-py-configmap
awx-democonfigmap01-02-py-configmap
awx-democonfigmap02-01-py-configmap
awx-demosecret01-01-py-secret
awx-demosecret01-02-py-secret
awx-demosecret02-01-py-secret

$ kubectl -n awx exec deployment/awx-task -- ls -l /etc/tower/conf.d
total 36
-rw-r--r--. 1 root root 613 Apr 22 06:20 credentials.py
-rw-r--r--. 1 root root  60 Apr 22 06:20 democonfigmap01_01.py
-rw-r--r--. 1 root root  86 Apr 22 06:20 democonfigmap01_02.py
-rw-r--r--. 1 root root 232 Apr 22 06:20 democonfigmap02_01.py
-rw-r--r--. 1 root root  98 Apr 22 06:20 demosecret01_01.py
-rw-r--r--. 1 root root 146 Apr 22 06:20 demosecret01_02.py
-rw-r--r--. 1 root root  99 Apr 22 06:20 demosecret02_01.py
-rw-r--r--. 1 root root 262 Apr 22 06:20 execution_environments.py
-rw-r--r--. 1 root root  91 Apr 22 06:20 ldap.py

$ kubectl -n awx exec deployment/awx-task -- bash -c 'find /etc/tower/conf.d -name 'demo*.py' | xargs grep -H .'
/etc/tower/conf.d/democonfigmap01_02.py:REMOTE_HOST_HEADERS = [
/etc/tower/conf.d/democonfigmap01_02.py:  "HTTP_X_FORWARDED_FOR",
/etc/tower/conf.d/democonfigmap01_02.py:  "REMOTE_ADDR",
/etc/tower/conf.d/democonfigmap01_02.py:  "REMOTE_HOST",
/etc/tower/conf.d/democonfigmap01_02.py:]
/etc/tower/conf.d/democonfigmap02_01.py:AUTOMATION_ANALYTICS_URL = "https://awesome.demo.example.com"
/etc/tower/conf.d/democonfigmap02_01.py:GALAXY_TASK_ENV = {
/etc/tower/conf.d/democonfigmap02_01.py:  "ANSIBLE_FORCE_COLOR": "false",
/etc/tower/conf.d/democonfigmap02_01.py:  "GIT_SSH_COMMAND": "ssh -o StrictHostKeyChecking=no",
/etc/tower/conf.d/democonfigmap02_01.py:  "EXTRA_SETTINGS_FROM_CONFIGMAP": "hello awx community",
/etc/tower/conf.d/democonfigmap02_01.py:}
/etc/tower/conf.d/democonfigmap01_01.py:UI_NEXT = False
/etc/tower/conf.d/democonfigmap01_01.py:AUTOMATION_ANALYTICS_GATHER_INTERVAL = 1800
/etc/tower/conf.d/demosecret02_01.py:SUBSCRIPTIONS_PASSWORD = "my-subscription-password123!"
/etc/tower/conf.d/demosecret02_01.py:REDHAT_PASSWORD = "my-redhat-password123!"
/etc/tower/conf.d/demosecret01_02.py:AWX_ISOLATION_SHOW_PATHS = [
/etc/tower/conf.d/demosecret01_02.py:  "/etc/pki/ca-trust:/etc/pki/ca-trust:O",
/etc/tower/conf.d/demosecret01_02.py:  "/usr/share/pki:/usr/share/pki:O",
/etc/tower/conf.d/demosecret01_02.py:  "/tmp/awx/demo:/tmp/awx/demo:0",
/etc/tower/conf.d/demosecret01_02.py:]
/etc/tower/conf.d/demosecret01_01.py:TOWER_URL_BASE = "https://extra.settings.from.secret.example.com"
/etc/tower/conf.d/demosecret01_01.py:ACTIVITY_STREAM_ENABLED = False

We can see from the UI that the settings have been changed. This is an excerpt:

image

To ensure there is no side-effect, I have also confirmed the minimal AWX CR without extra_settings_files can be deployed:

---
apiVersion: awx.ansible.com/v1beta1
kind: AWX
metadata:
  namespace: awx
  name: awx-demo
spec:
  service_type: nodeport
  nodeport_port: 30081
TODO
  • Documentation

@kurokobo
Copy link
Contributor Author

@rooftopcellist @TheRealHaoLiu
I have implemented the idea for now. I would like to have your opinion 😃
Sorry for this but I can't test changes for metrics-utility things on my side since I don't have any environment that metrics-utility connects to.

@kurokobo kurokobo marked this pull request as ready for review April 24, 2024 13:30
@kurokobo
Copy link
Contributor Author

@rooftopcellist @TheRealHaoLiu
Updated docs:

  • Add documentation about extra_settings_files and modify exsiting extra_settings documentation
  • Remove examples that mounts *.py files by extra_volumes and *__volume_mounts

Now I've marked this PR as ready for review. I am not sure if the parameter names and dictionary structure are ideally designed, but I think it allows for flexibility of use.

Any feedbacks are welcome! Of course feel free to reject this if this is not required for you. Thanks😃

@kurokobo kurokobo force-pushed the extra_settings_files branch from 12a25b9 to cb75ae5 Compare April 24, 2024 13:37
@kurokobo
Copy link
Contributor Author

kurokobo commented Apr 24, 2024

Documentation previews

localhost_8000_user-guide_advanced-configuration_extra-settings html (2)
localhost_8000_user-guide_advanced-configuration_custom-volume-and-volume-mount-options html

@fosterseth
Copy link
Member

fosterseth commented May 15, 2024

if the same setting is set in both extra_settings an in one of the .py files, how does that interaction work?

Also, the API blocks changing any setting if that setting is in extra_settings in the AWX spec, and I'm wondering if that same logic should apply to settings in the .py files

@kurokobo
Copy link
Contributor Author

@fosterseth
Hi, thanks for the comment.

if the same setting is set in both extra_settings an in one of the .py files, how does that interaction work?

Including any *.py files are by django-split-settings in production.py in AWX, so,

  1. Include settings.py which contains all settings from extra_settings first,
  2. Then include any /etc/tower/conf.d/*.py from extra_settings_files which overrides any settings insettings.py

Note that if two or more /etc/tower/conf.d/*.py files contain the same key, it would be difficult to predict which would be adopted due to the following behavior:

Note that files are included in the order that glob returns them, probably in the same order as what ls -U would list them. The files are NOT in alphabetical order.
https://github.com/wemake-services/django-split-settings

Also, the API blocks changing any setting if that setting is in extra_settings in the AWX spec, and I'm wondering if that same logic should apply to settings in the .py files

Yes, AFAIK the same logic will be applied for any /etc/tower/conf.d/*.py files, since both settings.py and *.py are included by above single include() in production.py, they should all be read-only as well.

@kurokobo kurokobo force-pushed the extra_settings_files branch from 084ea66 to fa072c4 Compare May 16, 2024 12:39
@kurokobo
Copy link
Contributor Author

Conflict resolved and some notes are added that @fosterseth pointed out 😃

@rooftopcellist
Copy link
Member

@kurokobo we have reviewed this PR and are ready to merge it, but it looks like there are some conflicts. Could you please rebase? I think that will resolve the CI failure too.

@kurokobo kurokobo force-pushed the extra_settings_files branch from 759c408 to 89013e4 Compare May 23, 2024 12:27
@kurokobo
Copy link
Contributor Author

@rooftopcellist
Thanks, resolved conflicts and rebased!

@kurokobo
Copy link
Contributor Author

Hmm 🤔
Something was happened in init-receptor?

2024-05-23T12:58:33.5143827Z     "msg": "Unable to retrieve log from Pod due to: container \"example-awx-ee\" in pod \"example-awx-task-65cfbfbd68-t5c5t\" is waiting to start: PodInitializing"
2024-05-23T12:58:33.5146601Z }
2024-05-23T12:58:33.5150069Z 
2024-05-23T12:58:33.5156040Z PLAY RECAP *********************************************************************
2024-05-23T12:58:33.5159721Z localhost                  : ok=42   changed=9    unreachable=0    failed=1    skipped=2    rescued=2    ignored=0kuro@kuro-dev01:~/work/kurokobo/private-odc-moderator/dify/docker$ 
2024-05-23T12:58:10.0399069Z                 "initContainerStatuses": [
2024-05-23T12:58:10.0401984Z                     {
2024-05-23T12:58:10.0405443Z                         "containerID": "containerd://757fa0dd390cc8246372831c691312be17ca18a33b4ff183433488d73ef1897f",
2024-05-23T12:58:10.0407640Z                         "image": "quay.io/ansible/awx:24.4.0",
2024-05-23T12:58:10.0411161Z                         "imageID": "quay.io/ansible/awx@sha256:62df9e77fef859f172c51c3e564d8a3ac03a5b32d4f4c398501eee65b9967633",
2024-05-23T12:58:10.0413551Z                         "lastState": {},
2024-05-23T12:58:10.0416360Z                         "name": "init-database",
2024-05-23T12:58:10.0419126Z                         "ready": true,
2024-05-23T12:58:10.0421940Z                         "restartCount": 0,
2024-05-23T12:58:10.0424703Z                         "started": false,
2024-05-23T12:58:10.0427582Z                         "state": {
2024-05-23T12:58:10.0430498Z                             "terminated": {
2024-05-23T12:58:10.0434083Z                                 "containerID": "containerd://757fa0dd390cc8246372831c691312be17ca18a33b4ff183433488d73ef1897f",
2024-05-23T12:58:10.0435949Z                                 "exitCode": 0,
2024-05-23T12:58:10.0439132Z                                 "finishedAt": "2024-05-23T12:57:12Z",
2024-05-23T12:58:10.0441800Z                                 "reason": "Completed",
2024-05-23T12:58:10.0444292Z                                 "startedAt": "2024-05-23T12:54:20Z"
2024-05-23T12:58:10.0446722Z                             }
2024-05-23T12:58:10.0449269Z                         }
2024-05-23T12:58:10.0452118Z                     },
2024-05-23T12:58:10.0454833Z                     {
2024-05-23T12:58:10.0458148Z                         "image": "quay.io/ansible/awx-ee:24.4.0",
2024-05-23T12:58:10.0460556Z                         "imageID": "",
2024-05-23T12:58:10.0463573Z                         "lastState": {},
2024-05-23T12:58:10.0466700Z                         "name": "init-receptor",
2024-05-23T12:58:10.0469385Z                         "ready": false,
2024-05-23T12:58:10.0472413Z                         "restartCount": 0,
2024-05-23T12:58:10.0475477Z                         "started": false,
2024-05-23T12:58:10.0478455Z                         "state": {
2024-05-23T12:58:10.0481321Z                             "waiting": {
2024-05-23T12:58:10.0484336Z                                 "reason": "PodInitializing"
2024-05-23T12:58:10.0487156Z                             }
2024-05-23T12:58:10.0489835Z                         }
2024-05-23T12:58:10.0492564Z                     }
2024-05-23T12:58:10.0495279Z                 ],

@TheRealHaoLiu
Copy link
Member

darn my latest debug artifact gathering for the molecule test does not gather log from init container... >.<
https://github.com/ansible/awx-operator/blob/devel/molecule/default/verify.yml/#L44-L49

@kurokobo
Copy link
Contributor Author

Since it succeeded in #1867, I can't imagine it well, but this PR may be the cause of the CI failure.

Before digging into this deeper, could you please try to re-run CI by closing / reopening this PR?

@kurokobo
Copy link
Contributor Author

Failed in very early stage 😞

Run ansible-galaxy collection install -r molecule/requirements.yml
Starting galaxy collection install process
Process install dependency map
ERROR! Error when getting collection version metadata for community.general:9.0.0 from default (https://galaxy.ansible.com/api/) (HTTP Code: 530, Message:  Code: Unknown)
Error: Process completed with exit code 1.

@TheRealHaoLiu
Copy link
Member

i rerun it again and it's successful... we been observing flakiness in our CI lately and i been trying to track it down (so far no success...)

@rooftopcellist rooftopcellist enabled auto-merge (squash) May 23, 2024 17:40
@rooftopcellist rooftopcellist merged commit 56df327 into ansible:devel May 23, 2024
7 checks passed
@kurokobo kurokobo deleted the extra_settings_files branch May 24, 2024 01:07
@kurokobo
Copy link
Contributor Author

@TheRealHaoLiu @rooftopcellist
Thanks for working on this, I don't think there is a developer out there who has not been plagued by flakiness😞
Resources for GHA are not very plentiful, so it would be difficult to reduce the occurrence to zero. It might be better to automate retries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow adding extra settings files from configmaps or secrets
4 participants