feature: k0s init, import some dependencies form k0sproject. #1619
Conversation
|
I use |
starComingup
force-pushed
the
feat/k0s-init
branch
from
1f04509 to
91911e3
Compare
starComingup
force-pushed
the
feat/k0s-init
branch
from
91911e3 to
20d38d5
Compare
starComingup
changed the title
starComingup
force-pushed
the
feat/k0s-init
branch
from
20d38d5 to
53e1ed3
Compare
| func (k *Runtime) init() error { | ||
| pipeline := []func() error{ | ||
| k.MergeConfigOnMaster0, | ||
| k.SSHKeyGenAndCopyIDToHosts, |
There was a problem hiding this comment.
I'm not very familiar with k0s, I have a question, is this necessary to configure ssh key login？
There was a problem hiding this comment.
if we don't ssh-keygen id_rsa config: when k0sctl lead to install cluster, it will report:ERRO [ssh] 172.16.161.63:22: attempt 1 of 60.. failed to connect: stat /root/.ssh/id_rsa: no such file or directory
if we don't ssh-copy-id: k0sctl will report :failed to connect: ssh: handshake failed: ssh: unable to authenticate, attempted methods [none publickey], no supported methods remain
| pipeline := []func() error{ | ||
| k.MergeConfigOnMaster0, | ||
| k.SSHKeyGenAndCopyIDToHosts, | ||
| k.GenerateCert, |
There was a problem hiding this comment.
This part of configuring private image repositories will be removed from the runtime pkg after we refactor run process, and i think k8s runtime also need those logic. see PR #1621
There was a problem hiding this comment.
Got it! will adapt it when new code merged.
There was a problem hiding this comment.
I suggest that we do this before the code merging. Or at least you add a TODO comment
// TODO: move all these registry operation to the specific registry packages.
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software |
There was a problem hiding this comment.
If this code is from the k0s project, please keep the k0s license. Or could we use those struct directly ?
There was a problem hiding this comment.
Let me keep original license.
| return c.convertIPVSToAddress(clusterFile) | ||
| } | ||
|
|
||
| func (c *K0sConfig) convertIPVSToAddress(clusterFile *v2.Cluster) error { |
There was a problem hiding this comment.
I think the logic here seems to be parsing the host configuration information in the *v2.Cluster structure. Do we need to rename this function? convertClusterHosts ?
There was a problem hiding this comment.
This func mainly use sealer cluster file spec Hosts:IPS as input and out put k0s cluster spec Hosts:SSH:Address, so i named this. If there is a better way to rename it? I think though convertClusterHosts seems like convert some thing in Hosts field, but it don't represent we convert Hosts every field under Hosts.
| return err | ||
| } | ||
| if err := osi.NewAtomicWriter(rootfs + "/k0sctl.yaml").WriteFile(marshal); err != nil { | ||
| return fmt.Errorf("write k0sctl config error: %s", err) |
There was a problem hiding this comment.
failed to write k0sctl config: %v ,err
| } | ||
| } | ||
| } | ||
| return 0, fmt.Errorf("ssh port parse failed") |
| } | ||
|
|
||
| func (c *K0sConfig) joinK0sHosts(ipList []string, hosts []v2.Host, ssh v1.SSH, role string) error { | ||
| for _, masterIP := range ipList { |
|
|
||
| // parseSSHPortByIP parse cluster file ssh port to k0s ssh port. | ||
| func (c *K0sConfig) parseSSHPortByIP(ipAddr string, hosts []v2.Host, ssh v1.SSH) (int, error) { | ||
| for _, host := range hosts { |
There was a problem hiding this comment.
The logic here should be able to simplify。
1.convert hosts to map[string]v1.SSH with ssh arg.
2. get map value from ipAddr.
|
Good job, 👍 I think @sealerio/sealer-maintainers could review this first, and @starComingup can illustrate this pull request in the maintainer's meeting this night. |
|
|
||
| // addHostField generate k0s config spec Host and append it to config file. | ||
| func (c *K0sConfig) addHostField(ipAddr string, port int, role, user string) { | ||
| c2 := c.Spec.Hosts |
There was a problem hiding this comment.
| } | ||
|
|
||
| // Cluster describes launchpad.yaml configuration | ||
| type Cluster struct { |
There was a problem hiding this comment.
I am afraid that we do not think this struct definition. Just make
type K0sConfig struct{
APIVersion string `yaml:"apiVersion"`
Kind string `yaml:"kind"`
Metadata *ClusterMetadata `yaml:"metadata"`
Spec *cluster.Spec `yaml:"spec"`
}
?
There was a problem hiding this comment.
This struct is copy from k0s project. if we need to change it?
pkg/runtime/k0s/runtime.go
Outdated
| *sync.Mutex | ||
| // cluster is sealer clusterFile | ||
| cluster *v2.Cluster | ||
| *Config |
There was a problem hiding this comment.
I think there is no need to set this anonymous field.
just
// Runtime struct is the runtime interface for k0s
type Runtime struct {
*sync.Mutex
// cluster is sealer clusterFile
cluster *v2.Cluster
Vlog int
RegConfig *registry.Config
// clusterFileK0sConfig is k0sctl cluster config file
K0sConfig *k0sctl.K0sConfig
}
pkg/runtime/k0s/runtime.go
Outdated
|
|
||
| // Runtime struct is the runtime interface for k0s | ||
| type Runtime struct { | ||
| *sync.Mutex |
There was a problem hiding this comment.
I do not see any place where used this Mutex. I am wondering if this is redundant? @starComingup
There was a problem hiding this comment.
Copy form k8s, now remove it!
pkg/runtime/k0s/runtime.go
Outdated
| return fmt.Errorf("hosts spec cannot be empty") | ||
| } | ||
| if k.cluster.GetMaster0IP() == nil { | ||
| return fmt.Errorf("controller hosts ip cannot be empty") |
There was a problem hiding this comment.
the error message seems to be not so readable. Just usemaster0 IP cannot be empty. Since no one could recognize what does controller mean.
|
Good job for the k0s initial work. 👍 I think there is still more work to do, maybe in the follow-up pull requests:
LGTM unless all review comments has been finished. and wish to see that we could iterate quickly with small steps. @starComingup |
|
|
||
| k.K0sConfig.DefineConfigFork0s(string(output), k.RegConfig.Domain, k.RegConfig.Port, k.cluster.Name) | ||
|
|
||
| //write k0sctl.yaml to master0 rootfs |
There was a problem hiding this comment.
I think the name of this func explicitly illustrates its ability. Maybe we don't have to annotate it. (It's ok to keep it.)
| } | ||
| c.Spec = &cluster.Spec{} | ||
|
|
||
| type clusterConfig Cluster |
| return nil, err | ||
| } | ||
|
|
||
| if logrus.GetLevel() == logrus.DebugLevel { |
There was a problem hiding this comment.
How about create a function for level transform?
There was a problem hiding this comment.
This would be fine, more readable.
| const ( | ||
| SeaHub = "sea.hub" | ||
| RemoteAddEtcHosts = "cat /etc/hosts |grep '%s' || echo '%s' >> /etc/hosts" | ||
| ContainerdLoginCommand = "nerdctl login -u %s -p %s %s" |
There was a problem hiding this comment.
Is this a general command? I think its readability fails to be better than writing in the code.
There was a problem hiding this comment.
This will use by join phase, so i define it as a const.
starComingup
force-pushed
the
feat/k0s-init
branch
from
53e1ed3 to
8ad8187
Compare
Signed-off-by: starComingup <[email protected]>
Signed-off-by: starComingup <[email protected]>
starComingup
force-pushed
the
feat/k0s-init
branch
from
167781d to
7681037
Compare
Signed-off-by: starComingup <[email protected]>
|
What is more, I think we could add a design document about different cluster runtime support into the sealer docs directory, which could also be a follow-up pull request. @starComingup |
OK, I will keep this in mind and after k0s、k3s all finished and stable, I would like to design it with use cases. |
|
LGTM, and this is related to sealerio/basefs#23 . Please help complete the k3s other part. @starComingup |
Signed-off-by: starComingup [email protected]
Describe what this PR does / why we need it
add k0s init step feature for sealer, it prepare k0sctl install env and image registry and so on.
Does this pull request fix one issue?
NONE, but is a step for k0s clusterRuntime.
Describe how you did it
NONE
Describe how to verify it
use unit test for check it work!
Todo list:
Note:
Why we don't need initMaster0 and GetKubectlConfig func like k8s init step?
In
k0sctldesign which lead the cluster creation, cluster init phase and node join phase integrate tok0sctl applyand it will reconcile byk0sctlitself, so to keep sealer now createProcess logic c.init and c.join. I would like to implement thek0sctl applycmd in join function rather than init function. And after apply, and we can usek0sctl kubeconfigto create the config file for kubectl.