node-servant: add preflight-convert command #686
Conversation
|
@Peeknut: GitHub didn't allow me to assign the following users: your_reviewer. Note that only openyurtio members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
Instructions 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/test-infra repository. |
|
Test cases: Success: Failure: run in OpenYurt Node |
Peeknut
force-pushed
the
nodeservant-precheck
branch
2 times, most recently
from
1324b0c to
ecdf6dc
Compare
|
@adamzhoul Do you have any comments about new command: |
Peeknut
force-pushed
the
nodeservant-precheck
branch
from
ecdf6dc to
d329680
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Peeknut, rambohe-ch The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -64,15 +60,6 @@ func (n *nodeConverter) validateOptions() error { | |||
| return nil | |||
| } | |||
|
|
|||
| func (n *nodeConverter) preflightCheck() error { | |||
There was a problem hiding this comment.
why we delete those?
and how can we make sure users do preflight before do convert?
or, what if we put core preflight capability in convert?
There was a problem hiding this comment.
There are two reasons for deletion:
(1) The node servant mainly serves for yurtctl convert, it is rare for users to use node-servant to convert nodes individually.
(2) In order to increase the conversion success rate of yrutcl convert, yurtctl convert will perform preflight check and issue node servant preflight check job. If the check is successful, the cloud will perform the conversion and issue node servant convert job. If add a check part to the node servant, the check part will be executed twice. It seems a bit cumbersome.
So I remove the check part in node-servant convert.
How about marking it in the usage document: Perform node-servant preflight check before executing node-servant convert?
There was a problem hiding this comment.
(1) The node servant mainly serves for yurtctl convert, it is rare for users to use node-servant to convert nodes individually.
I think it's not about we use it individually or not.
I only think node-servant should include preflight check capability,
and export its capability to the upper ( yurtctl convert level )
(2) In order to increase the conversion success rate of yrutcl convert,
is this be noticed by users?
I think end-user should not notice we have a preflight command if everything is right.
The end-user doesn't need to execute this manually I think.
what do you say?
There was a problem hiding this comment.
I think it's not about we use it individually or not...
Yes, considering the usability of users, it is better that convert include preflight check .So yurtctl convert does just that.
But in node-servant, we split it into two parts, one for preflight check and one for conversion to cooperate with the use of yurtctl convert.
If the node-servant convert contains check and also exposes the check capability, then yurtctl convert will call twice repeated checks (as mentioned above) during conversion.
is this be noticed by users?
If the user uses yurtctl convert, the preflight check content is included in the convert, so the user uses the same way as before.
If the user uses node-servant convert, preflight check and convert are two commands, so the user needs to execute preflight check first, and then execute convert, which needs to be stated in the document.
| @@ -17,7 +17,7 @@ limitations under the License. | |||
| package edgenode | |||
|
|
|||
| const ( | |||
| KubeletSvcPath = "/etc/systemd/system/kubelet.service.d/10-kubeadm.conf" | |||
| KubeletSvcPath = "/usr/lib/systemd/system/kubelet.service.d/10-kubeadm.conf" | |||
There was a problem hiding this comment.
According to the issues mentioned by the community, KubeletSvcPath is generally "/usr/lib/systemd/system/kubelet.service.d/10-kubeadm.conf", so it was changed to the default value.
In addition, If the default value is "/etc/systemd/system/kubelet.service.d/10-kubeadm.conf", the first time the user uses it, the conversion will often fail, and the parameter --kubeadm-conf-path must be passed in when converting. It is troublesome to use.
There was a problem hiding this comment.
can you share the issue address?
want to know some background in detail
Peeknut
force-pushed
the
nodeservant-precheck
branch
from
d329680 to
40fc6e4
Compare
|
@adamzhoul @rambohe-ch |
|
kubeadm-conf-path Test： convert: preflight check: |
|
/lgtm |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Split preflight check and convert. Preflight check is called by
yurtctl convertto perform pre-check to reduce the conversion failures.Which issue(s) this PR fixes:
Fixes #619
Special notes for your reviewer:
@DrmagicE @adamzhoul @rambohe-ch
Does this PR introduce a user-facing change?
other Note