feat: support ha mode for local registry #1901

kakaZhou719 · 2022-12-02T02:14:31Z

Describe what this PR does / why we need it

Master0 plays an important role in sealer cluster, it hosts many important components such as sealer builtin registry,
application images launched history and others cluster configuration files. if the master0 node is abnormal, a highly
available solution is required to recover on other master nodes.

Does this pull request fix one issue?

Describe how you did it

expose Registry struct to clusterfile which is contains configurations about local registry and remote registry.
sync all registry configuration and registry data to the registry deploy hosts when init the cluster.
set ha mode as default value for local registry.
bugfix: reset host alias about cluster api server to master's their own IP, after join the new master role.
optimized some flags of the sealer cluster command.

Describe how to verify it

if not set any registry configs, sealer will use non ha mode as default.
if set: below is my test clusterfile, this will prepare to deploy registry service on three nodes(172.16.26.162,172.16.26.163,172.16.26.164) with customized domain port, basic auth and insure mode options.

apiVersion: sealer.cloud/v2
kind: Cluster
metadata:
  creationTimestamp: null
  name: my-cluster
spec:
  hosts:
  - ips:
    - 172.16.26.162
    roles:
    - master
    ssh: {}
  - ips:
    - 172.16.26.163
    roles:
    - master
    ssh: {}
  - ips:
    - 172.16.26.164
    roles:
    - master
    ssh: {}
  - ips:
    - 172.16.26.165
    roles:
    - node
    ssh: {}
  registry: 
    localRegistry:
      haMode: true
      domain: kakazhou
      port: 5000 
      username: kakazhou
      password: kakazhou123
  image: registry.cn-qingdao.aliyuncs.com/sealer-io/kubernetes:v1.22.15
  ssh:
    passwd: XXX
    pk: /root/.ssh/id_rsa
    port: "22"
    user: root

Special notes for reviews

luckyfengyong · 2022-12-05T07:38:50Z

Please make sure it won't break backward compatibility and upgrade

cmd/sealer/cmd/cluster/apply.go

pkg/infradriver/ssh_infradriver.go

pkg/runtime/kubernetes/join_masters.go

codecov-commenter · 2022-12-12T01:57:33Z

Codecov Report

Base: 21.11% // Head: 21.70% // Increases project coverage by +0.58% 🎉

Coverage data is based on head (e7fbae0) compared to base (fbd817f).
Patch coverage: 24.73% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1901      +/-   ##
==========================================
+ Coverage   21.11%   21.70%   +0.58%     
==========================================
  Files          74       78       +4     
  Lines        6762     7155     +393     
==========================================
+ Hits         1428     1553     +125     
- Misses       5148     5395     +247     
- Partials      186      207      +21

Impacted Files	Coverage Δ
build/kubefile/parser/kubefile.go	`53.44% <0.00%> (-1.91%)`	⬇️
pkg/cluster-runtime/apps.go	`0.00% <0.00%> (ø)`
pkg/cluster-runtime/hook.go	`3.75% <0.00%> (-0.05%)`	⬇️
pkg/cluster-runtime/installer.go	`0.00% <0.00%> (ø)`
pkg/cluster-runtime/scale.go	`0.00% <0.00%> (ø)`
pkg/cluster-runtime/uninstall.go	`0.00% <0.00%> (ø)`
pkg/image/reference/util.go	`83.78% <ø> (ø)`
pkg/imagedistributor/scp_distributor.go	`0.00% <0.00%> (ø)`
pkg/logger/formatter.go	`81.13% <ø> (ø)`
pkg/runtime/kubernetes/kubeadm/kubeadm_config.go	`15.51% <0.00%> (-0.28%)`	⬇️
... and 19 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

starnop · 2022-12-12T03:45:52Z

cmd/sealer/cmd/cluster/apply.go

