fix: install local iso failed#3106
Conversation
Signed-off-by: redscholar <blacktiledhouse@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: redscholar The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
There was a problem hiding this comment.
Code Review
This pull request updates the backup file naming convention for repository files, renames the ISO file check to repository.iso, and refines the processing of OS version IDs in templates. Feedback was provided regarding the use of unquoted variables containing spaces in shell commands, which could lead to execution failures. Additionally, it was suggested to convert version IDs to lowercase for better consistency and compatibility with file naming conventions.
| kylin-{{ .os.release.VERSION_ID | replace "\"" "" | unquote | trim | lower }}{{ .sp_version | trim }} | ||
| {{- else if .os.release.ID_LIKE | unquote | eq "rhel fedora" }} | ||
| {{ .os.release.ID | replace "\"" "" | unquote | trim | lower }}{{ .os.release.VERSION_ID | trim }} | ||
| {{ .os.release.ID | replace "\"" "" | unquote | trim | lower }}{{ .os.release.VERSION_ID | replace "\"" "" | unquote | trim }} |
There was a problem hiding this comment.
For consistency with the Kylin implementation (line 26) and to ensure compatibility with lowercase filenames commonly used for ISO images, the VERSION_ID should be converted to lowercase. This also removes an unnecessary double space after the lower filter.
{{ .os.release.ID | replace "\"" "" | unquote | trim | lower }}{{ .os.release.VERSION_ID | replace "\"" "" | unquote | trim | lower }}| {{ .os.release.ID | replace "\"" "" | unquote | trim | lower }}{{ .os.release.VERSION_ID | replace "\"" "" | unquote | trim }} | ||
| {{- else }} | ||
| {{ .os.release.ID | replace "\"" "" | unquote | trim | lower }}-{{ .os.release.VERSION_ID | trim }} | ||
| {{ .os.release.ID | replace "\"" "" | unquote | trim | lower }}-{{ .os.release.VERSION_ID | replace "\"" "" | unquote | trim }} |
There was a problem hiding this comment.



What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR fixes two bugs in the offline repository installation logic that prevent KubeKey from using local ISO repositories on Debian/Ubuntu nodes.
Fixes incorrect local ISO path check:
In
install_package.yaml, the condition[ -f "{{ .tmp_dir }}/iso" ]checks a mount directory using the-f(file) test, which always returnsfalse. This causes the script to skip the local repository branch and fall back to public APT mirrors (archive.ubuntu.com), which are unreachable in offline/air-gapped environments. The condition is changed to[ -f "{{ .tmp_dir }}/repository.iso" ]to correctly detect the ISO file.Fixes mismatched backup/restore filenames:
The backup step uses the suffix
kubekey-$now.bak(timestamp before.bak), but the restore step incorrectly useskubekey.bak-$now(timestamp after.bak). Because the filenames do not match, the original APT/YUM source configurations fail to restore after package installation.Which issue(s) this PR fixes:
Fixes #
Special notes for reviewers:
Does this PR introduced a user-facing change?
Additional documentation, usage docs, etc.: