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

job: Validate job.OneShot name #10

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

ovidiutirla
Copy link
Contributor

Adds validation for job.OneShot to match regex ^[a-z][a-z0-9_\-]{0,30}$. It avoids construction of jobs that are not usable for metrics.

@ovidiutirla ovidiutirla requested a review from a team as a code owner September 6, 2024 09:43
@ovidiutirla ovidiutirla requested review from pippolo84 and removed request for a team September 6, 2024 09:43
@ovidiutirla
Copy link
Contributor Author

/cc @joamaki

@ovidiutirla ovidiutirla force-pushed the feature/validate-job-oneshot-name branch from 15f74e6 to 10486e3 Compare September 6, 2024 09:57
Copy link
Contributor

@joamaki joamaki left a comment

Choose a reason for hiding this comment

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

We should do the same for all job types (Timer, Observer).

@ovidiutirla ovidiutirla force-pushed the feature/validate-job-oneshot-name branch from 10486e3 to f291110 Compare September 9, 2024 08:34
@ovidiutirla
Copy link
Contributor Author

We should do the same for all job types (Timer, Observer).

Would you prefer to extract the fn as an common interface to avoid the fn==nil as argument?

@joamaki
Copy link
Contributor

joamaki commented Sep 9, 2024

We should do the same for all job types (Timer, Observer).

Would you prefer to extract the fn as an common interface to avoid the fn==nil as argument?

”fn” is very specific to the job type and I can imagine we’d at some point have jobs that don’t take an ”fn”. I’d keep the ”fn” validation in the job constructor itself.

Adds validation for job.OneShot, job.Timer, job.Observer to match regex `^[a-z][a-z0-9_\-]{0,30}$`. It avoids construction of jobs that are not usable for metrics.

Signed-off-by: Ovidiu Tirla <[email protected]>
@ovidiutirla ovidiutirla force-pushed the feature/validate-job-oneshot-name branch from f291110 to d22a3f8 Compare September 9, 2024 08:59
@ovidiutirla ovidiutirla requested a review from joamaki September 9, 2024 16:10
Copy link
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

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

Thanks! 💯

@joamaki joamaki merged commit c6e014a into cilium:main Sep 10, 2024
1 check passed
ovidiutirla added a commit to ovidiutirla/cilium that referenced this pull request Sep 11, 2024
The new version of hive have a new format for the job names to ensure the names are compliant with the prometheus metrics. This commit ensures that StateDB Reflector Job names are compliant with the new naming convention. See cilium/hive#10.

Signed-off-by: Ovidiu Tirla <[email protected]>
ovidiutirla added a commit to ovidiutirla/cilium that referenced this pull request Sep 11, 2024
The new version of hive have a new format for the job names to ensure the names are compliant with the prometheus metrics. This commit ensures that all jobs are compliant with the new naming convention. See cilium/hive#10.

Signed-off-by: Ovidiu Tirla <[email protected]>
@ovidiutirla
Copy link
Contributor Author

The max job name was bumped to 100. #14

ovidiutirla added a commit to ovidiutirla/cilium that referenced this pull request Sep 11, 2024
The new version of hive have a new format for the job names to ensure the names are compliant with the prometheus metrics. This commit ensures that all jobs are compliant with the new naming convention. See cilium/hive#10.

Signed-off-by: Ovidiu Tirla <[email protected]>
ovidiutirla added a commit to ovidiutirla/cilium that referenced this pull request Sep 12, 2024
The new version of hive have a new format for the job names to ensure the names are compliant with the prometheus metrics. This commit ensures that StateDB Reflector Job names are compliant with the new naming convention. See cilium/hive#10.

Signed-off-by: Ovidiu Tirla <[email protected]>
ovidiutirla added a commit to ovidiutirla/cilium that referenced this pull request Sep 12, 2024
The new version of hive have a new format for the job names to ensure the names are compliant with the prometheus metrics. This commit ensures that all jobs are compliant with the new naming convention. See cilium/hive#10.

Signed-off-by: Ovidiu Tirla <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Sep 13, 2024
The new version of hive have a new format for the job names to ensure the names are compliant with the prometheus metrics. This commit ensures that StateDB Reflector Job names are compliant with the new naming convention. See cilium/hive#10.

Signed-off-by: Ovidiu Tirla <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Sep 13, 2024
The new version of hive have a new format for the job names to ensure the names are compliant with the prometheus metrics. This commit ensures that all jobs are compliant with the new naming convention. See cilium/hive#10.

Signed-off-by: Ovidiu Tirla <[email protected]>
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