-
Notifications
You must be signed in to change notification settings - Fork 408
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix bugs of yurctl #317
fix bugs of yurctl #317
Conversation
246662c
to
4d7846b
Compare
Do the following test1.Compile the image locally on node:
|
e50871c
to
1aaf167
Compare
pkg/yurtctl/cmd/convert/edgenode.go
Outdated
@@ -242,7 +240,8 @@ func (c *ConvertEdgeNodeOptions) RunConvertEdgeNode() (err error) { | |||
klog.Errorf("fail to run ServantJobs: %s", err) | |||
return err | |||
} | |||
} else { | |||
} else if len(c.EdgeNodes) == 0 || (len(c.EdgeNodes) == 1 && c.EdgeNodes[0] == nodeName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about add nodeName != ""
for len(c.EdgeNodes) == 0
condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
pkg/yurtctl/cmd/convert/edgenode.go
Outdated
@@ -269,22 +266,44 @@ func (c *ConvertEdgeNodeOptions) RunConvertEdgeNode() (err error) { | |||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about move c.KubeadmConfPath
and c.PodMainfestPath
parameter check and joinToken
creation before LabelNode
? so convert can fail fast if any error happened
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good idea.
pkg/yurtctl/cmd/revert/edgenode.go
Outdated
@@ -193,7 +193,8 @@ func (r *RevertEdgeNodeOptions) RunRevertEdgeNode() (err error) { | |||
klog.Errorf("fail to revert edge node: %s", err) | |||
return err | |||
} | |||
} else { | |||
} else if len(r.EdgeNodes) == 0 || (len(r.EdgeNodes) == 1 && r.EdgeNodes[0] == nodeName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about add nodeName != "" for len(c.EdgeNodes) == 0 condition?
pkg/yurtctl/cmd/revert/edgenode.go
Outdated
} | ||
klog.Info("label openyurt.io/is-edge-worker is removed") | ||
defer func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about move remove edge label
routine to the end of RunRevertEdgeNode
func and after RevertKubelet
and RemoveYurthub
. because we should try to keep the reverse order of RunConvertEdgeNode
. at the same time, rollback routine will not be needed.
/kind bug |
/lgtm |
/approve |
[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 |
Ⅰ. Describe what this PR does
fix bugs of yurtctl:
--kubeadm-conf-path
Ⅱ. Does this pull request fix one issue?
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Ⅳ. Describe how to verify it
NOTE: After yurtctl is modified, you need to recompile the yurtctl servant image and use it yourself, because the default image of servant-convert/revert-job is out of date after modify it.
Ⅴ. Special notes for reviews