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

Add indirect node counting setting #15799

Open
wants to merge 4 commits into
base: devel
Choose a base branch
from

Conversation

jessicamack
Copy link
Member

SUMMARY

Point flag at setting to make it easier to change on the fly.

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • Other
AWX VERSION

ADDITIONAL INFORMATION

Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.25%. Comparing base (c418bc0) to head (c5ff67a).

✅ All tests successful. No failed tests found.

Copy link
Member

@AlanCoding AlanCoding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would seem to undermine the point of the flag source flags.sources.SettingsFlagsSource which is already enabled by default. If I understood correctly, this could be addressed by documenting what we already have. Also, this solution will create referential problems.

@pb82
Copy link
Contributor

pb82 commented Jan 31, 2025

@jessicamack @AlanCoding there is another proposal for how to handle feature flags at install time: https://handbook.eng.ansible.com/proposals/0068-Feature-flags-configuration-at-install-time

My understanding is that this is temporary, until we have Dynaconf.

@AlanCoding
Copy link
Member

I don't think the load order works.

https://github.com/ansible/awx/blob/devel/awx/settings/production.py

This first runs

from .defaults import *  # NOQA

So at that point you have in locals

INDIRECT_NODE_COUNTING_ENABLED = False
FLAGS = {'FEATURE_INDIRECT_NODE_COUNTING_ENABLED': [{'condition': 'boolean', 'value': False}]}

Then, in a later line

# Attempt to load settings from /etc/tower/settings.py first, followed by
# /etc/tower/conf.d/*.py.
try:
    include(settings_file, optional(settings_files), scope=locals())

I predict that would yield

INDIRECT_NODE_COUNTING_ENABLED = True
FLAGS = {'FEATURE_INDIRECT_NODE_COUNTING_ENABLED': [{'condition': 'boolean', 'value': False}]}

Now, backing all the way up to what I am interpreting to be the use case you are trying to solve:

  • User sets INDIRECT_NODE_COUNTING_ENABLED = True in /etc/tower/settings.py
  • User restarts services
  • User now has their nodes counted by this feature

Obviously, this would need to be tested. But assuming someone goes and tests it, my expectation is that it would not work, because of the load order mechanics here.

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.

3 participants