-
Notifications
You must be signed in to change notification settings - Fork 185
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
Uyuni Health Check Tool Disconnected Solution #9322
base: master
Are you sure you want to change the base?
Conversation
At this point the completion-checker script just processes .log files and not compressed files as it seems that the information in the positions.yaml file is not the number of bytes for the .tar.gz file but instead, the number of lines in the logfile inside the .tar.gz. This breaks the logic of the completion-checker so this is a partial solution at this moment.
health-check/README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is outside of python/
, we need to adapt the Python checkstyle workflow yaml to cover it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few questions inline. I think we should enable the linter as soon as we can, it will be a bit of work to bring the code up to our standards (as expected for PoC code)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Afaik, pip3.11 install
does not work in OBS.
@@ -0,0 +1,9 @@ | |||
FROM docker.io/grafana/grafana:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use this base image? Shouldn't we base this on a BCI image?
FROM opensuse/leap:latest | ||
|
||
|
||
COPY logcli-linux-amd64 /usr/bin/logcli |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to package logcli
, if it isn't packaged yet.
@@ -0,0 +1 @@ | |||
FROM docker.io/grafana/loki |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question here, can we use an image from docker.io?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small question, can we use TOML instead? It's similar, but we've started using it more in other projects and it's widely used in the Python world these days.
import json | ||
from jinja2 import Environment, FileSystemLoader | ||
|
||
class ConfigLoader: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO there is no need for this class, the things it does are generally unrelated.
|
||
class ConfigLoader: | ||
def __init__(self): | ||
self.base_dir = os.path.dirname(os.path.abspath(__file__)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to have the config outside of the Python package in the future.
DRAFT
What does this PR change?
Initial implementation of the Disconnected solution. No access to an Uyuni server or existing monitoring stack, only access to a supportconfig.
See: https://github.com/uyuni-project/uyuni-rfc/blob/master/accepted/00101-health-check-tool.md
GUI diff
No difference.
Before:
After:
Documentation
No documentation needed: add explanation. This can't be used if there is a GUI diff
No documentation needed: only internal and user invisible changes
Documentation issue was created: Link for SUSE Manager contributors, Link for community contributors.
API documentation added: please review the Wiki page Writing Documentation for the API if you have any changes to API documentation.
(OPTIONAL) Documentation PR
DONE
Test coverage
ℹ️ If a major new functionality is added, it is strongly recommended that tests for the new functionality are added to the Cucumber test suite
No tests: add explanation
No tests: already covered
Unit tests were added
Cucumber tests were added
DONE
Links
Issue(s): #
Port(s): # add downstream PR(s), if any
Changelogs
Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository
If you don't need a changelog check, please mark this checkbox:
If you uncheck the checkbox after the PR is created, you will need to re-run
changelog_test
(see below)Re-run a test
If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:
Before you merge
Check How to branch and merge properly!