@@ -99,17 +95,17 @@ func NewApplyCmd() *cobra.Command {

 			if extension.Type == v12.AppInstaller {


How about extract a public method to determine if it is an app image？

starnop · 2022-12-12T07:32:02Z

cmd/sealer/cmd/cluster/run.go

+
+	logrus.Infof("start to apply with mode(%s)", common.ApplyModeLoadImage)
+	deployHosts := infraDriver.GetHostIPListByRole(common.MASTER)
+	master0 := deployHosts[0]


it will panic here if the length of deploy hosts is zero

starnop · 2022-12-12T07:34:03Z

cmd/sealer/cmd/utils/cluster.go

 		}
+		if _, ok := node.Labels["node-role.kubernetes.io/master"]; ok {


use MasterRoleLabel here

starnop · 2022-12-12T07:34:50Z

cmd/sealer/cmd/utils/cluster.go

+}
+
+func getNodeAddress(node corev1.Node) net.IP {
+	if len(node.Status.Addresses) < 1 {


what will happen if len(node.Status.Addresses)==1?

add comment here if the logic is right

starnop · 2022-12-12T09:08:44Z

cmd/sealer/cmd/cluster/run-app.go

 }

 func installApplication(appImageName string, launchCmds []string, extension v12.ImageExtension,
-	infraDriver infradriver.InfraDriver, imageEngine imageengine.Interface) error {
+	infraDriver infradriver.InfraDriver, imageEngine imageengine.Interface, createMode string) error {


installMode ？

starnop · 2022-12-12T09:09:21Z

cmd/sealer/cmd/cluster/run-app.go

-		DeployHost: infraDriver.GetHostIPListByRole(common.MASTER)[0],
-		Registry:   config,
+	if createMode == common.ApplyModeLoadImage {
+		return loadImageMode(infraDriver, distributor)


loadImageMode

the function name should be a verb

starnop · 2022-12-12T09:13:32Z

cmd/sealer/cmd/cluster/run.go

@@ -148,10 +152,9 @@ func NewRunCmd() *cobra.Command {
 	return runCmd
 }

-func createNewCluster(clusterImageName string, infraDriver infradriver.InfraDriver, imageEngine imageengine.Interface, cf clusterfile.Interface) error {
+func createNewCluster(clusterImageName string, infraDriver infradriver.InfraDriver, imageEngine imageengine.Interface, cf clusterfile.Interface, createMode string) error {


It looks like the clusterImageName is unused

starnop · 2022-12-12T09:14:44Z

cmd/sealer/cmd/cluster/run.go

+func loadImageMode(infraDriver infradriver.InfraDriver, distributor imagedistributor.Distributor) error {
+	regConfig := infraDriver.GetClusterRegistryConfig()
+	// only support load image to local registry at present
+	if regConfig.LocalRegistry == nil {


add TODO here

starnop · 2022-12-12T09:20:44Z

pkg/infradriver/infradriver.go

@@ -17,6 +17,8 @@ package infradriver
 import (
 	"net"

+	v2 "github.com/sealerio/sealer/types/api/v2"
+


unused blank

starnop · 2022-12-12T09:22:19Z

pkg/infradriver/ssh_infradriver.go

-	if err := checkAllHostsSameFamily(ret.hosts); err != nil {
+	// check registry config is valid,
+	// make sure external registry domain is valid
+	if cluster.Spec.Registry.ExternalRegistry != nil {


in the future, we should not distinguish the local registry and external registry

add TODO here

starnop · 2022-12-12T09:33:32Z

set non-ha mode as the default value for the local registry if we support config it, and then we will make sure it won't break backward compatibility and upgrade

kakaZhou719 · 2022-12-13T02:33:22Z

pkg/runtime/kubernetes/kubeadm/kubeadm_config.go

@@ -189,7 +189,9 @@ func NewKubeadmConfig(fromClusterFile KubeadmConfig, fromFile string,
 	conf.APIServer.ExtraArgs[EtcdServers] = getEtcdEndpointsWithHTTPSPrefix(masters)
 	conf.IPVS.ExcludeCIDRs = append(conf.KubeProxyConfiguration.IPVS.ExcludeCIDRs, fmt.Sprintf("%s/32", apiServerVIP))
 	conf.KubeletConfiguration.CgroupDriver = cgroupDriver
-
+	// set cluster image repo,kubeadm will pull container image from this registry.
+	conf.ClusterConfiguration.ImageRepository = imageRepo


@VinceCui , here will affects the configuration of the ackd image. pls review.

types/api/v2/cluster_types.go

pkg/infradriver/ssh_infradriver.go

VinceCui · 2022-12-13T06:16:16Z

pkg/registry/local.go

@@ -326,7 +249,7 @@ func (c *localSingletonConfigurator) configureAccessCredential(hosts []net.IP) e
 	return nil
 }

-func (c *localSingletonConfigurator) copy2RemoteHosts(src, dest string, hosts []net.IP) error {
+func (c *localConfigurator) copy2RemoteHosts(src, dest string, hosts []net.IP) error {


this maybe a common func of infraDriver.

VinceCui · 2022-12-13T06:20:07Z

pkg/registry/installer.go

+	}
+}
+
+type localInstaller struct {


localInstaller and local registry configurator, difference? why need them both?

Installer provide registry lifecycle management local registry will implement it. Configurator provide registry configuration management, local registry and external registry will implement it.

VinceCui · 2022-12-13T06:21:56Z

types/api/v2/cluster_types.go

+	// if LocalRegistry is not specified, default value is true.
+	HaMode bool `json:"ha_mode,omitempty"`
+	// InsecureMode indicated that whether the local registry is exposed in HTTPS.
+	// if true sealer will generate default ssl cert.


if true, sealer will NOT generate default ssl cert.

update reviews update reviews

VinceCui · 2022-12-15T01:56:48Z

LGTM

starnop · 2022-12-15T02:23:08Z

LGTM

* feat: support recover master0 * feat: support ha mode for local registry update reviews update reviews

github-actions bot added Clusterfile test labels Dec 2, 2022

kakaZhou719 force-pushed the feat-recover-master0 branch from 08cd5ca to b9f2f31 Compare December 2, 2022 08:24

VinceCui reviewed Dec 7, 2022

View reviewed changes

cmd/sealer/cmd/cluster/apply.go Outdated Show resolved Hide resolved

pkg/infradriver/ssh_infradriver.go Outdated Show resolved Hide resolved

feat: support recover master0

1f70772

kakaZhou719 force-pushed the feat-recover-master0 branch from b9f2f31 to 1f70772 Compare December 8, 2022 02:02

VinceCui reviewed Dec 8, 2022

View reviewed changes

pkg/runtime/kubernetes/join_masters.go Outdated Show resolved Hide resolved

kakaZhou719 changed the title ~~feat: support recover master0~~ feat: support ha mode for local registry Dec 12, 2022

starnop reviewed Dec 12, 2022

View reviewed changes

kakaZhou719 force-pushed the feat-recover-master0 branch from e7fbae0 to 31186ad Compare December 12, 2022 10:09

kakaZhou719 commented Dec 13, 2022

View reviewed changes

VinceCui reviewed Dec 13, 2022

View reviewed changes

kakaZhou719 force-pushed the feat-recover-master0 branch 3 times, most recently from fe86cf7 to bfbfaf3 Compare December 13, 2022 11:03

feat: support ha mode for local registry

2a2cca2

update reviews update reviews

kakaZhou719 force-pushed the feat-recover-master0 branch from bfbfaf3 to 2a2cca2 Compare December 14, 2022 11:30

starnop approved these changes Dec 15, 2022

View reviewed changes

starnop merged commit ce6e62a into sealerio:main Dec 15, 2022

jsparter pushed a commit to jsparter/sealer that referenced this pull request Dec 16, 2022

feat: support ha mode for local registry (sealerio#1901)

3cbeb27

* feat: support recover master0 * feat: support ha mode for local registry update reviews update reviews

kakaZhou719 deleted the feat-recover-master0 branch March 8, 2023 05:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support ha mode for local registry #1901

feat: support ha mode for local registry #1901

kakaZhou719 commented Dec 2, 2022 •

edited

Loading

luckyfengyong commented Dec 5, 2022

codecov-commenter commented Dec 12, 2022 •

edited

Loading

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop Dec 12, 2022

starnop commented Dec 12, 2022

kakaZhou719 Dec 13, 2022

VinceCui Dec 13, 2022

VinceCui Dec 13, 2022

kakaZhou719 Dec 13, 2022

VinceCui Dec 13, 2022

VinceCui commented Dec 15, 2022

starnop commented Dec 15, 2022

		@@ -99,17 +95,17 @@ func NewApplyCmd() *cobra.Command {

		if extension.Type == v12.AppInstaller {

		}
		if _, ok := node.Labels["node-role.kubernetes.io/master"]; ok {

feat: support ha mode for local registry #1901

feat: support ha mode for local registry #1901

Conversation

kakaZhou719 commented Dec 2, 2022 • edited Loading

Describe what this PR does / why we need it

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

luckyfengyong commented Dec 5, 2022

codecov-commenter commented Dec 12, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

starnop commented Dec 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VinceCui commented Dec 15, 2022

starnop commented Dec 15, 2022

kakaZhou719 commented Dec 2, 2022 •

edited

Loading

codecov-commenter commented Dec 12, 2022 •

edited

Loading