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

GPU nfdhook label split fix #1285

Merged
merged 2 commits into from
Dec 31, 2022
Merged

GPU nfdhook label split fix #1285

merged 2 commits into from
Dec 31, 2022

Conversation

tkatila
Copy link
Contributor

@tkatila tkatila commented Dec 28, 2022

k8s labels cannot start or end with a non alphanumeric character. This new split method splits the label with the last alphanumeric character (under 63 characters) and starts the next label with a control character (Z).

uniemimu
uniemimu previously approved these changes Dec 28, 2022
@codecov-commenter
Copy link

codecov-commenter commented Dec 28, 2022

Codecov Report

Merging #1285 (d90e35a) into main (b534c16) will increase coverage by 0.05%.
The diff coverage is 74.07%.

@@            Coverage Diff             @@
##             main    #1285      +/-   ##
==========================================
+ Coverage   50.56%   50.61%   +0.05%     
==========================================
  Files          41       43       +2     
  Lines        4634     4682      +48     
==========================================
+ Hits         2343     2370      +27     
- Misses       2157     2176      +19     
- Partials      134      136       +2     
Impacted Files Coverage Δ
cmd/internal/pluginutils/labels.go 71.42% <71.42%> (ø)
cmd/gpu_nfdhook/labeler.go 85.52% <100.00%> (-0.13%) ⬇️
cmd/internal/pluginutils/sriov.go 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

If k8s label starts or ends with a non alphanumeric char, it is
ignored. This new split method cuts labels from the last alphanum
characters and adds a control character to the beginning of the next
chunk. The entity that uses these labels needs to then remove the
control character before concatenating the chunks.

Co-authored-by: Ukri Niemimuukko <[email protected]>
Signed-off-by: Tuomas Katila <[email protected]>
@tkatila
Copy link
Contributor Author

tkatila commented Dec 29, 2022

Renamed forbiddenChar as controlChar, no other changes vs. prev state.

Copy link
Member

@bart0sh bart0sh left a comment

Choose a reason for hiding this comment

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

/lgtm

@bart0sh bart0sh merged commit b54ba7a into intel:main Dec 31, 2022
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.

4 participants