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 Kubernetes collector #836

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

rkleinem
Copy link
Collaborator

@rkleinem rkleinem commented Jun 3, 2024

A first version of the Kubernetes collector.

Still to do:

  • Helm charts
  • Document charts
  • Dockerfile
  • Check whether an integration test is feasible

I found that it doesn't make much sense to use the queued client for this because we need a separate persistent storage anyway. The client would just be a second one.

Note:
I made a little change to AUDITOR: use the Display formatter for the Display implementation of ValidName. Is this ok? If not I can revert this and apply a workaround in the collector.

@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 284 lines in your changes missing coverage. Please review.

Project coverage is 63.75%. Comparing base (c4c408d) to head (2f086cd).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #836      +/-   ##
==========================================
+ Coverage   63.02%   63.75%   +0.72%     
==========================================
  Files          49       56       +7     
  Lines        5983     6834     +851     
  Branches       62       62              
==========================================
+ Hits         3771     4357     +586     
- Misses       2206     2471     +265     
  Partials        6        6              
Components Coverage Δ
apel_plugin 71.09% <ø> (ø)
auditor 70.36% <100.00%> (+1.51%) ⬆️
auditor_client 94.38% <ø> (ø)
slurm_collector 67.67% <ø> (ø)
slurm_epilog_collector 0.00% <ø> (ø)
htcondor_collector ∅ <ø> (∅)
priority_plugin 37.82% <ø> (ø)

Copy link
Collaborator

@raghuvar-vijay raghuvar-vijay left a comment

Choose a reason for hiding this comment

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

Thanks a lot:-) I've added a few minor comments.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Local dependencies can be transitioned to workspace dependencies

if ! [ -x "$(command -v sqlx)" ]; then
echo >&2 "Error: sqlx is not installed."
echo >&2 "Use:"
echo >&2 " cargo install --version=0.7.2 sqlx-cli --no-default-features --features postgres,rustls,sqlite"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This version number is different from the Cargo.toml sqlx dependency version number (v0.7.4)

use serde::Deserialize;
use tracing_subscriber::filter::LevelFilter;

pub fn load_configuration(p: impl AsRef<Path>) -> Config {
Copy link
Collaborator

Choose a reason for hiding this comment

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

load_configuration can be modified to return Result<Config, config::ConfigError>

@rkleinem
Copy link
Collaborator Author

Will do another commit in an hour or so

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

Successfully merging this pull request may close these issues.

3 participants