-
Notifications
You must be signed in to change notification settings - Fork 362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: k0s runtime remain work #1686
Changes from 4 commits
053e716
0691113
65a56fc
a487276
28bd67e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// Copyright © 2022 Alibaba Group Holding Ltd. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package processor | ||
|
||
import ( | ||
"github.com/sealerio/sealer/pkg/runtime" | ||
"github.com/sealerio/sealer/pkg/runtime/k0s" | ||
"github.com/sealerio/sealer/pkg/runtime/kubernetes" | ||
"github.com/sealerio/sealer/pkg/runtime/kubernetes/kubeadm" | ||
v2 "github.com/sealerio/sealer/types/api/v2" | ||
) | ||
|
||
func RuntimeChoose(rootfs string, cluster *v2.Cluster, config *kubeadm.KubeadmConfig) (runtime.Interface, error) { | ||
metadata, err := runtime.LoadMetadata(rootfs) | ||
if err != nil { | ||
return nil, err | ||
} | ||
switch metadata.ClusterRuntime { | ||
case runtime.K8s: | ||
return kubernetes.NewDefaultRuntime(cluster, config) | ||
case runtime.K0s: | ||
return k0s.NewK0sRuntime(cluster) | ||
// Todo case runtime.K3s: | ||
default: | ||
return kubernetes.NewDefaultRuntime(cluster, config) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wish to see more thinking details about the Otherwise, I think you should add a line of explicit log at Line31 to show the ClusterRuntime info. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with add a cluster runtime info and hint end-user using new version ClusterImage. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
// Copyright © 2022 Alibaba Group Holding Ltd. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package k0s | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"net" | ||
"strings" | ||
|
||
"github.com/sealerio/sealer/pkg/client/k8s" | ||
|
||
"github.com/pkg/errors" | ||
"github.com/sirupsen/logrus" | ||
"golang.org/x/sync/errgroup" | ||
) | ||
|
||
func (k *Runtime) deleteMasters(masters []net.IP) error { | ||
if len(masters) == 0 { | ||
return nil | ||
} | ||
eg, _ := errgroup.WithContext(context.Background()) | ||
for _, master := range masters { | ||
master := master | ||
eg.Go(func() error { | ||
logrus.Infof("Start to delete master %s", master) | ||
if err := k.deleteMaster(master); err != nil { | ||
return fmt.Errorf("failed to delete master %s: %v", master, err) | ||
} | ||
logrus.Infof("Succeeded in deleting master %s", master) | ||
return nil | ||
}) | ||
} | ||
return eg.Wait() | ||
} | ||
|
||
func (k *Runtime) deleteMaster(master net.IP) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a detailed deep implementation of delete a master node. I think we should explain clearly what is the STEPS to delete a master by comments. If we understand the steps and then the code review could be quite easy. Actually we should review the design firstly, and code secondly. No design there, and we can hardly do the code review. Do you think so ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep! when I read kubernetes runtime module, I have the same feel. which files to delete? and what logic to follow. I so sorry to made this, fix this ASAP. |
||
ssh, err := k.getHostSSHClient(master) | ||
if err != nil { | ||
return fmt.Errorf("failed to delete master: %v", err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think your error log message should be optimized. When logging error message, you should set up a rule for yourself, where to locate the wrap message. In this case, the wrap message is I wish that you could pay more attention on every part of the code. Only by patience and carefulness, could quality of software be achieved. |
||
} | ||
/** To delete a node from k0s cluster, following these steps. | ||
STEP1: stop k0s service | ||
STEP2: reset the node with install configuration | ||
STEP3: remove k0s cluster config generate by k0s under /etc/k0s | ||
STEP4: remove private registry config in /etc/host | ||
STEP5: remove bin file such as: kubectl, and remove .kube directory | ||
STEP6: remove k0s bin file | ||
STEP7: delete node though k8s client | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if one of the cmds failed, will sealer throw the error for that failed cmd? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the error will be thrown by ssh.Async and captured by the caller. |
||
remoteCleanCmd := []string{"k0s stop", | ||
fmt.Sprintf("k0s reset --config %s --cri-socket %s", DefaultK0sConfigPath, ExternalCRI), | ||
"rm -rf /etc/k0s/", | ||
fmt.Sprintf("sed -i \"/%s/d\" /etc/hosts", SeaHub), | ||
fmt.Sprintf("sed -i \"/%s/d\" /etc/hosts", k.RegConfig.Domain), | ||
fmt.Sprintf("rm -rf %s /%s*", DockerCertDir, k.RegConfig.Domain), | ||
fmt.Sprintf("rm -rf %s /%s*", DockerCertDir, SeaHub), | ||
"rm -rf /usr/bin/kube* && rm -rf ~/.kube/", | ||
"rm -rf /usr/bin/k0s"} | ||
|
||
if err := ssh.CmdAsync(master, remoteCleanCmd...); err != nil { | ||
return err | ||
} | ||
|
||
// remove master | ||
masterExist := len(k.cluster.GetMasterIPList()) > 1 | ||
if !masterExist { | ||
return errors.New("can not delete the last master") | ||
} | ||
|
||
hostname, err := k.getHostName(master) | ||
if err != nil { | ||
return err | ||
} | ||
client, err := k8s.Newk8sClient() | ||
if err != nil { | ||
return err | ||
} | ||
if err := client.DeleteNode(hostname); err != nil { | ||
return err | ||
} | ||
return nil | ||
} | ||
|
||
func (k *Runtime) getHostName(host net.IP) (string, error) { | ||
client, err := k8s.Newk8sClient() | ||
if err != nil { | ||
return "", err | ||
} | ||
nodeList, err := client.ListNodes() | ||
if err != nil { | ||
return "", err | ||
} | ||
var hosts []string | ||
for _, node := range nodeList.Items { | ||
hosts = append(hosts, node.GetName()) | ||
} | ||
|
||
hostName, err := k.CmdToString(host, "hostname", "") | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
var name string | ||
for _, h := range hosts { | ||
if strings.TrimSpace(h) == "" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You do TrimSpace(h) here. I guess the result from TrimSpace(h) should pass down? hh := strings.ToLower(strings.TrimSpace(h)) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I think we can't pass down value after processed. I don't know how hostname special, but I know field got by k8s client would be it input. |
||
continue | ||
} | ||
hh := strings.ToLower(h) | ||
fromH := strings.ToLower(hostName) | ||
if hh == fromH { | ||
name = h | ||
break | ||
} | ||
} | ||
return name, 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.
@starComingup Here I have a point to discuss with you.
If runtime is a field of details processor instance, or belongs to a processor's function?
If runtime is a field of processor, then RuntimeChoose should be used in a processor's initialization, rather than set runtime in processor's function.
For example, I see
And I see
I feel quite strange. Why do you
RuntimeChoose
in ScaleProcessor and set ScaleProcessor's runtime filed, instead of set runtime type inNewScaleProcessor
? Initialization in NewScaleProcessor could make code much more clear and more object-oriented.WDYT?
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.
In addition, please change the function name into
ChooseRuntime
orGetRuntime
rather thanRuntimeChoose
.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.
The old code set kubernetes.runtime here, so I just make this. But when I coded this, I also felt confused why origin logic set runtime in sub function. I will try to modify it!