WIP: rework accounting scripts - #288
Conversation
gmatthews20
left a comment
There was a problem hiding this comment.
I haven't yet reviewed any of the tests. Will take a look once we're happy with the functionality
| # Get log level from environment, defaulting to DEBUG | ||
| log_level_name = os.getenv("THECOUNT_LOG_LEVEL", "INFO").upper() | ||
| log_level = getattr(logging, log_level_name, logging.INFO) |
There was a problem hiding this comment.
The default level is INFO no?
| "--start-time", metavar="TIME", default=None, | ||
| help="start of the range (default: current time) ISO 8601 format( yyyy-mm-ddTHH:MM:SS )", |
There was a problem hiding this comment.
help statement says default is current time but default is set to None?
| interval_regex = re.compile( | ||
| r'((?P<days>\d+?)d)?((?P<hours>\d+?)h)?((?P<minutes>\d+?)m)?((?P<seconds>\d+?)s)?' | ||
| ) |
There was a problem hiding this comment.
Instead of trying to parse different formats as regex why not just force the user to provide the interval in minutes?
| source: Source object - used to fetch data | ||
| start_time: python datetime - the start time of the job | ||
| end_time: python datetime - the end time of the job | ||
| dry_run: boolean - whether to run the job in dry run mode - i.e. print to console instead of sending | ||
| it to source (useful for debugging) |
There was a problem hiding this comment.
You're missing sink from the docstring. Also I think a better description of what source and sink are is needed
| @staticmethod | ||
| def _transform(rows: List[Dict[str, Any]], end_time: datetime, instance) -> str: | ||
| """ | ||
| transform cinder data from source to get total amount of GB per second usage for block-storage per project |
There was a problem hiding this comment.
GB per second sounds like throughput, I think it should be GB seconds
|
|
||
| def _load_config(self, config_fp: str) -> None: | ||
| """ | ||
| Helper function to read theobjectcount config file and extrac source-related config |
There was a problem hiding this comment.
Is theobjectcount correct?
| Helper function to read theobjectcount config file and extrac source-related config | |
| Helper function to read thecount config file and extract source-related config |
| # Use Python 3.12 slim image as base | ||
| FROM python:3.12-slim |
There was a problem hiding this comment.
Why was version 3.12 chosen, any reason we couldn't use something newer like 3.14?
| RUN pip install --no-cache-dir . | ||
|
|
||
| # Create a non-root user | ||
| RUN useradd --create-home --shell /bin/bash monitoring |
There was a problem hiding this comment.
Don't think we need to create a home dir for the user?
| RUN useradd --create-home --shell /bin/bash monitoring | |
| RUN useradd --shell /bin/bash monitoring |
| requires-python = ">=3.10" | ||
| keywords = ["python", "openstack"] | ||
| classifiers = [ | ||
| "Development Status :: 4 - Beta", |
There was a problem hiding this comment.
Version 1.0.0 is a beta version?
| User with read access to the relevant databases for each OpenStack component you are accounting for - right now: | ||
| - cinder | ||
| - manila | ||
| - nova | ||
| - glance |
There was a problem hiding this comment.
Do the correct SQL procedures need to exist as well? How are these set up/kept?
make accounting scripts run as a docker container
requirements:
the docker container must run continuously - to get accounting data every interval
also, the container can be used to collect past accounting data by providing timestamps from the past.
there should be good logs so it can run on kayobe and be debugged easily
there should be a dry-run feature - so we can test improvements in the future without sending data anywhere
there should be good testing and linting to keep up with our coding standards
it should work on a supported python version and the dependencies should be up-to-date
Still TODO:
so that we can auto-build and version the docker container