Project-HAMi / HAMi

Heterogeneous AI Computing Virtualization Middleware
http://project-hami.io/
Apache License 2.0
956 stars 197 forks source link

Fix Kubernetes version string handling by stripping metadata #623

Closed Nimbus318 closed 3 days ago

Nimbus318 commented 5 days ago

What type of PR is this? /kind bug

What this PR does / why we need it: This PR fixes an issue where Kubernetes version strings with additional metadata (e.g., +k3s1) caused problems in dynamically generated image tags. The new strippedKubeVersion function removes the part after the + in the Kubernetes version string, ensuring consistent and predictable image tagging.

Which issue(s) this PR fixes: Fixes #621

Special notes for your reviewer: @archlitchi @wawa0210

Does this PR introduce a user-facing change?: Yes, this PR restores compatibility for specifying certain parameters during helm install/upgrade.

After version 2.4.1, users were unable to specify parameters like scheduler.kubeScheduler.tag (scheduler image version), devicePlugin.deviceSplitCount, devicePlugin.deviceMemoryScaling, and devicePlugin.deviceCoreScaling directly during Helm operations.

With this PR merged:

codecov[bot] commented 5 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Flag Coverage Δ
unittests 26.80% <ø> (-0.07%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

wawa0210 commented 3 days ago

Very good bug fix, have we done some testing? For example, can it work properly in the K3s environment?

also,you should rebase your commits into one commit

Nimbus318 commented 3 days ago

I have rebased the commits into one as requested.

Regarding testing, I have not tested this in a real K3s environment. However, I conducted mock testing for this function locally by injecting a mock Kubernetes version value in _helpers.tpl for validation. Below are the steps I followed:

Mock the Kubernetes version by temporarily injecting a value for testing:

{{/*
    Mock Kubernetes version for testing
*/}}
{{- define "mockedCapabilitiesKubeVersion" -}}
v1.31.1+k3s1
{{- end }}

Modify the existing logic to replace Capabilities.KubeVersion.Version with the mock value:

{{- define "strippedKubeVersion" -}}
{{- $parts := split "+" (include "mockedCapabilitiesKubeVersion" .) -}}
{{- print $parts._0 -}}
{{- end }}

Validated that the output correctly strips the metadata after +.