fix debian can not use in v4 and version check and clean process#3093
fix debian can not use in v4 and version check and clean process#3093zbb88888 wants to merge 6 commits intokubesphere:mainfrom
Conversation
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
Signed-off-by: zbb88888 <[email protected]>
|
This PR has multiple commits, and the default merge method is: squash. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Welcome @zbb88888! It looks like this is your first PR to kubesphere/kubekey 🎉 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zbb88888 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubesphere-prow
Bot
added
the
size/L
|
zbb88888
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for Debian, implements etcd version alignment and downgrade prevention, and refines the execution of pre/post-installation scripts. It also introduces a development build target in the Makefile and adds cleanup steps for residual processes during uninstallation. Feedback highlights potential issues with process termination due to Linux's 15-character name truncation in ps output and suggests using immediate assignment for Makefile variables to avoid redundant script execution.
| - name: Containerd | Terminate residual runtime shim processes | ||
| ignore_errors: true | ||
| command: | | ||
| ps -eo pid=,comm= | awk '$2 == "containerd-shim-runc-v2" || $2 == "containerd-shim" { print $1 }' | xargs -r kill -9 |
There was a problem hiding this comment.
On Linux, the process name in ps -o comm is truncated to 15 characters. Consequently, containerd-shim-runc-v2 (22 characters) will appear as containerd-shim, and the exact match $2 == "containerd-shim-runc-v2" will fail. This means residual shim processes might not be terminated as intended. Using pgrep -f is a more reliable approach as it matches against the full command line.
pgrep -f "containerd-shim" | xargs -r kill -9| - name: Docker | Terminate residual runtime shim processes | ||
| ignore_errors: true | ||
| command: | | ||
| ps -eo pid=,comm= | awk '$2 == "containerd-shim-runc-v2" || $2 == "containerd-shim" { print $1 }' | xargs -r kill -9 |
There was a problem hiding this comment.
The use of ps -o comm with an exact match for containerd-shim-runc-v2 will fail due to the 15-character truncation limit in the Linux kernel's task name. Residual processes will not be killed. It is better to use pgrep -f to match the process by its full command line.
pgrep -f "containerd-shim" | xargs -r kill -9| - name: DockerCompose | Terminate residual runtime shim processes | ||
| ignore_errors: true | ||
| command: | | ||
| ps -eo pid=,comm= | awk '$2 == "containerd-shim-runc-v2" || $2 == "containerd-shim" { print $1 }' | xargs -r kill -9 |
There was a problem hiding this comment.
The exact match for containerd-shim-runc-v2 in the awk filter will fail because ps -o comm truncates process names to 15 characters. This prevents the termination of residual shim processes. Using pgrep -f is recommended.
pgrep -f "containerd-shim" | xargs -r kill -9| - name: Kubernetes | Terminate residual control plane and kubelet processes | ||
| ignore_errors: true | ||
| command: | | ||
| ps -eo pid=,comm= | awk '$2 == "kube-apiserver" || $2 == "kube-controller-manager" || $2 == "kube-scheduler" || $2 == "kubelet" { print $1 }' | xargs -r kill -9 |
There was a problem hiding this comment.
Process names longer than 15 characters, such as kube-controller-manager, are truncated in the ps -o comm output. The current awk filter uses exact matches that will fail for these processes, leaving them running after uninstallation. Using pgrep -f with a regex pattern is a more robust solution.
pgrep -f "kube-apiserver|kube-controller-manager|kube-scheduler|kubelet" | xargs -r kill -9|
|
||
| # Set build time variables including version details | ||
| LDFLAGS := $(shell hack/version.sh) | ||
| LDFLAGS = $(shell hack/version.sh) |
There was a problem hiding this comment.
Changing LDFLAGS from immediate assignment (:=) to deferred assignment (=) causes the hack/version.sh script to be executed every time the variable is referenced. Since LDFLAGS is used in multiple targets (e.g., kk, docker-push-*), this adds unnecessary overhead to the build process. It is recommended to use := to ensure the version script runs only once.
LDFLAGS := $(shell hack/version.sh)
| CURRENT_BRANCH ?= $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null || echo dev) | ||
| SANITIZED_BRANCH ?= $(shell printf '%s' '$(CURRENT_BRANCH)' | sed -E 's/[^0-9A-Za-z.-]+/-/g; s/^-+//; s/-+$$//') | ||
| DEV_GIT_VERSION ?= v0.0.0-$(if $(SANITIZED_BRANCH),$(SANITIZED_BRANCH),dev) | ||
| DEV_LDFLAGS = $(shell GIT_VERSION=$(DEV_GIT_VERSION) hack/version.sh) |
There was a problem hiding this comment.
Similar to LDFLAGS, DEV_LDFLAGS should use immediate assignment (:=) to avoid redundant executions of the version script during a single make invocation.
DEV_LDFLAGS := $(shell GIT_VERSION=$(DEV_GIT_VERSION) hack/version.sh)
| - name: Defaults | Normalize installed etcd version | ||
| set_fact: | ||
| etcd_installed_version: >- | ||
| {{- $versionOutput := .etcd_install_version.stdout | default "" | toYaml -}} |
There was a problem hiding this comment.
etcd_install_version has been set to the YAML format via register_type on line 59.
| hosts: all | ||
| vars: | ||
| etcd: | ||
| etcd_version: >- |
There was a problem hiding this comment.
It is not recommended to override etcd.etcd_version here.
This field should preferably be defined by the user and passed in via config.yaml.
If the value is set through config.yaml or the --set parameter, it will not be overwritten.
| systemctl daemon-reload | ||
| systemctl reset-failed containerd.service | ||
|
|
||
| - name: Docker | Terminate residual runtime shim processes |
There was a problem hiding this comment.
This is an excellent change. In some scenarios, stopping containerd or docker does not terminate the container processes; it is necessary to stop containerd-shim.
2 participants
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
files in /etc/kubekey/scriptsin not neccessary whenkk create cluster, which will block creation processSpecial notes for reviewers:
Does this PR introduced a user-facing change?
Additional documentation, usage docs, etc.: