Skip to content
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

Merged
merged 5 commits into from
Sep 15, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions apply/driver/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,15 @@ import (
"fmt"
"net"

imagecommon "github.com/sealerio/sealer/pkg/define/options"

"github.com/sealerio/sealer/pkg/auth"
"github.com/sealerio/sealer/pkg/imageengine"

"github.com/sealerio/sealer/apply/processor"
"github.com/sealerio/sealer/common"
"github.com/sealerio/sealer/pkg/auth"
"github.com/sealerio/sealer/pkg/client/k8s"
"github.com/sealerio/sealer/pkg/clusterfile"
imagecommon "github.com/sealerio/sealer/pkg/define/options"
"github.com/sealerio/sealer/pkg/filesystem/clusterimage"
"github.com/sealerio/sealer/pkg/imageengine"
"github.com/sealerio/sealer/pkg/runtime"
"github.com/sealerio/sealer/pkg/runtime/kubernetes"
v2 "github.com/sealerio/sealer/types/api/v2"
"github.com/sealerio/sealer/utils"
osi "github.com/sealerio/sealer/utils/os"
Expand Down Expand Up @@ -237,7 +234,7 @@ func (applier *Applier) Upgrade(upgradeImgName string) error {
}

func (applier *Applier) upgrade() error {
runtimeInterface, err := kubernetes.NewDefaultRuntime(applier.ClusterDesired, applier.ClusterFile.GetKubeadmConfig())
runtimeInterface, err := processor.RuntimeChoose(platform.DefaultMountClusterImageDir(applier.ClusterDesired.Name), applier.ClusterDesired, applier.ClusterFile.GetKubeadmConfig())
if err != nil {
return fmt.Errorf("failed to init runtime: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions apply/processor/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"github.com/sealerio/sealer/pkg/guest"
"github.com/sealerio/sealer/pkg/plugin"
"github.com/sealerio/sealer/pkg/runtime"
"github.com/sealerio/sealer/pkg/runtime/kubernetes"
v2 "github.com/sealerio/sealer/types/api/v2"
"github.com/sealerio/sealer/utils/net"
"github.com/sealerio/sealer/utils/platform"
Expand Down Expand Up @@ -111,7 +110,8 @@ func (c *CreateProcessor) MountImage(cluster *v2.Cluster) error {
if err = c.cloudImageMounter.MountImage(cluster); err != nil {
return err
}
runTime, err := kubernetes.NewDefaultRuntime(cluster, c.ClusterFile.GetKubeadmConfig())
//TODO split kubeadm config from cluster file.
runTime, err := RuntimeChoose(platform.DefaultMountClusterImageDir(cluster.Name), cluster, c.ClusterFile.GetKubeadmConfig())
if err != nil {
return fmt.Errorf("failed to init runtime: %v", err)
}
Expand Down
12 changes: 5 additions & 7 deletions apply/processor/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,18 @@ package processor
import (
"fmt"

common2 "github.com/sealerio/sealer/pkg/define/options"

"github.com/sealerio/sealer/pkg/imageengine"
"github.com/sealerio/sealer/pkg/registry"

"github.com/sealerio/sealer/common"
"github.com/sealerio/sealer/pkg/clusterfile"
common2 "github.com/sealerio/sealer/pkg/define/options"
"github.com/sealerio/sealer/pkg/filesystem"
"github.com/sealerio/sealer/pkg/filesystem/cloudfilesystem"
"github.com/sealerio/sealer/pkg/filesystem/clusterimage"
"github.com/sealerio/sealer/pkg/imageengine"
"github.com/sealerio/sealer/pkg/plugin"
"github.com/sealerio/sealer/pkg/runtime/kubernetes"
"github.com/sealerio/sealer/pkg/registry"
v2 "github.com/sealerio/sealer/types/api/v2"
utilsnet "github.com/sealerio/sealer/utils/net"
"github.com/sealerio/sealer/utils/platform"
)

type DeleteProcessor struct {
Expand All @@ -40,7 +38,7 @@ type DeleteProcessor struct {
}

func (d *DeleteProcessor) Reset(cluster *v2.Cluster) error {
runTime, err := kubernetes.NewDefaultRuntime(cluster, d.ClusterFile.GetKubeadmConfig())
runTime, err := RuntimeChoose(platform.DefaultMountClusterImageDir(cluster.Name), cluster, d.ClusterFile.GetKubeadmConfig())
if err != nil {
return fmt.Errorf("failed to init runtime: %v", err)
}
Expand Down
17 changes: 6 additions & 11 deletions apply/processor/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,17 @@ package processor
import (
"fmt"
"net"

"github.com/sealerio/sealer/pkg/auth"

"github.com/sealerio/sealer/pkg/define/options"

"strconv"

"github.com/sealerio/sealer/pkg/imageengine"

"github.com/sealerio/sealer/pkg/registry"

"github.com/sealerio/sealer/common"
"github.com/sealerio/sealer/pkg/auth"
"github.com/sealerio/sealer/pkg/client/k8s"
"github.com/sealerio/sealer/pkg/clusterfile"
"github.com/sealerio/sealer/pkg/define/options"
"github.com/sealerio/sealer/pkg/filesystem"
"github.com/sealerio/sealer/pkg/filesystem/clusterimage"
"github.com/sealerio/sealer/pkg/imageengine"
"github.com/sealerio/sealer/pkg/registry"
"github.com/sealerio/sealer/pkg/runtime/kubernetes"
v2 "github.com/sealerio/sealer/types/api/v2"
utilsnet "github.com/sealerio/sealer/utils/net"
Expand Down Expand Up @@ -194,7 +189,7 @@ func (g *GenerateProcessor) MountImage(cluster *v2.Cluster) error {
if err = g.ImageMounter.MountImage(cluster); err != nil {
return err
}
runt, err := kubernetes.NewDefaultRuntime(cluster, nil)
runt, err := RuntimeChoose(platform.DefaultMountClusterImageDir(cluster.Name), cluster, nil)
if err != nil {
return err
}
Expand All @@ -207,7 +202,7 @@ func (g *GenerateProcessor) UnmountImage(cluster *v2.Cluster) error {
}

func (g *GenerateProcessor) ApplyRegistry(cluster *v2.Cluster) error {
runt, err := kubernetes.NewDefaultRuntime(cluster, nil)
runt, err := RuntimeChoose(platform.DefaultMountClusterImageDir(cluster.Name), cluster, nil)
if err != nil {
return err
}
Expand Down
9 changes: 3 additions & 6 deletions apply/processor/scale.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,16 @@ import (
"fmt"
"net"

"github.com/sealerio/sealer/utils/platform"

"github.com/sealerio/sealer/pkg/runtime/kubernetes/kubeadm"

"github.com/sealerio/sealer/common"
"github.com/sealerio/sealer/pkg/clusterfile"
"github.com/sealerio/sealer/pkg/config"
"github.com/sealerio/sealer/pkg/filesystem"
"github.com/sealerio/sealer/pkg/filesystem/cloudfilesystem"
"github.com/sealerio/sealer/pkg/plugin"
"github.com/sealerio/sealer/pkg/runtime"
"github.com/sealerio/sealer/pkg/runtime/kubernetes"
"github.com/sealerio/sealer/pkg/runtime/kubernetes/kubeadm"
v2 "github.com/sealerio/sealer/types/api/v2"
platform "github.com/sealerio/sealer/utils/platform"
)

type ScaleProcessor struct {
Expand Down Expand Up @@ -74,7 +71,7 @@ func (s *ScaleProcessor) GetPipeLine() ([]func(cluster *v2.Cluster) error, error
}

func (s *ScaleProcessor) PreProcess(cluster *v2.Cluster) error {
runTime, err := kubernetes.NewDefaultRuntime(cluster, s.KubeadmConfig)
runTime, err := RuntimeChoose(platform.DefaultMountClusterImageDir(cluster.Name), cluster, s.ClusterFile.GetKubeadmConfig())
if err != nil {
return fmt.Errorf("failed to init default runtime: %v", err)
}
Expand Down
39 changes: 39 additions & 0 deletions apply/processor/utils.go
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) {
Copy link
Member

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

type ScaleProcessor struct {
    runtime runtime.Interface
    xxx
}

And I see

func (s *ScaleProcessor) PreProcess(cluster *v2.Cluster) error {
	runTime, err := RuntimeChoose(platform.DefaultMountClusterImageDir(cluster.Name), cluster, s.ClusterFile.GetKubeadmConfig())
	if err != nil {
		return fmt.Errorf("failed to init default runtime: %v", err)
	}
	s.Runtime = runTime

I feel quite strange. Why do you RuntimeChoose in ScaleProcessor and set ScaleProcessor's runtime filed, instead of set runtime type in NewScaleProcessor? Initialization in NewScaleProcessor could make code much more clear and more object-oriented.

WDYT?

Copy link
Member

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 or GetRuntime rather than RuntimeChoose.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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!

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish to see more thinking details about the default case. From my point of view, if there is no metadata.ClusterRuntime in image rootfs details, we take it as k8s by default. Because in the original implementation of ClusterImage, there is no metadata.ClusterRuntime, and we have to make the new version of sealer compatible with the legacy ClusterImage format. For what is worth, I suggest to add a log to tell end-user that he is using a legacy version, and using new version of sealer and ClusterImage is encouraged.

Otherwise, I think you should add a line of explicit log at Line31 to show the ClusterRuntime info.

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

}
}
5 changes: 5 additions & 0 deletions pkg/runtime/k0s/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,16 @@
package k0s

const (
RuntimeFlag = "k0s"
VersionCmd = "k0s version"

DefaultAdminConf = "/var/lib/k0s/pki/admin.conf"

DefaultK0sConfigPath = "/etc/k0s/k0s.yaml"
DefaultK0sWorkerJoin = "/etc/k0s/worker"
DefaultK0sControllerJoin = "/etc/k0s/controller"
WorkerRole = "worker"
ControllerRole = "controller"

ExternalCRI = "/run/containerd/containerd.sock"
)
129 changes: 129 additions & 0 deletions pkg/runtime/k0s/delete_masters.go
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 {
Copy link
Member

Choose a reason for hiding this comment

The 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 ?

Copy link
Member Author

Choose a reason for hiding this comment

The 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)
Copy link
Member

Choose a reason for hiding this comment

The 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 failed to delete master:. And in Line 49, you have code logrus.Errorf("failed to delete master %s: %v", master, err). Then if an error is returned back, then you error message would be failed to delete master 1.1.1.1: failed to delete master: xxxxxx. It is duplicated and definitely inproper.

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
*/
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The 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) == "" {
Copy link
Member

Choose a reason for hiding this comment

The 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))

Copy link
Member Author

Choose a reason for hiding this comment

The 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
}
Loading